[9] RFR: 8138990: Implementation of HTTP Digest authentication may be more flexible

[Artem Smotrakov]

Hi Michael,

Thanks for review, it looks like BNF notation uses only a comma as a 
separator

http://www.w3.org/Notation.html

...
<l>#<m>element

indicating at least l and at most m elements, each separated by one or 
more commas (",").
...

And here is "qop" definition from https://tools.ietf.org/html/rfc2617

...
       qop-options       = "qop" "=" <"> 1#qop-value <">
       qop-value         = "auth" | "auth-int" | token
...

Please take a look at updated webrev:

http://cr.openjdk.java.net/~asmotrak/http_auth_digest/webrev.01/

Artem

On 12/22/2015 05:59 AM, Michael McMahon wrote:
> Hi Artem,
>
>
> On 04/12/15 11:41, Artem Smotrakov wrote:
>> Hello,
>>
>> Please review this small fix for DigestAuthentication class.
>>
>> 1. Added a check in DigestAuthentication.setNonce(String) that nonce 
>> is not null. NPE may happen if a buggy HTTP server returns 
>> "WWW-Authenticate" header which doesn't contain a "nonce" field. 
>> According to RFCs 2069 [1] and 2617 [2], this is not expected 
>> behaviour, but it would be better if an HTTP client threw a checked 
>> IOException instead of NPE.
>>
>
> That's fine.
>
>> 2. Updated DigestAuthentication.setQop(String) method to accept both 
>> a whitespace and a comma as a delimiter. RFC 2617 [2] says that "qop" 
>> may contain more than one token, but it doesn't specify a delimiter 
>> for "qop" field in "WWW-Authenticate" header. There is an example of 
>> "WWW-Authenticate" header in RFC 2617 [2] where a comma is used as a 
>> delimiter of value in "qop" field.
>>
>
> It looks like the BNF specification mandates a comma and optional 
> linear white space.
> So, the old code was buggy, but we didn't see the problem because 
> there is typically
> only at most ever one value used for the qop field. But, to be 
> strictly correct, we would
> have to check for TABs also. So, I think the correct behavior is to 
> delimit using comma
> and remove any white space
>
> - Michael.
>
>> 3. Added a test for Digest authentication.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8138990
>> Webrev: http://cr.openjdk.java.net/~asmotrak/http_auth_digest/webrev.00/
>>
>> [1] https://tools.ietf.org/html/rfc2069
>> [2] https://tools.ietf.org/html/rfc2617
>>
>> Artem
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20151229/8c8d3ffb/attachment-0001.html>