[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