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

[Andreas Rieber]

Hi Kurchi,

i removed the closed issue. Then i checked again the HttpURLConnection 
and moved the check so really only redirects are detected and handled 
properly with the IOException. The test i updated and moved to the right 
place.

Bug:
http://bugs.sun.com/view_bug.do?bug_id=6563286

Updated webrev:
http://cr.openjdk.java.net/~arieber/6563286/webrev.01/

cheers
Andreas

PS: i also liked the autocloseable, was waiting to long for the builds 
and had to do something else in between ;-)


On 25.06.13 22:18, Andreas Rieber wrote:
> Hi Kurchi,
>
> thanks for the first feedback. The problem i had was that the URL is 
> right (according spec) but at protocol level the used URI is wrong. 
> The ProxySelector is also right to send the IllegalArgumentException 
> (just needed to be documented to do that). And yes, a better error 
> message should come out when sending the IOException. So lets wait for 
> some more input on that issue.
>
> But where do you see a API change, i tried to avoid that?
>
> cheers
> Andreas
>
>
> On 25.06.13 20:57, Kurchi Hazra wrote:
>> 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