[RFR]: 8220575: Replace hardcoded 127.0.0.1 in URLs with new URI builder
Arthur Eubanks
aeubanks at google.com
Tue Mar 26 16:51:10 UTC 2019
Changed B6890349.java to use URL constructor.
Changed sequence of URIBuilder calls to `.host().port().path()`.
Added missing `.path("/")`.
Added logging for most of the constructed URLs.
PTAL:
http://cr.openjdk.java.net/~aeubanks/8220575/webrev.01/test/jdk/com/sun/net/httpserver/bugs/B6373555.java.udiff.html
I was considering adding a buildURL() to URIBuilder since so many of these
URIs end up getting converted to a URL in the end. Not sure if that's a
good idea or not.
On Tue, Mar 26, 2019 at 7:15 AM Chris Hegarty <chris.hegarty at oracle.com>
wrote:
> Arthur,
>
> I like the way this is turning out.
>
> > On 26 Mar 2019, at 12:05, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> >
> > ...
> > 2. B6890349.java
> >
> > Using URI.toURL() in this test will change the nature of
> > the test. I believe that in this specific case using the
> > multi-arg URL constuctor should be preferred.
> > The reason is that URI as per its specification encodes
> > illegal characters, while URL as per its specification,
> > does not. So going through URI here will lead to a
> > different URL than the one that the test is testing.
> >
> > I see from your mail that you have noticed the difference :-)
> >
> > For this one test I'd suggest to make an exception and
> > change the URL construction to:
> >
> > URL u = new URL("http",
> > InetAddress.getLoopbackAddress().getHostAddress(),
> > port,
> > "/foo\nbar");
> >
> > Even if we get to deprecating the URL constructors in the
> > future this would still be a valid case for using them in
> > a test.
>
> Right. The above accepting-illegal-characters behaviour is one of
> the reasons, non-test code, should avoid the URL constructors.
>
> > 3. on the 'nit' side it's a bit strange to see
> > the sequence of calls `.host(..).path(..).port(..)`,
> > as `.host(..).port(..).path(..)` would feel
> > natural - but don't feel obliged to make this change.
>
> URI component-wise ordering makes the code a little easier to
> read. I’d just do it.
>
> I dislike noisy tests, but it would be helpful for these tests to print
> the URL that has been constructed ( System.out.println("URL: " + url) ).
> It will make it easier to verify the test's behaviour on different
> system configuration ( I usually just look in the jtr file )
>
> -Chris.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/net-dev/attachments/20190326/c3eb3547/attachment.html>
More information about the net-dev
mailing list