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