diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java index eec8bf37e9..1678fd9a61 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java @@ -351,6 +351,7 @@ protected void parseNameAndNamespace(String uri, ActionMapping mapping, Configur Configuration config = configManager.getConfiguration(); String prefix = uri.substring(0, lastSlash); namespace = ""; + boolean rootAvailable = false; // Find the longest matching namespace, defaulting to the default for (Object o : config.getPackageConfigs().values()) { String ns = ((PackageConfig) o).getNamespace(); @@ -359,9 +360,19 @@ protected void parseNameAndNamespace(String uri, ActionMapping mapping, Configur namespace = ns; } } + if ("/".equals(ns)) { + rootAvailable = true; + } } + // must be read before the root namespace is selected below, as it is relative to "" name = uri.substring(namespace.length() + 1); + + // WW-5688, WW-2461: still none found, use the root namespace if it is declared, so that + // an id-bearing uri lands in the same namespace as the one without an id + if (rootAvailable && namespace.isEmpty()) { + namespace = "/"; + } } mapping.setNamespace(cleanupNamespaceName(namespace)); diff --git a/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperRootNamespaceTest.java b/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperRootNamespaceTest.java new file mode 100644 index 0000000000..b722b4d6cd --- /dev/null +++ b/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperRootNamespaceTest.java @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.rest; + +import com.opensymphony.xwork2.XWorkTestCase; +import org.apache.struts2.config.StrutsXmlConfigurationProvider; +import org.apache.struts2.dispatcher.mapper.ActionMapping; +import org.springframework.mock.web.MockHttpServletRequest; + +/** + * WW-5688: an action declared in the root namespace has to stay reachable whether or not the + * request URI carries an id, so that {@code index} and {@code show} resolve to the same action. + * + *

These assertions go all the way to the {@code ActionConfig} rather than stopping at the + * mapping, because the reported symptom is a 404 - the mapper handing back a namespace that + * the configuration cannot resolve.

+ */ +public class RestActionMapperRootNamespaceTest extends XWorkTestCase { + + private RestActionMapper mapper; + + @Override + protected void setUp() throws Exception { + super.setUp(); + loadConfigurationProviders(new StrutsXmlConfigurationProvider("ww-5688.xml")); + mapper = new RestActionMapper(); + } + + private ActionMapping map(String servletPath, String httpMethod) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setContextPath("/myapp"); + request.setMethod(httpMethod); + request.setRequestURI("/myapp" + servletPath); + request.setServletPath(servletPath); + return mapper.getMapping(request, configurationManager); + } + + private void assertResolves(String servletPath, String httpMethod, String expectedMethod) { + ActionMapping mapping = map(servletPath, httpMethod); + assertNotNull(httpMethod + " " + servletPath + " produced no mapping", mapping); + assertEquals("dog", mapping.getName()); + assertEquals(expectedMethod, mapping.getMethod()); + assertNotNull(httpMethod + " " + servletPath + " must resolve to the action declared in the root namespace," + + " but namespace '" + mapping.getNamespace() + "' does not hold it", + configurationManager.getConfiguration().getRuntimeConfiguration() + .getActionConfig(mapping.getNamespace(), mapping.getName())); + } + + public void testIndexResolvesInRootNamespace() { + assertResolves("/dog", "GET", "index"); + } + + public void testShowResolvesInRootNamespace() { + assertResolves("/dog/1", "GET", "show"); + } + + public void testUpdateResolvesInRootNamespace() { + assertResolves("/dog/1", "PUT", "update"); + } + + public void testDestroyResolvesInRootNamespace() { + assertResolves("/dog/1", "DELETE", "destroy"); + } + + public void testIdIsStillExtracted() { + ActionMapping mapping = map("/dog/1", "GET"); + assertEquals("1", ((String[]) mapping.getParams().get("id"))[0]); + } +} diff --git a/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java b/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java index 12e04b896f..dd5262a810 100644 --- a/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java +++ b/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java @@ -261,6 +261,23 @@ public void testParseNameAndNamespaceWithEdit() { tryUri("/my/foo/23;edit", "/my", "foo/23;edit"); } + /** + * WW-5688: when a package declares the root namespace, a uri that matches no more specific + * namespace belongs to that root rather than to the default namespace, so that "/foo" and + * "/foo/23" agree on where the action lives. Without a root package the default namespace + * still wins - see {@link #testParseNameAndNamespace()}. + */ + public void testParseNameAndNamespaceWithRootPackage() { + config.addPackageConfig("root", new PackageConfig.Builder("root").namespace("/").build()); + + tryUri("/foo/23", "/", "foo/23"); + tryUri("/foo/", "/", "foo/"); + tryUri("/", "/", ""); + + // a longer declared namespace still outranks the root + tryUri("/my/foo/23", "/my", "foo/23"); + } + public void testShouldAllowExclamation() throws Exception { req.setRequestURI("/myapp/animals/dog/fido!edit"); req.setServletPath("/animals/dog/fido!edit"); diff --git a/plugins/rest/src/test/resources/ww-5688.xml b/plugins/rest/src/test/resources/ww-5688.xml new file mode 100644 index 0000000000..1b553347c0 --- /dev/null +++ b/plugins/rest/src/test/resources/ww-5688.xml @@ -0,0 +1,31 @@ + + + + + + + + + +