CXF-5610 - Jetty transport should warn the user if the endpoint address conflicts with the published service
Project: http://git-wip-us.apache.org/repos/asf/cxf/repo Commit: http://git-wip-us.apache.org/repos/asf/cxf/commit/902671ab Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/902671ab Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/902671ab Branch: refs/heads/master Commit: 902671ab118051ba947a61e74126b1072c5cb5d4 Parents: 926ba99 Author: Willem Jiang <willem.ji...@gmail.com> Authored: Thu Mar 13 19:54:10 2014 +0800 Committer: Willem Jiang <willem.ji...@gmail.com> Committed: Fri Mar 14 20:16:46 2014 +0800 ---------------------------------------------------------------------- .../transport/http_jetty/JettyHTTPHandler.java | 11 ++----- .../http_jetty/JettyHTTPServerEngine.java | 33 +++++++++++++++++-- .../transport/http_jetty/Messages.properties | 2 ++ .../http_jetty/JettyHTTPServerEngineTest.java | 34 ++++++++++++++++---- .../http_jetty/JettyHTTPTestHandler.java | 3 +- .../apache/cxf/transport/http/HttpUrlUtil.java | 34 ++++++++++++++++++++ .../systest/handlers/HandlerInvocationTest.java | 2 +- .../HandlerInvocationUsingAddNumbersTest.java | 1 + 8 files changed, 100 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cxf/blob/902671ab/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/JettyHTTPHandler.java ---------------------------------------------------------------------- diff --git a/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/JettyHTTPHandler.java b/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/JettyHTTPHandler.java index 0894cbe..396e3b2 100644 --- a/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/JettyHTTPHandler.java +++ b/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/JettyHTTPHandler.java @@ -25,6 +25,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.cxf.transport.http.HttpUrlUtil; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.AbstractHandler; @@ -53,14 +54,6 @@ public class JettyHTTPHandler extends AbstractHandler { return urlName; } - boolean checkContextPath(String target) { - String pathString = urlName; - if (!pathString.endsWith("/")) { - pathString = pathString + "/"; - } - return target.startsWith(pathString); - } - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { if (contextMatchExact) { @@ -68,7 +61,7 @@ public class JettyHTTPHandler extends AbstractHandler { jettyHTTPDestination.doService(servletContext, request, response); } } else { - if (target.equals(urlName) || checkContextPath(target)) { + if (target.equals(urlName) || HttpUrlUtil.checkContextPath(urlName, target)) { jettyHTTPDestination.doService(servletContext, request, response); } } http://git-wip-us.apache.org/repos/asf/cxf/blob/902671ab/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/JettyHTTPServerEngine.java ---------------------------------------------------------------------- diff --git a/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/JettyHTTPServerEngine.java b/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/JettyHTTPServerEngine.java index 3d45679..ac571aa 100644 --- a/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/JettyHTTPServerEngine.java +++ b/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/JettyHTTPServerEngine.java @@ -22,6 +22,7 @@ package org.apache.cxf.transport.http_jetty; import java.io.IOException; import java.net.URL; import java.security.GeneralSecurityException; +import java.util.ArrayList; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -35,6 +36,7 @@ import org.apache.cxf.common.util.SystemPropertyAction; import org.apache.cxf.configuration.jsse.TLSServerParameters; import org.apache.cxf.interceptor.Fault; import org.apache.cxf.transport.HttpUriMapper; +import org.apache.cxf.transport.http.HttpUrlUtil; import org.apache.cxf.transport.https_jetty.JettySslConnectorFactory; import org.eclipse.jetty.security.SecurityHandler; import org.eclipse.jetty.server.AbstractConnector; @@ -115,6 +117,8 @@ public class JettyHTTPServerEngine * has been called. */ private boolean configFinalized; + + private List<String> registedPaths = new ArrayList<String>(); /** * This constructor is called by the JettyHTTPServerEngineFactory. @@ -177,6 +181,7 @@ public class JettyHTTPServerEngine * remove it from the factory's cache. */ public void shutdown() { + registedPaths.clear(); if (shouldDestroyPort()) { if (servantCount == 0) { JettyHTTPServerEngineFactory.destroyForPort(port); @@ -271,6 +276,22 @@ public class JettyHTTPServerEngine maxIdleTime = maxIdle; } + protected void checkRegistedContext(URL url) { + String path = url.getPath(); + for (String registedPath : registedPaths) { + if (path.equals(registedPath) + || HttpUrlUtil.checkContextPath(registedPath, path)) { + // Throw the address is already used exception + throw new Fault(new Message("ADD_HANDLER_CONTEXT_IS_USED_MSG", LOG, url, registedPath)); + } + if (HttpUrlUtil.checkContextPath(path, registedPath)) { + throw new Fault(new Message("ADD_HANDLER_CONTEXT_CONFILICT_MSG", LOG, url, registedPath)); + } + } + + } + + /** * Register a servant. * @@ -278,6 +299,9 @@ public class JettyHTTPServerEngine * @param handler notified on incoming HTTP requests */ public synchronized void addServant(URL url, JettyHTTPHandler handler) { + + checkRegistedContext(url); + SecurityHandler securityHandler = null; if (server == null) { DefaultHandler defaultHandler = null; @@ -444,8 +468,8 @@ public class JettyHTTPServerEngine LOG.log(Level.WARNING, "ADD_HANDLER_FAILED_MSG", new Object[] {ex.getMessage()}); } } - - + + registedPaths.add(url.getPath()); ++servantCount; } @@ -515,6 +539,7 @@ public class JettyHTTPServerEngine final String contextName = HttpUriMapper.getContextName(url.getPath()); final String smap = HttpUriMapper.getResourceBase(url.getPath()); + boolean found = false; if (server != null && server.isRunning()) { @@ -543,8 +568,9 @@ public class JettyHTTPServerEngine if (!found) { LOG.log(Level.WARNING, "CAN_NOT_FIND_HANDLER_MSG", new Object[]{url}); } - + registedPaths.remove(url.getPath()); --servantCount; + } @@ -690,6 +716,7 @@ public class JettyHTTPServerEngine * */ protected void stop() throws Exception { + registedPaths.clear(); if (server != null) { try { connector.stop(); http://git-wip-us.apache.org/repos/asf/cxf/blob/902671ab/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/Messages.properties ---------------------------------------------------------------------- diff --git a/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/Messages.properties b/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/Messages.properties index 021f1d3..5c173f4 100644 --- a/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/Messages.properties +++ b/rt/transports/http-jetty/src/main/java/org/apache/cxf/transport/http_jetty/Messages.properties @@ -20,6 +20,8 @@ # START_UP_SERVER_FAILED_MSG = Could not start Jetty server on port {1}: {0} ADD_HANDLER_FAILED_MSG = Could not add cxf jetty handler to Jetty server: {0} +ADD_HANDLER_CONTEXT_IS_USED_MSG = Could not add cxf jetty handler for url {0} to Jetty server, as the path {1} is still in use. +ADD_HANDLER_CONTEXT_CONFILICT_MSG = Could not add cxf jetty handler for url {0} to Jetty server, as it conflicts with the registered path {1}. REMOVE_HANDLER_FAILED_MSG = Could not remove cxf jetty handler from Jetty server: {0} CAN_NOT_FIND_HANDLER_MSG = Could not find the handler to remove for context url {0} FAILED_TO_SHUTDOWN_ENGINE_MSG = Failed to shutdown Jetty server on port {0,number,####0} because it is still in use http://git-wip-us.apache.org/repos/asf/cxf/blob/902671ab/rt/transports/http-jetty/src/test/java/org/apache/cxf/transport/http_jetty/JettyHTTPServerEngineTest.java ---------------------------------------------------------------------- diff --git a/rt/transports/http-jetty/src/test/java/org/apache/cxf/transport/http_jetty/JettyHTTPServerEngineTest.java b/rt/transports/http-jetty/src/test/java/org/apache/cxf/transport/http_jetty/JettyHTTPServerEngineTest.java index bba05e3..b5ab2a6 100644 --- a/rt/transports/http-jetty/src/test/java/org/apache/cxf/transport/http_jetty/JettyHTTPServerEngineTest.java +++ b/rt/transports/http-jetty/src/test/java/org/apache/cxf/transport/http_jetty/JettyHTTPServerEngineTest.java @@ -193,9 +193,27 @@ public class JettyHTTPServerEngineTest extends Assert { response = getResponse(urlStr); assertEquals("The jetty http handler did not take effect", response, "string1"); - engine.addServant(new URL(urlStr), handler2); - response = getResponse(urlStr); - assertEquals("The jetty http handler did not take effect", response, "string1string2"); + try { + engine.addServant(new URL(urlStr), handler2); + fail("We don't support to publish the two service at the same context path"); + } catch (Exception ex) { + assertTrue("Get a wrong exception message", ex.getMessage().indexOf("hello/test") > 0); + } + + try { + engine.addServant(new URL(urlStr + "/test"), handler2); + fail("We don't support to publish the two service at the same context path"); + } catch (Exception ex) { + assertTrue("Get a wrong exception message", ex.getMessage().indexOf("hello/test/test") > 0); + } + + try { + engine.addServant(new URL("http://localhost:" + PORT1 + "/hello"), handler2); + fail("We don't support to publish the two service at the same context path"); + } catch (Exception ex) { + assertTrue("Get a wrong exception message", ex.getMessage().indexOf("hello") > 0); + } + engine.addServant(new URL(urlStr2), handler2); Set<ObjectName> s = CastUtils.cast(ManagementFactory.getPlatformMBeanServer(). @@ -281,6 +299,7 @@ public class JettyHTTPServerEngineTest extends Assert { fail("Can't get the reponse from the server " + ex); } engine.stop(); + JettyHTTPServerEngineFactory.destroyForPort(PORT2); } @Test @@ -318,7 +337,7 @@ public class JettyHTTPServerEngineTest extends Assert { @Test public void testJettyHTTPHandler() throws Exception { - String urlStr1 = "http://localhost:" + PORT3 + "/hello/test"; + String urlStr1 = "http://localhost:" + PORT3 + "/hello/test1"; String urlStr2 = "http://localhost:" + PORT3 + "/hello/test2"; JettyHTTPServerEngine engine = factory.createJettyHTTPServerEngine(PORT3, "http"); @@ -328,9 +347,12 @@ public class JettyHTTPServerEngineTest extends Assert { JettyHTTPHandler handler1 = new JettyHTTPTestHandler("test", false); JettyHTTPHandler handler2 = new JettyHTTPTestHandler("test2", false); engine.addServant(new URL(urlStr1), handler1); + + contextHandler = engine.getContextHandler(new URL(urlStr1)); + engine.addServant(new URL(urlStr2), handler2); - - + contextHandler = engine.getContextHandler(new URL(urlStr2)); + String response = null; try { response = getResponse(urlStr1 + "/test"); http://git-wip-us.apache.org/repos/asf/cxf/blob/902671ab/rt/transports/http-jetty/src/test/java/org/apache/cxf/transport/http_jetty/JettyHTTPTestHandler.java ---------------------------------------------------------------------- diff --git a/rt/transports/http-jetty/src/test/java/org/apache/cxf/transport/http_jetty/JettyHTTPTestHandler.java b/rt/transports/http-jetty/src/test/java/org/apache/cxf/transport/http_jetty/JettyHTTPTestHandler.java index e89a44d..47a105f 100644 --- a/rt/transports/http-jetty/src/test/java/org/apache/cxf/transport/http_jetty/JettyHTTPTestHandler.java +++ b/rt/transports/http-jetty/src/test/java/org/apache/cxf/transport/http_jetty/JettyHTTPTestHandler.java @@ -25,6 +25,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.cxf.transport.http.HttpUrlUtil; import org.eclipse.jetty.server.Request; public class JettyHTTPTestHandler extends JettyHTTPHandler { @@ -49,7 +50,7 @@ public class JettyHTTPTestHandler extends JettyHTTPHandler { resp.flushBuffer(); } else { - if (target.equals(getName()) || checkContextPath(target)) { + if (target.equals(getName()) || HttpUrlUtil.checkContextPath(getName(), target)) { resp.getOutputStream().write(response.getBytes()); resp.flushBuffer(); } http://git-wip-us.apache.org/repos/asf/cxf/blob/902671ab/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HttpUrlUtil.java ---------------------------------------------------------------------- diff --git a/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HttpUrlUtil.java b/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HttpUrlUtil.java new file mode 100644 index 0000000..bf76d22 --- /dev/null +++ b/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HttpUrlUtil.java @@ -0,0 +1,34 @@ +/** + * 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.cxf.transport.http; + +public final class HttpUrlUtil { + + private HttpUrlUtil() { + // Util class + } + + public static boolean checkContextPath(String urlName, String target) { + String pathString = urlName; + if (!pathString.endsWith("/")) { + pathString = pathString + "/"; + } + return target.startsWith(pathString); + } +} http://git-wip-us.apache.org/repos/asf/cxf/blob/902671ab/systests/jaxws/src/test/java/org/apache/cxf/systest/handlers/HandlerInvocationTest.java ---------------------------------------------------------------------- diff --git a/systests/jaxws/src/test/java/org/apache/cxf/systest/handlers/HandlerInvocationTest.java b/systests/jaxws/src/test/java/org/apache/cxf/systest/handlers/HandlerInvocationTest.java index 19ce2e2..5fb4bac 100644 --- a/systests/jaxws/src/test/java/org/apache/cxf/systest/handlers/HandlerInvocationTest.java +++ b/systests/jaxws/src/test/java/org/apache/cxf/systest/handlers/HandlerInvocationTest.java @@ -88,7 +88,7 @@ public class HandlerInvocationTest extends AbstractBusClientServerTestBase { // "com.ibm.ws.webservices.engine.soap.MessageFactoryImpl"); //System.setProperty(SAAJFactoryResolver.SOAP_FACTORY_KEY, // "com.ibm.ws.webservices.engine.xmlsoap.SOAPFactory"); - + createStaticBus(); assertTrue("server did not launch correctly", launchServer(Server.class, true)); } http://git-wip-us.apache.org/repos/asf/cxf/blob/902671ab/systests/jaxws/src/test/java/org/apache/cxf/systest/handlers/HandlerInvocationUsingAddNumbersTest.java ---------------------------------------------------------------------- diff --git a/systests/jaxws/src/test/java/org/apache/cxf/systest/handlers/HandlerInvocationUsingAddNumbersTest.java b/systests/jaxws/src/test/java/org/apache/cxf/systest/handlers/HandlerInvocationUsingAddNumbersTest.java index 2255c31..7163019 100644 --- a/systests/jaxws/src/test/java/org/apache/cxf/systest/handlers/HandlerInvocationUsingAddNumbersTest.java +++ b/systests/jaxws/src/test/java/org/apache/cxf/systest/handlers/HandlerInvocationUsingAddNumbersTest.java @@ -58,6 +58,7 @@ public class HandlerInvocationUsingAddNumbersTest extends AbstractBusClientServe @BeforeClass public static void startServers() throws Exception { + createStaticBus(); assertTrue("server did not launch correctly", launchServer(HandlerServer.class, true)); }