Further testing has shown the escaping to be necessary. Apologies folks,
I'm withdrawing this review.
-Rob
On 06/07/17 04:57, Rob McKenna wrote:
> Thanks Vyom, updated test at:
>
> http://cr.openjdk.java.net/~robm/8177806/webrev.02/
>
> The sleep proved necessary in a similar earlier test so I'll leave it in
> there. (iirc the request failed as it was sent before the server started
> accepting connections)
>
> -Rob
>
> On 06/07/17 01:51, Vyom Tewari wrote:
> > Hi Rob,
> >
> > Code change looks OK, below are few comments on test case.
> >
> > 1-> Remove System.exit(1) and throw Exception from main if test fails.
> >
> > 2-> Put srv.close() in finally block, in case of exception close will not be
> > called.
> >
> > 3-> Do you need Thread.sleep(500) ?. I think it is not required.
> >
> > 4-> i will suggest you to use below tag order.
> >
> > /**
> > * @test
> > * @bug 8177806
> > * @summary Test NameImpl escape character processing
> > * @modules java.naming/javax.naming:open
> > * @run main/othervm LdapEscapeTest
> > */
> >
> > Thanks,
> >
> > Vyom
> >
> >
> > On Thursday 06 July 2017 04:14 AM, Rob McKenna wrote:
> > >Hi folks,
> > >
> > >Looking for a review for the following change.
> > >
> > >NameImpl.java is stripping escape characters unnecessarily:
> > >
> > >http://cr.openjdk.java.net/~robm/8177806/webrev.01/
> > >https://bugs.openjdk.java.net/browse/JDK-8177806
> > >
> > >Thanks,
> > >
> > > -Rob
> > >
> >
