diff --git a/modules/kernel/src/org/apache/axis2/dispatchers/HTTPLocationBasedDispatcher.java b/modules/kernel/src/org/apache/axis2/dispatchers/HTTPLocationBasedDispatcher.java index 5a58882f50..ce561e5aca 100644 --- a/modules/kernel/src/org/apache/axis2/dispatchers/HTTPLocationBasedDispatcher.java +++ b/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("?"); diff --git a/modules/kernel/src/org/apache/axis2/dispatchers/RequestURIBasedOperationDispatcher.java b/modules/kernel/src/org/apache/axis2/dispatchers/RequestURIBasedOperationDispatcher.java index 761dd1f93d..dbb95001f0 100644 --- a/modules/kernel/src/org/apache/axis2/dispatchers/RequestURIBasedOperationDispatcher.java +++ b/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); diff --git a/modules/kernel/src/org/apache/axis2/util/Utils.java b/modules/kernel/src/org/apache/axis2/util/Utils.java index 35efd3fc5f..edc3d2d030 100644 --- a/modules/kernel/src/org/apache/axis2/util/Utils.java +++ b/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,7 +288,7 @@ 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 */ @@ -299,13 +296,18 @@ 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); @@ -313,11 +315,6 @@ public static String getOperationName(String path, String serviceName) { } 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); diff --git a/modules/kernel/test/org/apache/axis2/dispatchers/HTTPLocationBasedDispatcherTest.java b/modules/kernel/test/org/apache/axis2/dispatchers/HTTPLocationBasedDispatcherTest.java new file mode 100644 index 0000000000..3773329f3b --- /dev/null +++ b/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 httpLocationTable = new HashMap(); + 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")); + } +} diff --git a/modules/kernel/test/org/apache/axis2/dispatchers/RequestURIBasedOperationDispatcherTest.java b/modules/kernel/test/org/apache/axis2/dispatchers/RequestURIBasedOperationDispatcherTest.java index e5befef516..e74aaab598 100644 --- a/modules/kernel/test/org/apache/axis2/dispatchers/RequestURIBasedOperationDispatcherTest.java +++ b/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()); + } } diff --git a/modules/kernel/test/org/apache/axis2/dispatchers/RequestURIBasedServiceDispatcherTest.java b/modules/kernel/test/org/apache/axis2/dispatchers/RequestURIBasedServiceDispatcherTest.java index fad0957bd3..c9241fbf9e 100644 --- a/modules/kernel/test/org/apache/axis2/dispatchers/RequestURIBasedServiceDispatcherTest.java +++ b/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()); + } } diff --git a/modules/kernel/test/org/apache/axis2/util/UtilsParseRequestTest.java b/modules/kernel/test/org/apache/axis2/util/UtilsParseRequestTest.java index 8fdb1cd15a..cd1c9bf0de 100644 --- a/modules/kernel/test/org/apache/axis2/util/UtilsParseRequestTest.java +++ b/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"); + } } diff --git a/systests/webapp-tests/src/test/java/org/apache/axis2/webapp/RequestURIDispatchITCase.java b/systests/webapp-tests/src/test/java/org/apache/axis2/webapp/RequestURIDispatchITCase.java new file mode 100644 index 0000000000..4ac7f33071 --- /dev/null +++ b/systests/webapp-tests/src/test/java/org/apache/axis2/webapp/RequestURIDispatchITCase.java @@ -0,0 +1,59 @@ +/* + * 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.webapp; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; + +import org.junit.Test; + +/** + * The service and operation a request is dispatched to come from the request + * path, which is what the container matched its URL patterns against. A service + * path carried in a query value, or appended after the real service name, must + * not select anything. + */ +public class RequestURIDispatchITCase { + private static HttpResponse get(String pathAndQuery) throws Exception { + int port = Integer.getInteger("jetty.httpPort", 8080); + HttpRequest request = HttpRequest.newBuilder( + URI.create("http://localhost:" + port + "/axis2" + pathAndQuery)).GET().build(); + return HttpClient.newHttpClient().send(request, HttpResponse.BodyHandlers.ofString()); + } + + @Test + public void testServiceIsTakenFromThePathOnly() throws Exception { + // Control: the operation answers when the path names it. + HttpResponse response = get("/services/Version/getVersion"); + assertThat(response.statusCode()).isEqualTo(200); + assertThat(response.body()).contains("getVersionResponse"); + // The path names no deployed service; neither the query string nor a + // repeated service path after it may route the request to Version. + response = get("/services/NoSuchService?next=/axis2/services/Version/getVersion"); + assertThat(response.statusCode()).isEqualTo(404); + assertThat(response.body()).doesNotContain("getVersionResponse"); + response = get("/services/NoSuchService/axis2/services/Version/getVersion"); + assertThat(response.statusCode()).isEqualTo(404); + assertThat(response.body()).doesNotContain("getVersionResponse"); + } +}