dispatch on the request path only in the request uri dispatchers

modules/kernel/src/org/apache/axis2/dispatchers/HTTPLocationBasedDispatcher.java

@@ -99,7 +99,14 @@ public void initDispatcher() {
     private String parseRequestURL(String path, String serviceName) {
 
         serviceName = "/" + serviceName;
-        int index = path.lastIndexOf(serviceName);
+        // The query string stays in the location, since location templates may
+        // match on it, but only the path may say where the service name and the
+        // location start: a query value can contain both.
+        int pathEnd = path.indexOf('?');
+        if (pathEnd == -1) {
+            pathEnd = path.length();
+        }
+        int index = path.lastIndexOf(serviceName, pathEnd - serviceName.length());
         String httpLocation = null;
 
         if (-1 != index) {
@@ -111,7 +118,8 @@ private String parseRequestURL(String path, String serviceName) {
 
         if (httpLocation != null) {
             index = httpLocation.indexOf("/");
-            if (-1 != index) {
+            int locationQuery = httpLocation.indexOf("?");
+            if (-1 != index && (locationQuery == -1 || index < locationQuery)) {
                 httpLocation = httpLocation.substring(index);
             } else {
                 int queryIndex = httpLocation.indexOf("?");

modules/kernel/src/org/apache/axis2/dispatchers/RequestURIBasedOperationDispatcher.java

@@ -50,7 +50,13 @@ public AxisOperation findOperation(AxisService service, MessageContext messageCo
         EndpointReference toEPR = messageContext.getTo();
         if (toEPR != null) {
             String filePart = toEPR.getAddress();
-            String operation  = Utils.getOperationName(filePart, service.getName());
+            // Read the operation from the part after the service context path, the
+            // part the service was resolved from, so a suffix that repeats the
+            // service name cannot pick a different operation.
+            String serviceOpPart = Utils.getServiceAndOperationPart(filePart,
+                    messageContext.getConfigurationContext().getServiceContextPath());
+            String operation  = Utils.getOperationName(
+                    serviceOpPart != null ? serviceOpPart : filePart, service.getName());
 
             if (operation != null) {
                 QName operationName = new QName(operation);

modules/kernel/src/org/apache/axis2/util/Utils.java

@@ -210,8 +210,13 @@ public static String[] parseRequestURLForServiceAndOperation(String path, String
         }
         String[] values = new String[2];
 
+        // The query string is not part of the path the container matched, so it
+        // takes no part in choosing the service or the operation. The service path
+        // is then the first match: a later one is a suffix the caller appended.
+        path = withoutQuery(path);
+
         // TODO. This is kind of brittle. Any service with the name /services would cause fun.
-        int index = path.lastIndexOf(servicePath);
+        int index = path.indexOf(servicePath);
         String service;
 
         if (-1 != index) {
@@ -220,12 +225,6 @@ public static String[] parseRequestURLForServiceAndOperation(String path, String
             if (path.length() > serviceStart + 1) {
                 service = path.substring(serviceStart + 1);
 
-                int queryIndex = service.indexOf('?');
-
-                if (queryIndex > 0) {
-                    service = service.substring(0, queryIndex);
-                }
-
                 int operationIndex = service.indexOf('/');
 
                 if (operationIndex > 0) {
@@ -265,7 +264,11 @@ public static String getServiceAndOperationPart(String path, String servicePath)
         	servicePath = servicePath+"/";
         }
 
-        int index = path.lastIndexOf(servicePath);
+        // Only the path names the service: a query value or a repeated suffix that
+        // contains the service path must not win over the segment the container saw.
+        path = withoutQuery(path);
+
+        int index = path.indexOf(servicePath);
         String serviceOpPart = null;
 
         if (-1 != index) {
@@ -274,12 +277,6 @@ public static String getServiceAndOperationPart(String path, String servicePath)
             //get the string after services path
             if (path.length() > serviceStart) {
                 serviceOpPart = path.substring(serviceStart);
-
-                //remove everything after ?
-                int queryIndex = serviceOpPart.indexOf('?');
-                if (queryIndex > 0) {
-                    serviceOpPart = serviceOpPart.substring(0, queryIndex);
-                }
             }
         }
         return serviceOpPart;
@@ -291,33 +288,33 @@ public static String getServiceAndOperationPart(String path, String servicePath)
      * Ex:  ../services/Echo/echoString -> echoString
      *      ../services/foo/1.0.0/Echo/echoString -> echoString
      *      ../services/Echo/ -> null
-     * @param path - request URI
+     * @param path - request URI, or the part of it after the service context path
      * @param serviceName - service name
      * @return - operation name if any, else null
      */
     public static String getOperationName(String path, String serviceName) {
         if (path == null || serviceName == null) {
             return null;
         }
-        int idx = path.lastIndexOf(serviceName + "/");
+        // The operation is read from the path only; a query value may legitimately
+        // contain the service name followed by a slash. The operation follows the
+        // first occurrence of the service name: a later one is a suffix the caller
+        // appended.
+        path = withoutQuery(path);
+        int idx = path.indexOf(serviceName + "/");
         String operationName = null;
         if (idx != -1) {
             operationName = path.substring(idx + serviceName.length() + 1);
         } else {
             //this scenario occurs if the endpoint name is there in the URL after service name
-            idx = path.lastIndexOf(serviceName + ".");
+            idx = path.indexOf(serviceName + ".");
             if (idx != -1) {
                 operationName = path.substring(idx + serviceName.length() + 1);
                 operationName = operationName.substring(operationName.indexOf('/') + 1);
             }
         }
 
         if (operationName != null) {
-            //remove everyting after '?'
-            int queryIndex = operationName.indexOf('?');
-            if (queryIndex > 0) {
-                operationName = operationName.substring(0, queryIndex);
-            }
             //take the part upto / as the operation name
             if (operationName.indexOf("/") != -1) {
                 operationName = operationName.substring(0, operationName.indexOf("/"));
@@ -326,6 +323,14 @@ public static String getOperationName(String path, String serviceName) {
         return operationName;
     }
 
+    /**
+     * Strips the query string from a request URL or URI, if it has one.
+     */
+    private static String withoutQuery(String path) {
+        int queryIndex = path.indexOf('?');
+        return queryIndex == -1 ? path : path.substring(0, queryIndex);
+    }
+
     public static ConfigurationContext getNewConfigurationContext(String repositry)
             throws Exception {
         final File file = new File(repositry);

modules/kernel/test/org/apache/axis2/dispatchers/HTTPLocationBasedDispatcherTest.java

@@ -0,0 +1,90 @@
+/*
+ * 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.axis2.dispatchers;
+
+import junit.framework.TestCase;
+import org.apache.axis2.AxisFault;
+import org.apache.axis2.addressing.EndpointReference;
+import org.apache.axis2.context.ConfigurationContext;
+import org.apache.axis2.context.ConfigurationContextFactory;
+import org.apache.axis2.context.MessageContext;
+import org.apache.axis2.description.AxisBinding;
+import org.apache.axis2.description.AxisEndpoint;
+import org.apache.axis2.description.AxisOperation;
+import org.apache.axis2.description.AxisService;
+import org.apache.axis2.description.InOnlyAxisOperation;
+import org.apache.axis2.description.WSDL2Constants;
+import org.apache.axis2.kernel.http.HTTPConstants;
+
+import javax.xml.namespace.QName;
+import java.util.HashMap;
+import java.util.Map;
+
+public class HTTPLocationBasedDispatcherTest extends TestCase {
+
+    private AxisService service;
+    private AxisOperation safe;
+    private AxisOperation delete;
+    private AxisEndpoint endpoint;
+    private ConfigurationContext cc;
+
+    protected void setUp() throws Exception {
+        service = new AxisService("Svc");
+        safe = new InOnlyAxisOperation(new QName("safe"));
+        delete = new InOnlyAxisOperation(new QName("delete"));
+        service.addOperation(safe);
+        service.addOperation(delete);
+
+        Map<String, AxisOperation> httpLocationTable = new HashMap<String, AxisOperation>();
+        httpLocationTable.put("GET/safe", safe);
+        httpLocationTable.put("GET/delete", delete);
+        AxisBinding binding = new AxisBinding();
+        binding.setProperty(WSDL2Constants.HTTP_LOCATION_TABLE, httpLocationTable);
+        endpoint = new AxisEndpoint();
+        endpoint.setBinding(binding);
+
+        cc = ConfigurationContextFactory.createEmptyConfigurationContext();
+        cc.getAxisConfiguration().addService(service);
+    }
+
+    private AxisOperation findOperation(String to) throws AxisFault {
+        MessageContext messageContext = cc.createMessageContext();
+        messageContext.setTo(new EndpointReference(to));
+        messageContext.setAxisService(service);
+        messageContext.setProperty(WSDL2Constants.ENDPOINT_LOCAL_NAME, endpoint);
+        messageContext.setProperty(HTTPConstants.HTTP_METHOD, "GET");
+        return new HTTPLocationBasedDispatcher().findOperation(service, messageContext);
+    }
+
+    public void testLocationFromPath() throws AxisFault {
+        assertEquals(safe, findOperation("/axis2/services/Svc/safe"));
+        assertEquals(delete, findOperation("/axis2/services/Svc/delete"));
+        assertEquals(safe, findOperation("/axis2/services/Svc/safe?x=1"));
+    }
+
+    /**
+     * The location starts after the service name in the request path; a query
+     * value that repeats the service name must not select a different location.
+     */
+    public void testLocationNotTakenFromQueryString() throws AxisFault {
+        assertEquals(safe, findOperation("/axis2/services/Svc/safe?x=/Svc/delete"));
+        assertNull(findOperation("/axis2/services/Svc?x=/Svc/delete"));
+    }
+}

modules/kernel/test/org/apache/axis2/dispatchers/RequestURIBasedOperationDispatcherTest.java

@@ -117,4 +117,60 @@ public void testFindService() throws AxisFault {
         assertEquals(operation4, messageContext.getAxisOperation());
     }
 
+    /**
+     * The operation comes from the request path alone, never from a query value
+     * that happens to contain the service name.
+     */
+    public void testOperationNotTakenFromQueryString() throws AxisFault {
+        AxisService service = new AxisService("Service1");
+        AxisOperation operation1 = new InOnlyAxisOperation(new QName("operation1"));
+        AxisOperation operation2 = new InOnlyAxisOperation(new QName("operation2"));
+        service.addOperation(operation1);
+        service.addOperation(operation2);
+        ConfigurationContext cc = ConfigurationContextFactory.createEmptyConfigurationContext();
+        cc.getAxisConfiguration().addService(service);
+        RequestURIBasedOperationDispatcher ruisd = new RequestURIBasedOperationDispatcher();
+
+        MessageContext messageContext = cc.createMessageContext();
+        messageContext.setTo(new EndpointReference(
+                "/axis2/services/Service1/operation1?next=/axis2/services/Service1/operation2"));
+        messageContext.setAxisService(service);
+        ruisd.invoke(messageContext);
+        assertEquals(operation1, messageContext.getAxisOperation());
+
+        messageContext = cc.createMessageContext();
+        messageContext.setTo(new EndpointReference(
+                "/axis2/services/Service1?next=/axis2/services/Service1/operation2"));
+        messageContext.setAxisService(service);
+        ruisd.invoke(messageContext);
+        assertNull(messageContext.getAxisOperation());
+    }
+
+    /**
+     * A path suffix that repeats the service name must not choose the operation:
+     * the operation is the segment right after the service the path names.
+     */
+    public void testOperationNotTakenFromRepeatedServiceName() throws AxisFault {
+        AxisService service = new AxisService("Echo");
+        AxisOperation echo = new InOnlyAxisOperation(new QName("echo"));
+        AxisOperation deleteAll = new InOnlyAxisOperation(new QName("deleteAll"));
+        service.addOperation(echo);
+        service.addOperation(deleteAll);
+        ConfigurationContext cc = ConfigurationContextFactory.createEmptyConfigurationContext();
+        cc.getAxisConfiguration().addService(service);
+        RequestURIBasedOperationDispatcher ruisd = new RequestURIBasedOperationDispatcher();
+
+        MessageContext messageContext = cc.createMessageContext();
+        messageContext.setTo(new EndpointReference("/axis2/services/Echo/echo/Echo/deleteAll"));
+        messageContext.setAxisService(service);
+        ruisd.invoke(messageContext);
+        assertEquals(echo, messageContext.getAxisOperation());
+
+        messageContext = cc.createMessageContext();
+        messageContext.setTo(new EndpointReference(
+                "/axis2/services/Echo.EchoEndpoint/echo/Echo.EchoEndpoint/deleteAll"));
+        messageContext.setAxisService(service);
+        ruisd.invoke(messageContext);
+        assertEquals(echo, messageContext.getAxisOperation());
+    }
 }

modules/kernel/test/org/apache/axis2/dispatchers/RequestURIBasedServiceDispatcherTest.java

@@ -158,4 +158,37 @@ public void testFindService() throws AxisFault {
         assertEquals(as4, messageContext.getAxisService());
     }
 
+    /**
+     * The service comes from the request path alone. A REST To address still
+     * carries the query string, and the servlet container matched its URL
+     * patterns against the path only, so a service path inside a query value or
+     * in a repeated path suffix must not select a different service.
+     */
+    public void testServiceNotTakenFromQueryStringOrRepeatedPath() throws AxisFault {
+        AxisService as1 = new AxisService("Service1");
+        AxisService as2 = new AxisService("Service2");
+        ConfigurationContext cc = ConfigurationContextFactory.createEmptyConfigurationContext();
+        AxisConfiguration ac = cc.getAxisConfiguration();
+        ac.addService(as1);
+        ac.addService(as2);
+        RequestURIBasedServiceDispatcher ruisd = new RequestURIBasedServiceDispatcher();
+
+        MessageContext messageContext = cc.createMessageContext();
+        messageContext.setTo(new EndpointReference(
+                "/axis2/services/Service1?next=/axis2/services/Service2/operation"));
+        ruisd.invoke(messageContext);
+        assertEquals(as1, messageContext.getAxisService());
+
+        messageContext = cc.createMessageContext();
+        messageContext.setTo(new EndpointReference(
+                "/axis2/services/NoSuchService?next=/axis2/services/Service2"));
+        ruisd.invoke(messageContext);
+        assertNull(messageContext.getAxisService());
+
+        messageContext = cc.createMessageContext();
+        messageContext.setTo(new EndpointReference(
+                "/axis2/services/Service1/axis2/services/Service2"));
+        ruisd.invoke(messageContext);
+        assertEquals(as1, messageContext.getAxisService());
+    }
 }

modules/kernel/test/org/apache/axis2/util/UtilsParseRequestTest.java

@@ -80,4 +80,13 @@ private void assertParsesTo(String path, String service, String operation) {
         assertEquals(operation, strings[1]);
     }
 
+    public void testServiceNotTakenFromQuery() throws Exception {
+        assertParsesTo("http://localhost:8081/axis2/services/System?next=/axis2/services/Other/operation",
+                       "System");
+    }
+
+    public void testOperationNotTakenFromQuery() throws Exception {
+        assertParsesTo("http://localhost:8081/axis2/services/System/operation?next=/axis2/services/Other/other",
+                       "System", "operation");
+    }
 }