[GEOS-8727] (WMS) Layer Preview does not properly encode layer identi…

…fiers

src/community/wfs3/src/main/java/org/geoserver/wfs3/response/AbstractHTMLResponse.java

@@ -19,14 +19,15 @@
 import org.geoserver.config.GeoServer;
 import org.geoserver.ows.Response;
 import org.geoserver.ows.URLMangler;
+import org.geoserver.ows.util.OwsUtils;
 import org.geoserver.ows.util.ResponseUtils;
 import org.geoserver.platform.GeoServerResourceLoader;
 import org.geoserver.platform.Operation;
 import org.geoserver.platform.ServiceException;
 import org.geoserver.template.TemplateUtils;
 import org.geoserver.wfs.WFSInfo;
 import org.geoserver.wfs3.BaseRequest;
-import org.geoserver.wfs3.GetFeatureType;
+import org.geotools.util.Converters;
 
 public abstract class AbstractHTMLResponse extends Response {
 
@@ -119,13 +120,11 @@ static void addServiceLinkFunctions(
 
     static String getBaseURL(Operation operation) {
         Object firstParam = operation.getParameters()[0];
-        if (firstParam instanceof BaseRequest) {
-            BaseRequest request = (BaseRequest) firstParam;
-            return request.getBaseUrl();
-        } else if (firstParam instanceof GetFeatureType) {
-            return ((GetFeatureType) firstParam).getBaseUrl();
+        String baseURL = Converters.convert(OwsUtils.get(firstParam, "baseUrl"), String.class);
+        if (baseURL == null) {
+            throw new IllegalArgumentException("Cannot extract base URL from " + firstParam);
         }
-        throw new IllegalArgumentException("Cannot extract base URL from " + firstParam);
+        return baseURL;
     }
 
     /**

src/extension/app-schema/app-schema-test/src/test/java/org/geoserver/test/web/Gml311LinksTest.java

@@ -15,12 +15,12 @@ public class Gml311LinksTest extends AbstractMapPreviewPageTest {
     public Gml311LinksTest() {
         super(
                 Arrays.asList(
-                        "http://localhost:80/context/gsml/ows?service=WFS&version=1.1.0&request=GetFeature&typeName=gsml:MappedFeature&outputFormat=gml3&maxFeatures=50",
-                        "http://localhost:80/context/gsml/ows?service=WFS&version=1.1.0&request=GetFeature&typeName=gsml:GeologicUnit&outputFormat=gml3&maxFeatures=50",
-                        "http://localhost:80/context/ex/ows?service=WFS&version=1.1.0&request=GetFeature&typeName=ex:FirstParentFeature&outputFormat=gml3&maxFeatures=50",
-                        "http://localhost:80/context/ex/ows?service=WFS&version=1.1.0&request=GetFeature&typeName=ex:SecondParentFeature&outputFormat=gml3&maxFeatures=50",
-                        "http://localhost:80/context/ex/ows?service=WFS&version=1.1.0&request=GetFeature&typeName=ex:ParentFeature&outputFormat=gml3&maxFeatures=50",
-                        "http://localhost:80/context/om/ows?service=WFS&version=1.1.0&request=GetFeature&typeName=om:Observation&outputFormat=gml3&maxFeatures=50"));
+                        "http://localhost:80/context/gsml/ows?service=WFS&version=1.1.0&request=GetFeature&typeName=gsml%3AMappedFeature&outputFormat=gml3&maxFeatures=50",
+                        "http://localhost:80/context/gsml/ows?service=WFS&version=1.1.0&request=GetFeature&typeName=gsml%3AGeologicUnit&outputFormat=gml3&maxFeatures=50",
+                        "http://localhost:80/context/ex/ows?service=WFS&version=1.1.0&request=GetFeature&typeName=ex%3AFirstParentFeature&outputFormat=gml3&maxFeatures=50",
+                        "http://localhost:80/context/ex/ows?service=WFS&version=1.1.0&request=GetFeature&typeName=ex%3ASecondParentFeature&outputFormat=gml3&maxFeatures=50",
+                        "http://localhost:80/context/ex/ows?service=WFS&version=1.1.0&request=GetFeature&typeName=ex%3AParentFeature&outputFormat=gml3&maxFeatures=50",
+                        "http://localhost:80/context/om/ows?service=WFS&version=1.1.0&request=GetFeature&typeName=om%3AObservation&outputFormat=gml3&maxFeatures=50"));
     }
 
     @Override

src/extension/app-schema/app-schema-test/src/test/java/org/geoserver/test/web/Gml32LinksTest.java

@@ -15,7 +15,7 @@ public class Gml32LinksTest extends AbstractMapPreviewPageTest {
     public Gml32LinksTest() {
         super(
                 Arrays.asList(
-                        "http://localhost:80/context/gsml/ows?service=WFS&version=1.1.0&request=GetFeature&typeName=gsml:MappedFeature&outputFormat=gml32&maxFeatures=50"));
+                        "http://localhost:80/context/gsml/ows?service=WFS&version=1.1.0&request=GetFeature&typeName=gsml%3AMappedFeature&outputFormat=gml32&maxFeatures=50"));
     }
 
     @Override

src/web/demo/src/main/java/org/geoserver/web/demo/MapPreviewPage.java

@@ -95,7 +95,7 @@ protected Component getComponentForProperty(
                             f.add(new ExternalLink("ol", olUrl, "OpenLayers"));
                             // kml preview
                             final String kmlUrl =
-                                    layer.getBaseUrl("wms") + "/kml?layers=" + layer.getName();
+                                    layer.getBaseURL("wms") + "/kml?layers=" + layer.getName();
                             f.add(new ExternalLink("kml", kmlUrl, "KML"));
                             // gml preview (we actually want it only for vector layers)
                             final String gmlUrl =
@@ -248,9 +248,7 @@ private Component buildJSWMSSelect(
                 "'" + layer.getWmsLink() + "&format=' + this.options[this.selectedIndex].value";
         String wfsUrl =
                 "'"
-                        + layer.getBaseUrl("ows")
-                        + "?service=WFS&version=1.0.0&request=GetFeature&typeName="
-                        + layer.getName()
+                        + layer.buildWfsLink()
                         + getMaxFeatures()
                         + "&outputFormat=' + this.options[this.selectedIndex].value";
         String choice =

src/web/demo/src/main/java/org/geoserver/web/demo/PreviewLayer.java

@@ -9,6 +9,7 @@
 import java.io.IOException;
 import java.io.Serializable;
 import java.util.ArrayList;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.logging.Level;
@@ -184,13 +185,12 @@ private List<MapLayerInfo> expandLayers(Catalog catalog) {
         return layers;
     }
 
-    String getBaseUrl(String service) {
-        return getBaseUrl(service, false);
+    String getBaseURL(String service) {
+        return getBaseURL(service, false);
     }
 
-    String getBaseUrl(String service, boolean useGlobalRef) {
-        HttpServletRequest req = GeoServerApplication.get().servletRequest();
-        String base = ResponseUtils.baseURL(req);
+    String getBaseURL(String service, boolean useGlobalRef) {
+        String base = getBaseURL();
 
         String ws = getWorkspace();
         if (ws == null || useGlobalRef) {
@@ -201,6 +201,21 @@ String getBaseUrl(String service, boolean useGlobalRef) {
         }
     }
 
+    private String getBaseURL() {
+        HttpServletRequest req = GeoServerApplication.get().servletRequest();
+        return ResponseUtils.baseURL(req);
+    }
+
+    String getPath(String service, boolean useGlobalRef) {
+        String ws = getWorkspace();
+        if (ws == null || useGlobalRef) {
+            // global reference
+            return service;
+        } else {
+            return ws + "/" + service;
+        }
+    }
+
     /**
      * Given a request and a target format, builds the WMS request
      *
@@ -212,25 +227,19 @@ public String getWmsLink() {
         final Envelope bbox = request.getBbox();
         if (bbox == null) return null;
 
-        return getBaseUrl("wms")
-                + "?service=WMS&version=1.1.0&request=GetMap" //
-                + "&layers="
-                + getName() //
-                + "&styles=" //
-                + "&bbox="
-                + bbox.getMinX()
-                + ","
-                + bbox.getMinY() //
-                + ","
-                + bbox.getMaxX()
-                + ","
-                + bbox.getMaxY() //
-                + "&width="
-                + request.getWidth() //
-                + "&height="
-                + request.getHeight()
-                + "&srs="
-                + request.getSRS();
+        Map<String, String> params = new LinkedHashMap<>();
+        params.put("service", "WMS");
+        params.put("version", "1.1.0");
+        params.put("request", "GetMap");
+        params.put("layers", getName());
+        String bboxValue =
+                bbox.getMinX() + "," + bbox.getMinY() + "," + bbox.getMaxX() + "," + bbox.getMaxY();
+        params.put("bbox", bboxValue);
+        params.put("width", String.valueOf(request.getWidth()));
+        params.put("height", String.valueOf(request.getHeight()));
+        params.put("srs", String.valueOf(request.getSRS()));
+
+        return ResponseUtils.buildURL(getBaseURL(), getPath("wms", false), params, URLType.SERVICE);
     }
 
     /**
@@ -258,7 +267,7 @@ public String getGmlLink(Map<String, GMLOutputParams> gmlParamsCache) {
                             } else {
                                 // use global OWS service to make sure all secondary namespaces
                                 // are accessible
-                                gmlParams.baseUrl = getBaseUrl("ows", false);
+                                gmlParams.baseUrl = getBaseURL();
                                 // always use WFS 1.1.0 for app-schema layers
                                 gmlParams.wfsVersion =
                                         org.geotools.wfs.v1_1.WFS.getInstance().getVersion();
@@ -287,7 +296,7 @@ public String getGmlLink(Map<String, GMLOutputParams> gmlParamsCache) {
             }
         }
 
-        return buildGmlLink(gmlParams);
+        return buildWfsLink(gmlParams);
     }
 
     /**
@@ -354,33 +363,36 @@ private boolean isAbstractFeatureType(AttributeType type) {
         }
     }
 
-    String buildGmlLink(GMLOutputParams gmlParams) {
-        StringBuilder urlBuilder = new StringBuilder();
-        urlBuilder.append(gmlParams.baseUrl).append("?");
-        urlBuilder.append("service=WFS").append("&");
-        urlBuilder.append("version=").append(gmlParams.wfsVersion).append("&");
-        urlBuilder.append("request=GetFeature").append("&");
-        urlBuilder.append("typeName=").append(getName());
+    String buildWfsLink() {
+        return this.buildWfsLink(new GMLOutputParams());
+    }
+
+    String buildWfsLink(GMLOutputParams gmlParams) {
+        Map<String, String> params = new LinkedHashMap<>();
+        params.put("service", "WFS");
+        params.put("version", gmlParams.wfsVersion);
+        params.put("request", "GetFeature");
+        params.put("typeName", getName());
         if (gmlParams.gmlVersion != null) {
-            urlBuilder.append("&");
-            urlBuilder.append("outputFormat=").append(gmlParams.gmlVersion);
+            params.put("outputFormat", gmlParams.gmlVersion);
         }
 
-        return urlBuilder.toString();
+        return ResponseUtils.buildURL(
+                gmlParams.baseUrl, getPath("ows", false), params, URLType.SERVICE);
     }
 
     class GMLOutputParams {
         String wfsVersion;
         String gmlVersion;
         String baseUrl;
 
-        private GMLOutputParams() {
+        public GMLOutputParams() {
             // by default, use WFS 1.0.0
             wfsVersion = org.geotools.wfs.v1_0.WFS.getInstance().getVersion();
             // by default, infer GML version from WFS version
             gmlVersion = null;
             // by default, use virtual ows services
-            baseUrl = getBaseUrl("ows");
+            baseUrl = getBaseURL();
         }
     }
 }

src/web/demo/src/test/java/org/geoserver/web/demo/MapPreviewPageTest.java

@@ -5,28 +5,42 @@
  */
 package org.geoserver.web.demo;
 
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 import java.util.Iterator;
 import org.apache.wicket.AttributeModifier;
 import org.apache.wicket.MarkupContainer;
 import org.apache.wicket.behavior.Behavior;
 import org.apache.wicket.markup.html.basic.Label;
+import org.apache.wicket.markup.html.link.ExternalLink;
 import org.apache.wicket.markup.repeater.data.DataView;
+import org.apache.wicket.util.tester.TagTester;
 import org.geoserver.catalog.Catalog;
 import org.geoserver.catalog.CatalogBuilder;
+import org.geoserver.catalog.FeatureTypeInfo;
 import org.geoserver.catalog.LayerGroupInfo;
 import org.geoserver.catalog.LayerInfo;
 import org.geoserver.config.GeoServer;
 import org.geoserver.data.test.MockData;
+import org.geoserver.web.GeoServerApplication;
 import org.geoserver.web.GeoServerWicketTestSupport;
 import org.geoserver.web.wicket.GeoServerTablePanel;
 import org.geoserver.wfs.WFSInfo;
+import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
 
 public class MapPreviewPageTest extends GeoServerWicketTestSupport {
+
+    @Before
+    public void setOutputPaths() {
+        GeoServerApplication.get().getDebugSettings().setComponentPathAttributeName("wicketPath");
+    }
+
     @Test
     public void testValues() throws Exception {
         tester.startPage(MapPreviewPage.class);
@@ -171,4 +185,65 @@ private void assertMaxFeaturesInData(DataView data, int maxFeatures) {
             }
         }
     }
+
+    @Test
+    public void testNameURLEncoding() {
+        Catalog catalog = getCatalog();
+        FeatureTypeInfo ft = catalog.getFeatureTypeByName("cite:Lakes");
+        ft.setName("Lakes + a plus");
+        catalog.save(ft);
+        try {
+
+            tester.startPage(MapPreviewPage.class);
+            tester.assertRenderedPage(MapPreviewPage.class);
+            // print(tester.getLastRenderedPage(), true, true, true);
+
+            DataView data =
+                    (DataView) tester.getComponentFromLastRenderedPage("table:listContainer:items");
+
+            boolean exists = false;
+            String path = null;
+            for (Iterator it = data.iterator(); it.hasNext(); ) {
+                MarkupContainer c = (MarkupContainer) it.next();
+                Label l = (Label) c.get("itemProperties:2:component");
+                String model = l.getDefaultModelObjectAsString();
+                if ("cite:Lakes + a plus".equals(model)) {
+                    exists = true;
+                    path = c.getPageRelativePath();
+
+                    // check visible links
+                    ExternalLink olLink = (ExternalLink) c.get("itemProperties:3:component:ol");
+                    ExternalLink gmlLink = (ExternalLink) c.get("itemProperties:3:component:gml");
+
+                    assertEquals(
+                            "http://localhost:80/context/cite/wms?service=WMS&version=1.1.0&request=GetMap&layers=cite%3ALakes%20%2B%20a%20plus&bbox=-180.0%2C-90.0%2C180.0%2C90.0&width=768&height=384&srs=EPSG%3A4326&format=application/openlayers",
+                            olLink.getDefaultModelObjectAsString());
+                    assertThat(
+                            gmlLink.getDefaultModelObjectAsString(),
+                            containsString(
+                                    "http://localhost:80/context/cite/ows?service=WFS&version=1.0.0&request=GetFeature&typeName=cite%3ALakes%20%2B%20a%20plus"));
+                }
+            }
+            assertTrue("Could not find layer with expected name", exists);
+
+            String html = tester.getLastResponseAsString();
+            TagTester menuTester =
+                    TagTester.createTagByAttribute(
+                            html,
+                            "wicketPath",
+                            path.replace(":", "_") + "_itemProperties_4_component_menu");
+            String onchange = menuTester.getAttribute("onchange");
+            assertThat(
+                    onchange,
+                    containsString(
+                            "http://localhost:80/context/cite/wms?service=WMS&version=1.1.0&request=GetMap&layers=cite%3ALakes%20%2B%20a%20plus&bbox=-180.0%2C-90.0%2C180.0%2C90.0&width=768&height=384&srs=EPSG%3A4326&format="));
+            assertThat(
+                    onchange,
+                    containsString(
+                            "http://localhost:80/context/cite/ows?service=WFS&version=1.0.0&request=GetFeature&typeName=cite%3ALakes%20%2B%20a%20plus"));
+        } finally {
+            ft.setName("Lines");
+            catalog.save(ft);
+        }
+    }
 }