6563286: HttpURLConnection.followRedirect(..) follows malformed url.

Kurchi Hazra kurchi.subhra.hazra at oracle.com
Tue Jun 25 11:57:38 PDT 2013


Hi Andreas,

    Your changes look good to me.  5069130 had been closed as not a 
defect - since we throw an
unchecked exception, and was decided to be the right course of action. 
Now, I don't have a problem with changing
the code to throw a checked exception in this case - but we have to wait 
for an OpenJDK reviewer to agree as well.

  My only suggestion is to add a good message to the IOException being 
thrown, so that it is clear that the URL sent by
the server is problematic and make debugging easier. In the test, the 
first RuntimeException message being
printed in line 46 is probably misleading, but this is a minor issue.

   I'll wait for a JDK reviewer's input on this. I can sponsor the 
change for you. We may need an internal approval here
for the minor API change.

Thanks,
- Kurchi

On 6/24/2013 12:42 PM, Andreas Rieber wrote:
> Hi,
>
> here a small fix for 2 older issues. First i wrote the test for wrong
> URL at connection time and at redirect time.
>
> Bug(s):
> http://bugs.sun.com/view_bug.do?bug_id=6563286
> http://bugs.sun.com/view_bug.do?bug_id=5069130
>
> Webrev:
> http://cr.openjdk.java.net/~arieber/6563286/webrev.00/
>
> What happens is that the java.net.ProxySelector.select(URI uri) is
> called, which throws the IllegalArgumentException if uri is null. But
> it uses sun.net.spi.DefaultProxySelector.java and also throws the
> exception if uri.getScheme() is null or uri.getHost() is null. I
> updated the javadoc in ProxySelector.
>
> To change the exception to an IOException would mean a wider
> refactoring and API change, not good. So i checked down to
> net.www.protocol.http.HttpURLConnection.java.
>
> There the IllegalArgumentException can be caught and thrown as
> IOException. This will handle wrong URLs in both cases (connect and
> redirect). I checked also all other possible cases but they are
> handled with correct exceptions.
>
> I run all tests on ubuntu, so a test run on all relevant platforms is
> required.
>
> thanks for checking this one
> Andreas

-- 
-Kurchi




More information about the net-dev mailing list