RFR 8199849: HttpServer/BasicAuthenticator: unicode bytes are not correctly handled

[Michael McMahon]

Thanks Daniel. Suggested changes made and new webrev at:

http://cr.openjdk.java.net/~michaelm/8199849/webrev.4/

- Michael.

On 06/08/2019, 14:37, Daniel Fuchs wrote:
> Hi Michael, Patrick,
>
> BasicAuthentication.java:
>
>   The two constructors might benefit from a bit of refactoring
>   allowing them to share the code that encode the credentials.
>
> BasicAuthenticator.java:
>
>   76         if ("".equals(realm))
>
>   I'd suggest using
>
>              if (realm.isEmpty())
>
>   If you do so you might also decide to add a comment that
>   this is an implicit null check and decide to remove the
>   previous line: Objects.requiresNonNull(realm).
>
> BasicAuthenticatorCharset.java:
>
>   This appears as a new test, yet it has a copyright
>   line 2005, 2019, and two @bug refs?
>
>   typo: setAuthenticatonPW
>
>  146         // should fail once with UNICODE_PW and unsupporting 
> character set
>  147         if (failCount != 1)
>  148             throw new RuntimeException("Fail count exceeded: 1 != 
> " + failCount);
>
>   well... OK. But technically the test could succeed
>   if the failing charset didn't fail but the passing
>   charset did.
>
>   It would be much better to test that the charset that
>   is expected to fail fails and that the charset that
>   is expected to pass passes...
>
> TestHttpUnicode.java
>
>   Are the copyright years OK or is this just
>   copy-paste mistake?
>
>     44     static class testAuthenticator extends 
> java.net.Authenticator {
>
>   can this class start with an upper case please?
>   it's a bit confusing given that the server authenticator is a
>   stored in a variable named `testAuthenticator` as well.
>
>
> Otherwise, looks good to me!
>
> best regards,
>
> -- daniel
>
>
> On 06/08/2019 12:44, Michael McMahon wrote:
>> This change went through a couple of rounds of review before, and I 
>> got diverted to something else
>> at the time. So, just picking it up now.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8199849
>>
>> Webrev: http://cr.openjdk.java.net/~michaelm/8199849/webrev.3/index.html
>>
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8215275
>>
>> The two tests were contributed by Patrick Concannon.
>>
>> Thanks,
>> Michael.
>