Ticket #13118: 13118.patch

src/org/openstreetmap/josm/actions/AddImageryLayerAction.java

@@ -10 +10 @@
 import java.awt.event.ActionEvent;
 import java.io.IOException;
 import java.net.MalformedURLException;
+import java.net.URISyntaxException;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -131 +132 @@
             }
             ret.setServerProjections(supportedCrs);
             return ret;
-        } catch (MalformedURLException ex) {
+        } catch (MalformedURLException | URISyntaxException ex) {
             if (!GraphicsEnvironment.isHeadless()) {
                 JOptionPane.showMessageDialog(Main.parent, tr("Invalid service URL."),
                         tr("WMS Error"), JOptionPane.ERROR_MESSAGE);

src/org/openstreetmap/josm/gui/preferences/imagery/AddWMSLayerPanel.java

@@ -5 +5 @@
 
 import java.io.IOException;
 import java.net.MalformedURLException;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.List;
 
@@ -76 +77 @@
                 List<String> wmsFormats = wms.getFormats();
                 formats.setModel(new DefaultComboBoxModel<>(wmsFormats.toArray(new String[wmsFormats.size()])));
                 formats.setSelectedItem(wms.getPreferredFormats());
-            } catch (MalformedURLException ex1) {
+            } catch (MalformedURLException | URISyntaxException ex1) {
                 Main.error(ex1, false);
                 JOptionPane.showMessageDialog(getParent(), tr("Invalid service URL."),
                         tr("WMS Error"), JOptionPane.ERROR_MESSAGE);

src/org/openstreetmap/josm/io/imagery/WMSImagery.java

@@ -5 +5 @@
 import java.io.IOException;
 import java.io.StringReader;
 import java.net.MalformedURLException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -13 +15 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Set;
-import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
@@ -29 +31 @@
 import org.openstreetmap.josm.data.imagery.ImageryInfo;
 import org.openstreetmap.josm.data.projection.Projections;
 import org.openstreetmap.josm.tools.HttpClient;
+import org.openstreetmap.josm.tools.HttpUrl;
 import org.openstreetmap.josm.tools.Utils;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
@@ -110 +113 @@
     }
 
     private List<LayerDetails> layers;
-    private URL serviceUrl;
+    private HttpUrl serviceUrl;
     private List<String> formats;
 
     /**
@@ -126 +129 @@
      * @return the service URL
      */
     public URL getServiceUrl() {
-        return serviceUrl;
+        try {
+            return serviceUrl == null ? null : serviceUrl.toURL();
+        } catch (MalformedURLException e) {
+            Main.warn(e);
+            return null;
+        }
     }
 
     /**
@@ -138 +146 @@
     }
 
     /**
-     * Gets the preffered format for this imagery layer.
-     * @return The preffered format as mime type.
+     * Gets the preferred format for this imagery layer.
+     * @return The preferred format as mime type.
      */
     public String getPreferredFormats() {
         if (formats.contains("image/jpeg")) {
@@ -153 +161 @@
         }
     }
 
-    String buildRootUrl() {
-        if (serviceUrl == null) {
-            return null;
-        }
-        StringBuilder a = new StringBuilder(serviceUrl.getProtocol());
-        a.append("://").append(serviceUrl.getHost());
-        if (serviceUrl.getPort() != -1) {
-            a.append(':').append(serviceUrl.getPort());
-        }
-        a.append(serviceUrl.getPath()).append('?');
-        if (serviceUrl.getQuery() != null) {
-            a.append(serviceUrl.getQuery());
-            if (!serviceUrl.getQuery().isEmpty() && !serviceUrl.getQuery().endsWith("&")) {
-                a.append('&');
-            }
-        }
-        return a.toString();
-    }
-
     public String buildGetMapUrl(Collection<LayerDetails> selectedLayers) {
         return buildGetMapUrl(selectedLayers, "image/jpeg");
     }
 
     public String buildGetMapUrl(Collection<LayerDetails> selectedLayers, String format) {
-        return buildRootUrl() + "FORMAT=" + format + (imageFormatHasTransparency(format) ? "&TRANSPARENT=TRUE" : "")
-                + "&VERSION=1.1.1&SERVICE=WMS&REQUEST=GetMap&LAYERS="
-                + selectedLayers.stream().map(x -> x.ident).collect(Collectors.joining(","))
+        serviceUrl.putQueryParameter("REQUEST", "GetMap");
+        serviceUrl.putQueryParameter("FORMAT", format);
+        serviceUrl.putQueryParameter("TRANSPARENT", imageFormatHasTransparency(format) ? "TRUE" : "");
+        serviceUrl.putQueryParameter("LAYERS", selectedLayers.stream().map(x -> x.ident).collect(Collectors.joining(",")));
+        return serviceUrl.toString()
                 + "&STYLES=&SRS={proj}&WIDTH={width}&HEIGHT={height}&BBOX={bbox}";
     }
 
-    public void attemptGetCapabilities(String serviceUrlStr) throws IOException, WMSGetCapabilitiesException {
-        URL getCapabilitiesUrl = null;
+    public void attemptGetCapabilities(final String serviceUrlStr) throws IOException, WMSGetCapabilitiesException, URISyntaxException {
         try {
-            if (!Pattern.compile(".*GetCapabilities.*", Pattern.CASE_INSENSITIVE).matcher(serviceUrlStr).matches()) {
+            final URL url = new URL(serviceUrlStr); // causing MalformedURLException on invalid input
+            final HttpUrl getCapabilitiesUrl = new HttpUrl(url.toURI(), false);
+            if (!"GetCapabilities".equals(getCapabilitiesUrl.getQueryParameter("REQUEST"))) {
                 // If the url doesn't already have GetCapabilities, add it in
-                getCapabilitiesUrl = new URL(serviceUrlStr);
-                final String getCapabilitiesQuery = "VERSION=1.1.1&SERVICE=WMS&REQUEST=GetCapabilities";
-                if (getCapabilitiesUrl.getQuery() == null) {
-                    getCapabilitiesUrl = new URL(serviceUrlStr + '?' + getCapabilitiesQuery);
-                } else if (!getCapabilitiesUrl.getQuery().isEmpty() && !getCapabilitiesUrl.getQuery().endsWith("&")) {
-                    getCapabilitiesUrl = new URL(serviceUrlStr + '&' + getCapabilitiesQuery);
-                } else {
-                    getCapabilitiesUrl = new URL(serviceUrlStr + getCapabilitiesQuery);
+                getCapabilitiesUrl.putQueryParameter("REQUEST", "GetCapabilities");
+                if (!getCapabilitiesUrl.containsQueryParameter("SERVICE")) {
+                    getCapabilitiesUrl.putQueryParameter("SERVICE", "WMS");
+                }
+                if (!getCapabilitiesUrl.containsQueryParameter("VERSION")) {
+                    getCapabilitiesUrl.putQueryParameter("VERSION", "1.1.1");
                 }
             } else {
                 // Otherwise assume it's a good URL and let the subsequent error
                 // handling systems deal with problems
-                getCapabilitiesUrl = new URL(serviceUrlStr);
             }
-            serviceUrl = new URL(serviceUrlStr);
+            serviceUrl = getCapabilitiesUrl;
         } catch (HeadlessException e) {
             Main.warn(e);
             return;
         }
 
-        final String incomingData = HttpClient.create(getCapabilitiesUrl).connect().fetchContent();
+        final String incomingData = HttpClient.create(serviceUrl.toURL()).connect().fetchContent();
         Main.debug("Server response to Capabilities request:");
         Main.debug(incomingData);
 
@@ -244 +233 @@
                 String baseURL = child.getAttribute("xlink:href");
                 if (baseURL != null && !baseURL.equals(serviceUrlStr)) {
                     Main.info("GetCapabilities specifies a different service URL: " + baseURL);
-                    serviceUrl = new URL(baseURL);
+                    final Map<String, String> parameters = serviceUrl.getQueryParameters();
+                    serviceUrl = new HttpUrl(URI.create(baseURL), false);
+                    serviceUrl.putQueryParameters(parameters);
                 }
             }
 

src/org/openstreetmap/josm/io/remotecontrol/handler/RequestHandler.java

@@ -19 +19 @@
 
 import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.io.remotecontrol.PermissionPrefWithDefault;
+import org.openstreetmap.josm.tools.HttpUrl;
 import org.openstreetmap.josm.tools.Utils;
 
 /**
@@ -195 +196 @@
      * @throws URISyntaxException if request URL is invalid
      */
     protected void parseArgs() throws URISyntaxException {
-        this.args = getRequestParameter(new URI(this.request));
-    }
-
-    /**
-     * Returns the request parameters.
-     * @param uri URI as string
-     * @return map of request parameters
-     * @see <a href="http://blog.lunatech.com/2009/02/03/what-every-web-developer-must-know-about-url-encoding">
-     *      What every web developer must know about URL encoding</a>
-     */
-    static Map<String, String> getRequestParameter(URI uri) {
-        Map<String, String> r = new HashMap<>();
-        if (uri.getRawQuery() == null) {
-            return r;
-        }
-        for (String kv : uri.getRawQuery().split("&")) {
-            final String[] kvs = Utils.decodeUrl(kv).split("=", 2);
-            r.put(kvs[0], kvs.length > 1 ? kvs[1] : null);
-        }
-        return r;
+        this.args = HttpUrl.getQueryParameter(new URI(this.request), true);
     }
 
     void checkMandatoryParams() throws RequestHandlerBadRequestException {

new file src/org/openstreetmap/josm/tools/HttpUrl.java

@@ +1 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.tools;
+
+import java.net.MalformedURLException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.stream.Collectors;
+
+/**
+ * This class allows building HTTP URLs with query parameters.
+ * @since 11059
+ */
+public class HttpUrl {
+
+    private final URI uri;
+    private final Map<String, String> query;
+
+    /**
+     * Creates a new instance from the specified URI
+     * @param uri the URI
+     * @param caseSensitive whether query parameters are considered case sensitive
+     */
+    public HttpUrl(URI uri, boolean caseSensitive) {
+        this.uri = uri;
+        this.query = getQueryParameter(uri, caseSensitive);
+    }
+
+    /**
+     * Returns the request query parameter.
+     * @param uri the URI
+     * @return map of request parameters
+     * @see <a href="https://www.talisman.org/~erlkonig/misc/lunatech%5ewhat-every-webdev-must-know-about-url-encoding/">
+     *      What every web developer must know about URL encoding</a>
+     */
+    public static Map<String, String> getQueryParameter(URI uri, boolean caseSensitive) {
+        Map<String, String> r = new TreeMap<>(caseSensitive ? Comparator.<String>naturalOrder() : String.CASE_INSENSITIVE_ORDER);
+        if (uri.getRawQuery() == null) {
+            return r;
+        }
+        for (String kv : uri.getRawQuery().split("&")) {
+            if (kv.isEmpty()) {
+                continue;
+            }
+            final String[] kvs = Utils.decodeUrl(kv).split("=", 2);
+            r.put(kvs[0], kvs.length > 1 ? kvs[1] : null);
+        }
+        return r;
+    }
+
+    /**
+     * Tests whether the query parameter {@code key} is present
+     * @param key the query parameter
+     * @return true if the query parameter {@code key} is present
+     */
+    public boolean containsQueryParameter(String key) {
+        return query.containsKey(key);
+    }
+
+    /**
+     * Returns the value for the query parameter {@code key}
+     * @param key the query parameter
+     * @return the value for the query parameter
+     */
+    public String getQueryParameter(String key) {
+        return query.get(key);
+    }
+
+    /**
+     * Returns a map of all query parameters
+     * @return a map of all query parameters
+     */
+    public Map<String, String> getQueryParameters() {
+        return Collections.unmodifiableMap(query);
+    }
+
+    /**
+     * Sets the the value for the query parameter {@code key}
+     * @param key the query parameter
+     * @param value the value for the query parameter
+     * @return the old value for the query parameter, or {@code null}
+     */
+    public String putQueryParameter(String key, String value) {
+        return query.put(key, value);
+    }
+
+    /**
+     * Sets all query parameters
+     * @param parameters the query parameters
+     */
+    public void putQueryParameters(Map<? extends String, ? extends String> parameters) {
+        query.putAll(parameters);
+    }
+
+    /**
+     * Constructs a URI from this.
+     * @return a URI
+     */
+    public URI toURI() {
+        final String query = this.query.entrySet().stream()
+                .map(entry -> entry.getKey() + "=" + entry.getValue())
+                .collect(Collectors.joining("&"));
+        try {
+            return new URI(uri.getScheme(), uri.getUserInfo(), uri.getHost(), uri.getPort(), uri.getPath(), query, uri.getFragment());
+        } catch (URISyntaxException ex) {
+            throw new RuntimeException(ex);
+        }
+    }
+
+    /**
+     * Constructs a URL from this.
+     * @return a URL
+     * @throws MalformedURLException If a protocol handler for the URL could not be found, or if some other error occurred while constructing the URL
+     */
+    public URL toURL() throws MalformedURLException {
+        return toURI().toURL();
+    }
+
+    @Override
+    public String toString() {
+        return toURI().toString();
+    }
+}

.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
-package org.openstreetmap.josm.io.remotecontrol.handler;
+package org.openstreetmap.josm.tools;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
+import java.net.URI;
+import java.net.URL;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
 import org.junit.Test;
-import org.openstreetmap.josm.io.remotecontrol.PermissionPrefWithDefault;
-import org.openstreetmap.josm.io.remotecontrol.handler.RequestHandler.RequestHandlerBadRequestException;
 
 /**
- * Unit tests of {@link RequestHandler} class.
+ * Unit tests of {@link HttpUrl} class.
  */
-public class RequestHandlerTest {
+public class HttpUrlTest {
 
-    Map<String, String> getRequestParameter(String url) throws RequestHandlerBadRequestException {
-        final RequestHandler req = new RequestHandler() {
-            @Override
-            protected void validateRequest() throws RequestHandlerBadRequestException {
-            }
-
-            @Override
-            protected void handleRequest() throws RequestHandlerErrorException, RequestHandlerBadRequestException {
-            }
-
-            @Override
-            public String getPermissionMessage() {
-                return null;
-            }
-
-            @Override
-            public PermissionPrefWithDefault getPermissionPref() {
-                return null;
-            }
+    private static Map<String, String> getRequestParameter(String url) {
+        return HttpUrl.getQueryParameter(URI.create(url), true);
+    }
 
-            @Override
-            public String[] getMandatoryParams() {
-                return new String[0];
-            }
-        };
-        req.setUrl(url);
-        return req.args;
+    /**
+     * Test request parameter - case 1
+     */
+    @Test
+    public void testRequestParameter0() {
+        final Map<String, String> expected = new HashMap<>();
+        assertEquals(expected, getRequestParameter("http://example.com/"));
+        assertEquals(expected, getRequestParameter("http://example.com/?"));
+        assertEquals(expected, getRequestParameter("http://example.com/#foobar"));
     }
 
     /**
      * Test request parameter - case 1
-     * @throws RequestHandlerBadRequestException never
      */
     @Test
-    public void testRequestParameter1() throws RequestHandlerBadRequestException {
+    public void testRequestParameter1() {
         final Map<String, String> expected = new HashMap<>();
         expected.put("query", "a");
         expected.put("b", "=c");
@@ -59 +45 @@
 
     /**
      * Test request parameter - case 2
-     * @throws RequestHandlerBadRequestException never
      */
     @Test
-    public void testRequestParameter2() throws RequestHandlerBadRequestException {
+    public void testRequestParameter2() {
         assertEquals(Collections.singletonMap("query", "a&b==c"),
                 getRequestParameter("http://example.com/?query=a%26b==c"));
     }
 
     /**
      * Test request parameter - case 3
-     * @throws RequestHandlerBadRequestException never
      */
     @Test
-    public void testRequestParameter3() throws RequestHandlerBadRequestException {
+    public void testRequestParameter3() {
         assertEquals(Collections.singleton("blue+light blue"),
                 getRequestParameter("http://example.com/blue+light%20blue?blue%2Blight+blue").keySet());
     }
 
     /**
      * Test request parameter - case 4
-     * @see <a href="http://blog.lunatech.com/2009/02/03/what-every-web-developer-must-know-about-url-encoding">
+     * @see <a href="https://www.talisman.org/~erlkonig/misc/lunatech%5ewhat-every-webdev-must-know-about-url-encoding/">
      *      What every web developer must know about URL encoding</a>
-     * @throws RequestHandlerBadRequestException never
      */
     @Test
-    public void testRequestParameter4() throws RequestHandlerBadRequestException {
+    public void testRequestParameter4() {
         assertEquals(Collections.singletonMap("/?:@-._~!$'()* ,;", "/?:@-._~!$'()* ,;=="), getRequestParameter(
                 // CHECKSTYLE.OFF: LineLength
                 "http://example.com/:@-._~!$&'()*+,=;:@-._~!$&'()*+,=:@-._~!$&'()*+,==?/?:@-._~!$'()*+,;=/?:@-._~!$'()*+,;==#/?:@-._~!$&'()*+,;="));
@@ -93 +76 @@
 
     /**
      * Test request parameter - case 5
-     * @throws RequestHandlerBadRequestException never
      */
     @Test
-    public void testRequestParameter5() throws RequestHandlerBadRequestException {
+    public void testRequestParameter5() {
         final Map<String, String> expected = new HashMap<>();
         expected.put("space", " ");
         expected.put("tab", "\t");
@@ -105 +87 @@
 
     /**
      * Test request parameter - case 6
-     * @throws RequestHandlerBadRequestException never
      */
     @Test
-    public void testRequestParameter6() throws RequestHandlerBadRequestException {
+    public void testRequestParameter6() {
         final Map<String, String> expected = new HashMap<>();
         expected.put("addtags", "wikipedia:de=Weiße_Gasse|maxspeed=5");
         expected.put("select", "way23071688,way23076176,way23076177,");
@@ -124 +105 @@
 
     /**
      * Test request parameter - invalid case
-     * @throws RequestHandlerBadRequestException always
      */
-    @Test(expected = RequestHandlerBadRequestException.class)
-    public void testRequestParameterInvalid() throws RequestHandlerBadRequestException {
+    @Test(expected = IllegalArgumentException.class)
+    public void testRequestParameterInvalid() {
         getRequestParameter("http://localhost:8111/load_and_zoom"+
                 "?addtags=wikipedia:de=Wei%C3%9Fe_Gasse|maxspeed=5"+
                 "&select=way23071688,way23076176,way23076177,"+
                 "&left=13.739727546842&right=13.740890970188&top=51.049987191025&bottom=51.048466954325");
     }
+
+    /**
+     * Test request parameter changing
+     */
+    @Test
+    public void testSettingQueryParameter() throws Exception {
+        final HttpUrl url = new HttpUrl(URI.create("https://www.wms.com/guest?VERSION=1.3&SERVICE=WMS&REQUEST=GetCapabilities"), true);
+        assertEquals("https://www.wms.com/guest?REQUEST=GetCapabilities&SERVICE=WMS&VERSION=1.3", url.toString());
+        assertTrue(url.containsQueryParameter("REQUEST"));
+        assertEquals("GetCapabilities", url.getQueryParameter("REQUEST"));
+        url.putQueryParameter("REQUEST", "GetMap");
+        assertEquals("https://www.wms.com/guest?REQUEST=GetMap&SERVICE=WMS&VERSION=1.3", url.toString());
+        assertEquals("{REQUEST=GetMap, SERVICE=WMS, VERSION=1.3}", url.getQueryParameters().toString());
+    }
+
+    @Test
+    public void testUrlUnchanged() throws Exception {
+        final URL url1 = new URL("http://example.com/:@-._~!$&'()*+,=;:@-._~!$&'()*+,=:@-._~!$&'()*+,==?/?" +
+                ":@-._~!$'()*%20,;=/?:@-._~!$'()*%20,;==#/?:@-._~!$&'()*+,;=");
+        final URL uri2 = new HttpUrl(url1.toURI(), true).toURL();
+        assertEquals(url1, uri2);
+    }
 }