[RFR]: 8220575: Replace hardcoded 127.0.0.1 in URLs with new URI builder

Chris Hegarty chris.hegarty at oracle.com
Tue Mar 26 14:15:14 UTC 2019


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.




More information about the net-dev mailing list