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

2019-08-12 Thread Daniel Fuchs
Looks good Michael! Nice cleanup. best regards, -- daniel On 12/08/2019 16:57, Michael McMahon wrote: I have made some more changes and there is a new webrev at: http://cr.openjdk.java.net/~michaelm/8199849/webrev.5/ I would like to finalise the CSR soon also. So, any further comments apprec

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

2019-08-12 Thread Chris Hegarty
> On 12 Aug 2019, at 16:57, Michael McMahon > wrote: > > I have made some more changes and there is a new webrev at: > > http://cr.openjdk.java.net/~michaelm/8199849/webrev.5/ Looks good, with good test coverage. -Chris

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

2019-08-12 Thread Michael McMahon
I have made some more changes and there is a new webrev at: http://cr.openjdk.java.net/~michaelm/8199849/webrev.5/ I would like to finalise the CSR soon also. So, any further comments appreciated. Thanks, Michael On 06/08/2019, 17:20, Michael McMahon wrote: Thanks Daniel. Suggested changes m

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

2019-08-06 Thread Daniel Fuchs
On 06/08/2019 17:20, Michael McMahon wrote: Thanks Daniel. Suggested changes made and new webrev at: http://cr.openjdk.java.net/~michaelm/8199849/webrev.4/ Thanks Michael. LGTM now! best regards, -- daniel - Michael. On 06/08/2019, 14:37, Daniel Fuchs wrote: Hi Michael, Patrick, BasicA

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

2019-08-06 Thread 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 t

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

2019-08-06 Thread Daniel Fuchs
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

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

2019-08-06 Thread Michael McMahon
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.j

Re: RFR: 8199849 HttpServer/BasicAuthenticator: unicode bytes are not correctly handled and no workaround is provided

2018-12-12 Thread Michael McMahon
Hi Daniel, On 12/12/2018, 11:46, Daniel Fuchs wrote: Hi Michael, BasicAuthentication.java: An alternative to: byte[] passwdBytes = (new String(passwd)).getBytes(cs); would be: ByteBuffer bb = cs.newEncoder() .encode(CharBuffer.wrap(passwd)); then use ByteBuffer::get to copy the b

Re: RFR: 8199849 HttpServer/BasicAuthenticator: unicode bytes are not correctly handled and no workaround is provided

2018-12-12 Thread Daniel Fuchs
Hi Michael, BasicAuthentication.java: An alternative to: byte[] passwdBytes = (new String(passwd)).getBytes(cs); would be: ByteBuffer bb = cs.newEncoder() .encode(CharBuffer.wrap(passwd)); then use ByteBuffer::get to copy the bytes into `concat` Or better you could construct a Cha

RFR: 8199849 HttpServer/BasicAuthenticator: unicode bytes are not correctly handled and no workaround is provided

2018-12-12 Thread Michael McMahon
Could I get the following webrev reviewed please? http://cr.openjdk.java.net/~michaelm/8199849/webrev.1/ A CSR for the small API change will follow. Thanks, Michael