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

[Andreas Rieber]

Hi,

i did one more update to avoid duplicate code (3 times used). The 
redirect status codes are checked now in one static method. I added 
"Malformed redirect" to the exception, so it is possible to trace. All 
tests run on ubuntu.

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

thanks
Andreas


On 26.06.13 21:50, Andreas Rieber wrote:
> 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
>
>
>