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

[Artem Smotrakov]

Hi Michael,

On 01/04/2016 02:28 AM, Michael McMahon wrote:
> On 30/12/15 03:22, Artem Smotrakov wrote:
>> 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 (",").
>> ...
>>
>
> Hi Artem,
>
> The BNF definitions used in RFC 2617 come from section 2.1 of RFC 2616
> which allows linear white space between tokens.
>
> #rule
>        A construct "#" is defined, similar to "*", for defining lists of
>        elements. The full form is "<n>#<m>element" indicating at least
>        <n> and at most <m> elements, each separated by one or more commas
>        (",") and OPTIONAL linear white space (LWS).
If I read it correctly, it means that a comma is used as a separator, 
but there also may be a linear whitespace after a comma. So for example 
"auth,auth-int" and "auth, auth-int" are equal, and should result to a 
list of two elements. Current version of webrev seems to follow it 
(first, it splits a string by a comma, and then ignores white spaces):

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

Am I missing something?

Artem

>
> So, I think we should be conservative and check for white-space.
>
> - Michael.
> Michael
>> 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/20160104/93a5d33b/attachment-0001.html>