[rfc] [icedtea-web] fix for PR811

[Pavel Tisnovsky]

Jiri Vanek wrote:
> On 05/16/2012 05:00 PM, Pavel Tisnovsky wrote:
>> Hi Jiri,
>>
>> just a quick shove:
>>
>> protocol.equals("file")
>>
>> should be "file".equals(protocol)
>>
> 
> Shame on me!!!-DDD
> 
> fixed patch attached. There was actually one more improvement needed.
> 
> So You are an voulnteer to review both fix and tests? Thanx a lot! But

well I'll try! Below are my comments:

> be very carefull. I'm afraid URL of resources is one of basic stones in
> netx. I have tried really a lot to figure what everything is affected by
> this change, and it looks like all. And there are dark corners in netx;)
> 
> 
> Thanx a lot again - J.
> 

diff -r 8c9b71e1db03 NEWS
--- a/NEWS	Fri May 11 21:33:54 2012 +0200
+++ b/NEWS	Wed May 16 18:29:32 2012 +0200
@@ -11,6 +11,7 @@
 New in release 1.3 (2012-XX-XX):
 * NetX
   - PR898: signed applications with big jnlp-file doesn't start (webstart affect like "frozen")
+  - PR811: javaws is not handling urls with spaces (and other characters needing encoding) correctly
 * Plugin
   - PR820: IcedTea-Web 1.1.3 crashing Firefox when loading Citrix XenApp
   - PR895: IcedTea-Web searches for missing classes on each loadClass or findClass
diff -r 8c9b71e1db03 netx/net/sourceforge/jnlp/cache/CacheUtil.java
--- a/netx/net/sourceforge/jnlp/cache/CacheUtil.java	Fri May 11 21:33:54 2012 +0200
+++ b/netx/net/sourceforge/jnlp/cache/CacheUtil.java	Wed May 16 18:29:32 2012 +0200
@@ -66,17 +66,29 @@
         if (u1 == null || u2 == null)
             return false;

-        if (!compare(u1.getProtocol(), u2.getProtocol(), true) ||
-                !compare(u1.getHost(), u2.getHost(), true) ||
-                //u1.getDefaultPort() != u2.getDefaultPort() || // only in 1.4
-                !compare(u1.getPath(), u2.getPath(), false) ||
-                !compare(u1.getQuery(), u2.getQuery(), false) ||
-                !compare(u1.getRef(), u2.getRef(), false))
-            return false;
-        else
-            return true;
+       if (notNullUrlEquals(u1, u2)) return true;
+        try{
+       URL nu1=ResourceTracker.normalizeUrl(u1, false);
+       URL nu2=ResourceTracker.normalizeUrl(u2, false);
+       if (notNullUrlEquals(nu1, nu2)) return true;

Looks like there's a problem with:
1) indentation (in the try block)
2) spaces should be used around = assignment


+        }catch(Exception ex){

dtto spaces

+            //keep sielnt here and return false
+        }
+       return false;
     }

+    private static boolean notNullUrlEquals(URL u1, URL u2) {
+        if (!compare(u1.getProtocol(), u2.getProtocol(), true)
+                || !compare(u1.getHost(), u2.getHost(), true)
+                || //u1.getDefaultPort() != u2.getDefaultPort() || // only in 1.4
+                !compare(u1.getPath(), u2.getPath(), false)
+                || !compare(u1.getQuery(), u2.getQuery(), false)
+                || !compare(u1.getRef(), u2.getRef(), false)) {
+            return false;
+        } else {
+            return true;
+        }
+    }
     /**
      * Caches a resource and returns a URL for it in the cache;
      * blocks until resource is cached.  If the resource location is
diff -r 8c9b71e1db03 netx/net/sourceforge/jnlp/cache/ResourceTracker.java
--- a/netx/net/sourceforge/jnlp/cache/ResourceTracker.java	Fri May 11 21:33:54 2012 +0200
+++ b/netx/net/sourceforge/jnlp/cache/ResourceTracker.java	Wed May 16 18:29:32 2012 +0200
@@ -24,10 +24,13 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.io.UnsupportedEncodingException;
 import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLConnection;
+import java.net.URLDecoder;
+import java.net.URLEncoder;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.ArrayList;
@@ -173,7 +176,12 @@
     public void addResource(URL location, Version version, DownloadOptions options, UpdatePolicy updatePolicy) {
         if (location == null)
             throw new IllegalArgumentException("location==null");
-
+        try{
+        location=normalizeUrl(location, JNLPRuntime.isDebug());

dtto ;-)

+        }catch(Exception ex){
+            System.err.println("Normalization of "+location.toString()+" have failed");
+            ex.printStackTrace();
+        }
         Resource resource = Resource.getResource(location, version, updatePolicy);
         boolean downloaded = false;

@@ -1127,4 +1135,71 @@
         }
     };

+     public static URL normalizeUrl(URL u, boolean debug) throws MalformedURLException, UnsupportedEncodingException {
+        if (u == null) {
+            return null;
+        }
+        String protocol = u.getProtocol();
+        if (protocol==null || "file".equals(protocol)) {
+            return u;
+        }
+        String host = u.getHost();
+        String query = u.getQuery();
+        String ref = u.getRef();
+        int port = u.getPort();
+        String file = u.getPath();

I'm not sure if it could or could not be null, in this case NPE should occured on following lines

+        String[] ss = file.split("/");
+        int normalized = 0;
+        if (debug) {
+            System.out.println("normalizng " + file + " in " + u.toString());

normalizng -> normalizing

+        }
+        for (int i = 0; i < ss.length; i++) {
+            String base = ss[i];
+            String ssE = URLDecoder.decode(base, "utf-8");
+//            System.out.println("*" + base + "*");
+//            System.out.println("-" + ssE + "-");
+            if (base.equals(ssE)) {
+                ss[i] = URLEncoder.encode(base, "utf-8");
+                if (!ss[i].equals(base)) {
+                    normalized++;
+                }
+                if (debug) {
+                    System.out.println(base + " chunk Needs to be encoded => " + ss[i]);

Needs->lowercase

+                }
+                ss[i] = URLEncoder.encode(base, "utf-8");
+            } else {
+                if (debug) {
+                    System.out.println(base + " chunk already encoded");
+                }
+            }
+
+        }
+        if (normalized == 0) {
+            if (debug) {
+                System.out.println("Nothing was normalized in this url");
+            }
+            return u;
+        }
+        StringBuilder composed = new StringBuilder("");

I'm not 100% sure about following code. At least it is IMHO better to
use characters instead of one-characters strings and refactor these
magical constants into final fields - PATH_SEPARATOR, QUERY_SEPARATOR etc. etc.

Btw is it really working for all characters - ie. the ones outside of ASCII?

+        for (int i = 0; i < ss.length; i++) {
+            String string = ss[i];
+            composed.append("/").append(string);
+        }
+        String composed1 = composed.toString();
+        if (query != null && !query.trim().equals("")) {
+            composed.append("?").append(query);
+        }
+        if (ref != null && !ref.trim().equals("")) {
+            composed.append("#").append(ref);
+        }
+
+        URL result = new URL(protocol, host, port, composed.toString());
+
+        if (debug) {
+            System.out.println("normalized " + composed1 + " in " + result.toString());
+        }
+        return result;
+
+    }
+
 }