commit 609bb4918526fa89ac5ddc86e5a2f205cbd5a488 Author: Simon Legner Date: Tue Sep 27 15:39:14 2016 +0200 see #13118 - WMSImagery: retain specified version for URL generation diff --git a/src/org/openstreetmap/josm/actions/AddImageryLayerAction.java b/src/org/openstreetmap/josm/actions/AddImageryLayerAction.java index 32eb835..5a46619 100644 --- a/src/org/openstreetmap/josm/actions/AddImageryLayerAction.java +++ b/src/org/openstreetmap/josm/actions/AddImageryLayerAction.java @@ -10,6 +10,7 @@ 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,7 +132,7 @@ protected ImageryInfo getWMSLayerInfo() { } 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); diff --git a/src/org/openstreetmap/josm/gui/preferences/imagery/AddWMSLayerPanel.java b/src/org/openstreetmap/josm/gui/preferences/imagery/AddWMSLayerPanel.java index 049b36e..63b39a8 100644 --- a/src/org/openstreetmap/josm/gui/preferences/imagery/AddWMSLayerPanel.java +++ b/src/org/openstreetmap/josm/gui/preferences/imagery/AddWMSLayerPanel.java @@ -5,6 +5,7 @@ import java.io.IOException; import java.net.MalformedURLException; +import java.net.URISyntaxException; import java.net.URL; import java.util.List; @@ -76,7 +77,7 @@ public AddWMSLayerPanel() { List 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); diff --git a/src/org/openstreetmap/josm/io/imagery/WMSImagery.java b/src/org/openstreetmap/josm/io/imagery/WMSImagery.java index b21afa3..f9fca14 100644 --- a/src/org/openstreetmap/josm/io/imagery/WMSImagery.java +++ b/src/org/openstreetmap/josm/io/imagery/WMSImagery.java @@ -5,6 +5,8 @@ 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,9 +15,9 @@ 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,6 +31,7 @@ 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,7 +113,7 @@ public String getIncomingData() { } private List layers; - private URL serviceUrl; + private HttpUrl serviceUrl; private List formats; /** @@ -126,7 +129,12 @@ public String getIncomingData() { * @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,8 +146,8 @@ public URL getServiceUrl() { } /** - * 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,62 +161,43 @@ public String getPreferredFormats() { } } - 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 selectedLayers) { return buildGetMapUrl(selectedLayers, "image/jpeg"); } public String buildGetMapUrl(Collection 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,7 +233,9 @@ public void attemptGetCapabilities(String serviceUrlStr) throws IOException, WMS 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 parameters = serviceUrl.getQueryParameters(); + serviceUrl = new HttpUrl(URI.create(baseURL), false); + serviceUrl.putQueryParameters(parameters); } } diff --git a/src/org/openstreetmap/josm/io/remotecontrol/handler/RequestHandler.java b/src/org/openstreetmap/josm/io/remotecontrol/handler/RequestHandler.java index cd83e6d..afd8f17 100644 --- a/src/org/openstreetmap/josm/io/remotecontrol/handler/RequestHandler.java +++ b/src/org/openstreetmap/josm/io/remotecontrol/handler/RequestHandler.java @@ -19,6 +19,7 @@ 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,26 +196,7 @@ public void setUrl(String url) throws RequestHandlerBadRequestException { * @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 - * What every web developer must know about URL encoding - */ - static Map getRequestParameter(URI uri) { - Map 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 { diff --git a/src/org/openstreetmap/josm/tools/HttpUrl.java b/src/org/openstreetmap/josm/tools/HttpUrl.java new file mode 100644 index 0000000..ec11531 --- /dev/null +++ b/src/org/openstreetmap/josm/tools/HttpUrl.java @@ -0,0 +1,127 @@ +// 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 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 + * What every web developer must know about URL encoding + */ + public static Map getQueryParameter(URI uri, boolean caseSensitive) { + Map r = new TreeMap<>(caseSensitive ? Comparator.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 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 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(); + } +} diff --git a/test/unit/org/openstreetmap/josm/io/remotecontrol/handler/RequestHandlerTest.java b/test/unit/org/openstreetmap/josm/tools/HttpUrlTest.java similarity index 55% rename from test/unit/org/openstreetmap/josm/io/remotecontrol/handler/RequestHandlerTest.java rename to test/unit/org/openstreetmap/josm/tools/HttpUrlTest.java index eb342d6..f121a09 100644 --- a/test/unit/org/openstreetmap/josm/io/remotecontrol/handler/RequestHandlerTest.java +++ b/test/unit/org/openstreetmap/josm/tools/HttpUrlTest.java @@ -1,56 +1,42 @@ // 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 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 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 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 expected = new HashMap<>(); expected.put("query", "a"); expected.put("b", "=c"); @@ -59,32 +45,29 @@ public void testRequestParameter1() throws RequestHandlerBadRequestException { /** * 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 + * @see * What every web developer must know about URL encoding - * @throws RequestHandlerBadRequestException never */ @Test - public void testRequestParameter4() throws RequestHandlerBadRequestException { + public void testRequestParameter4() { assertEquals(Collections.singletonMap("/?:@-._~!$'()* ,;", "/?:@-._~!$'()* ,;=="), getRequestParameter( // CHECKSTYLE.OFF: LineLength "http://example.com/:@-._~!$&'()*+,=;:@-._~!$&'()*+,=:@-._~!$&'()*+,==?/?:@-._~!$'()*+,;=/?:@-._~!$'()*+,;==#/?:@-._~!$&'()*+,;=")); @@ -93,10 +76,9 @@ public void testRequestParameter4() throws RequestHandlerBadRequestException { /** * Test request parameter - case 5 - * @throws RequestHandlerBadRequestException never */ @Test - public void testRequestParameter5() throws RequestHandlerBadRequestException { + public void testRequestParameter5() { final Map expected = new HashMap<>(); expected.put("space", " "); expected.put("tab", "\t"); @@ -105,10 +87,9 @@ public void testRequestParameter5() throws RequestHandlerBadRequestException { /** * Test request parameter - case 6 - * @throws RequestHandlerBadRequestException never */ @Test - public void testRequestParameter6() throws RequestHandlerBadRequestException { + public void testRequestParameter6() { final Map expected = new HashMap<>(); expected.put("addtags", "wikipedia:de=Weiße_Gasse|maxspeed=5"); expected.put("select", "way23071688,way23076176,way23076177,"); @@ -124,13 +105,34 @@ public void testRequestParameter6() throws RequestHandlerBadRequestException { /** * 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); + } }