[rfc][icedtea-web] fixfor portalbank.no
Jiri Vanek
jvanek at redhat.com
Thu May 2 02:44:50 PDT 2013
...snip
>> return responseCode;
>> }
>> @@ -891,7 +887,7 @@
>> * @param resource the resource
>> * @return the best URL, or null if all failed to resolve
>> */
>> - private URL findBestUrl(Resource resource) {
>> + URL findBestUrl(Resource resource) {
>> DownloadOptions options = downloadOptions.get(resource);
>> if (options == null) {
>> options = new DownloadOptions(false, false);
>> @@ -910,8 +906,9 @@
>>
>> int responseCode = getUrlResponseCode(url, requestProperties, "HEAD");
>>
>> - if (responseCode == HttpURLConnection.HTTP_NOT_IMPLEMENTED ) {
>> - System.err.println("NOTE: The server does not appear to support HEAD
>> requests, falling back to GET requests.");
>> + if (responseCode < 200 || responseCode >= 300) {
>> + System.err.println("NOTE: The server has returned " + responseCode + " code
>> for HEAD request for " + url.toExternalForm() + ". IcedTea-Web will fail back to GET.");
>> + System.err.println("NOTE: it is possible that server is just not supporting
>> HEAD request, but more likely the file is really invalid or missing");
>
> This is a poor error message, please remove it.
> Or if you wish, you may keep the old message in the case that 'responseCode ==
> HttpURLConnection.HTTP_NOT_IMPLEMENTED'. In that case the server is telling us they do not support
> HEAD requests.
> Actually, this method would be a bit better (and not require 2 connections per failure) if the outer
> loop was:
>
> /* Use GET request as a fallback in the rare case the server does not support HEAD requests */
> final String requestMethods = {"HEAD", "GET"};
> for (String requestMethod : requestMethods) {
> for (URL url : urls) {
> ....snip...
> int responseCode = getUrlResponseCode(url, requestProperties, requestMethod);
> ....snip...
>
> Then we need not fall-back inside the loop at all. The logic would remain pretty much the same.
> In this case we can still explicitly check for ==NOT_IMPLEMENTED in which case we will report
> '"Server does not support " + requestMethod'. (Not supporting GET would really be something special
> though :-)
>
> I am in favour of this variation (Omair seemed to prefer it too :-). It guarantees the largest
> benefit from HEAD requests in the case of trial-and-error jar name guesses, for just a little bit of
> restructuring.
>
Much better indeed. Done
>> /* Fallback: use GET request in the rare case the server does not support
>> HEAD requests */
>> responseCode = getUrlResponseCode(url, requestProperties, "GET");
>> }
>> @@ -922,6 +919,10 @@
>> System.err.println("best url for " + resource.toString() + " is " +
>> url.toString());
>> }
>> return url; /* This is the best URL */
>> + } else {
>> + if (JNLPRuntime.isDebug()) {
>> + System.err.println(resource.toString() + "'s url " + url.toString()+"
>> returned "+responseCode);
>> + }
>
> OK.
>
>> }
>> } catch (IOException e) {
>> // continue to next candidate
>> diff -r e34db561b7b9 netx/net/sourceforge/jnlp/util/HttpUtils.java
>> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>> +++ b/netx/net/sourceforge/jnlp/util/HttpUtils.java Tue Apr 30 16:12:59 2013 +0200
>> @@ -0,0 +1,71 @@
>> +/*
>> + Copyright (C) 2011 Red Hat, Inc.
>> +
>> + This file is part of IcedTea.
>> +
>> + IcedTea is free software; you can redistribute it and/or
>> + modify it under the terms of the GNU General Public License as published by
>> + the Free Software Foundation, version 2.
>> +
>> + IcedTea is distributed in the hope that it will be useful,
>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + General Public License for more details.
>> +
>> + You should have received a copy of the GNU General Public License
>> + along with IcedTea; see the file COPYING. If not, write to
>> + the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> + 02110-1301 USA.
>> +
>> + Linking this library statically or dynamically with other modules is
>> + making a combined work based on this library. Thus, the terms and
>> + conditions of the GNU General Public License cover the whole
>> + combination.
>> +
>> + As a special exception, the copyright holders of this library give you
>> + permission to link this library with independent modules to produce an
>> + executable, regardless of the license terms of these independent
>> + modules, and to copy and distribute the resulting executable under
>> + terms of your choice, provided that you also meet, for each linked
>> + independent module, the terms and conditions of the license of that
>> + module. An independent module is a module which is not derived from
>> + or based on this library. If you modify this library, you may extend
>> + this exception to your version of the library, but you are not
>> + obligated to do so. If you do not wish to do so, delete this
>> + exception statement from your version.
>> + */
>> +package net.sourceforge.jnlp.util;
>> +
>> +import java.io.IOException;
>> +import java.io.InputStream;
>> +import java.net.HttpURLConnection;
>> +
>> +public class HttpUtils {
>> +
>> + /**
>> + * Ensure a HttpURLConnection is fully read, required for correct behaviour
>> + * in some APIs, namely HttpURLConnection.
>> + */
>
> This message is confusing. Please drop the last part ('in some APIs, namely HttpURLConnection').
> Please duplicate this comment for 'consumeAndCloseConnection', and additionally indicate for
> 'consumeAndCloseConnection' that IOException's are caught and printed.
Done.
>
>> + public static void consumeAndCloseConnectionSilently(HttpURLConnection c) {
>> + try {
>> + consumeAndCloseConnection(c);
>> + } catch (IOException ex) {
>> + ex.printStackTrace(System.err);
>> + }
>> + }
>> +
>> + public static void consumeAndCloseConnection(HttpURLConnection c) throws IOException {
>> + InputStream in = null;
>> + try {
>> + in = c.getInputStream();
>> + byte[] throwAwayBuffer = new byte[256];
>> + while (in.read(throwAwayBuffer) > 0) {
>> + /* ignore contents */
>> + }
>> + } finally {
>> + if (in!=null){
>> + in.close();
>> + }
>> + }
>> + }
>> +}
>> diff -r e34db561b7b9 netx/net/sourceforge/jnlp/util/StreamUtils.java
>> --- a/netx/net/sourceforge/jnlp/util/StreamUtils.java Mon Apr 29 16:24:37 2013 +0200
>> +++ b/netx/net/sourceforge/jnlp/util/StreamUtils.java Tue Apr 30 16:12:59 2013 +0200
>> @@ -42,22 +42,10 @@
>> import java.io.IOException;
>> import java.io.InputStream;
>> import java.io.InputStreamReader;
>> +import java.net.HttpURLConnection;
>>
>> public class StreamUtils {
>>
>> - /**
>> - * Ensure a stream is fully read, required for correct behaviour in some
>> - * APIs, namely HttpURLConnection.
>> - * @throws IOException
>> - */
>> - public static void consumeAndCloseInputStream(InputStream in) throws IOException {
>> - byte[] throwAwayBuffer = new byte[256];
>> - while (in.read(throwAwayBuffer) > 0) {
>> - /* ignore contents */
>> - }
>> - in.close();
>> - }
>> -
>> /***
>> * Closes a stream, without throwing IOException.
>> * In case of IOException, prints the stack trace to System.err.
>
> Will review tests separately.
>
> Thanks,
> -Adam
Thanx for cehck. I hope Omair will be happy too:)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fixedPortalBank-fix.diff
Type: text/x-patch
Size: 9732 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130502/6947677f/fixedPortalBank-fix.diff
More information about the distro-pkg-dev
mailing list