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

Daniel Fuchs daniel.fuchs at oracle.com
Tue Aug 6 13:37:11 UTC 2019


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.



More information about the net-dev mailing list