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
