On Tue, 26 Oct 2021 13:31:35 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:

>> This change ensures that the realm string passed to the BasicAuthenticator 
>> constructor is a quoted-string, as per RFC7230 [1]. A Utils class is added 
>> to jdk.httpserver/sun.net.httpserver that holds the new isQuotedString() 
>> method and the pre-existing isValidName() method (previously in ServerImpl.) 
>> Two tests are included:
>> - BasicAuthenticatorRealm.java to check that Latin-1 chars in the realm 
>> string are transported correctly,
>> - BasicAuthenticatorExceptionCheck.java to check realm strings with escaped 
>> quotes.
>> 
>> Testing: tier 1-3.
>> 
>> [1] https://datatracker.ietf.org/doc/html/rfc7230
>
> src/jdk.httpserver/share/classes/com/sun/net/httpserver/BasicAuthenticator.java
>  line 77:
> 
>> 75:      * <p>Where a backslash ("\") is used as quoting mechanism within 
>> the realm
>> 76:      * string, it must be escaped by two preceding backslashes, for 
>> example
>> 77:      * {@code "foo\\\"bar\\\""} will be embedded as {@code "foo\"bar\""}.
> 
> I would drop this sentence as I find it confusing - even though I understand 
> what you are trying to say.
> 
> I would replace it with something like:
> 
> 
> The value of the {@code realm} parameter will be embedded in a quoted string. 
> Any quote it contains must be escaped by the caller.

I still think it is too much of a corner case to impose on the API doc so much. 
How about changing the @throws to


      * @throws IllegalArgumentException if realm is an empty string or is not 
correctly
      *         escaped, as specified in <a 
href="https://tools.ietf.org/html/rfc7230#section-3.2";>
      *         RFC 7230 section-3.2</a>.
 ```

-------------

PR: https://git.openjdk.java.net/jdk/pull/6117

Reply via email to