[rfc][icedtea-web] fixfor portalbank.no

[Jiri Vanek]

On 04/29/2013 06:15 PM, Omair Majid wrote:
> On 04/29/2013 12:03 PM, Jiri Vanek wrote:
>> Hi!
>>
>> Today I accidentally stumbled accross another regression in HEAD
>> portalbank.no donot implements HTTP requests but provides desired jars
>> on GET.
>> So I made (findBestUrl) less strict.
>
> The change itself looks okay to me. We are dealing with more-or-less
> misconfigured servers here, so I guess we should not be too concerned
> about optimizing this case. A fallback to GET seems appropriate.
>
>> * net/sourceforge/jnlp/cache/ResourceTracker : (findBestUrl)
>>    now trying GET after each error request of HEAD
>
> Please complete the ChangeLog ;)

ups:)


* net/sourceforge/jnlp/cache/ResourceTracker : (findBestUrl)
now trying GET after each error request of HEAD type. Changed and
added debug messages. (getUrlResponseCode) closing of stream
moved to separate method HttpUtils.consumeAndCloseConnectionSilently
* net/sourceforge/jnlp/util/HttpUtils.java: new file designed  for
http utils. Now contains (consumeAndCloseConnection) and
(consumeAndCloseConnectionSilently) which calls consumeAndCloseConnection
but do not rethrow exception
* netx/net/sourceforge/jnlp/util/StreamUtils.java: removed (consumeAndCloseInputStream)
now improved and moved to HttpUtils


tests:

* netx/net/sourceforge/jnlp/cache/Resource.java: added fixme to warn before wrong url comparator
* netx/net/sourceforge/jnlp/Version.java: removed useless main. Its purpose moved to new
* tests/netx/unit/net/sourceforge/jnlp/VersionTest: some small tests to version class
* tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTrackerTest.java: added tests to
(getUrlResponseCode) and (findBestUrl)
* tests/netx/unit/net/sourceforge/jnlp/util/HttpUtilsTest.java: added tests for
(consumeAndCloseConnectionSilently) and (consumeAndCloseConnection)
* tests/netx/unit/net/sourceforge/jnlp/util/UrlUtilsTest: added license header
* tests/test-extensions/net/sourceforge/jnlp/ServerLauncher.java: and
* tests/test-extensions/net/sourceforge/jnlp/TinyHttpdImpl.java: added support for simulation
of not working HEAD request



>
>> @@ -910,7 +911,7 @@
>>
>>                   int responseCode = getUrlResponseCode(url, requestProperties, "HEAD");
>>
>> -                if (responseCode == HttpURLConnection.HTTP_NOT_IMPLEMENTED ) {
>> +                if (responseCode < 200 || responseCode >= 300) {
>>                       System.err.println("NOTE: The server does not appear to support HEAD requests, falling back to GET requests.");
>
> The error message is not very accurate now. We are going through a list
> of URLs, searching for the best one. It's likely the URLs will not exist
> and so we will probably get 404 errors. The message can be very
> misleading in that case.

Ok. I have improved the message.
>
>> +public class HttpUtils {
>
> Could you add a unit test for this?

Ok, I have at the end added unittests to all methods I have touched. Imho it is worthy - but no more 
bugs was found this time :)

If you do not like the tests,please let me push the fix and discuss the tests separately.

Also do you mind you can check - Re: [rfc] [icedtea-web] renaming Messages_cs_CZ to Messages_cs (was 
Re:ITW tranlsation) ? It should just need and experience yes/now. The code change itself should be 
very simple then (just renaming the file..)
>
> Cheers,
> Omair
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: fixedPortalBank-tests.diff
Type: text/x-patch
Size: 34233 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130430/e04a3885/fixedPortalBank-tests.diff 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fixedPortalBank-fix.diff
Type: text/x-patch
Size: 7801 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130430/e04a3885/fixedPortalBank-fix.diff