https://issues.apache.org/jira/browse/AMQ-6182

Reworking patch so that the http trace method is also turned off by
default for the HttpTransport, besides just for the Websocket transport.
Also added SSL tests for both transports.

activemq-http/src/main/java/org/apache/activemq/transport/WebTransportServerSupport.java

@@ -18,10 +18,15 @@ package org.apache.activemq.transport;
 
 import java.net.InetAddress;
 import java.net.URI;
+import java.util.Map;
 
 import org.apache.activemq.util.InetAddressUtil;
+import org.apache.activemq.util.IntrospectionSupport;
+import org.eclipse.jetty.security.ConstraintMapping;
+import org.eclipse.jetty.security.ConstraintSecurityHandler;
 import org.eclipse.jetty.server.Connector;
 import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.util.security.Constraint;
 
 abstract public class WebTransportServerSupport extends TransportServerSupport {
 
@@ -30,6 +35,7 @@ abstract public class WebTransportServerSupport extends TransportServerSupport {
     protected Connector connector;
     protected SocketConnectorFactory socketConnectorFactory;
     protected String host;
+    protected final HttpOptions httpOptions = new HttpOptions();
 
     public WebTransportServerSupport(URI location) {
         super(location);
@@ -47,6 +53,7 @@ abstract public class WebTransportServerSupport extends TransportServerSupport {
             //ignore, jetty 8.
         }
     }
+
     public URI bind() throws Exception {
 
         URI bind = getBindLocation();
@@ -67,4 +74,35 @@ abstract public class WebTransportServerSupport extends TransportServerSupport {
         setConnectURI(boundUri);
         return boundUri;
     }
+
+    protected void configureTraceMethod(ConstraintSecurityHandler securityHandler,
+            boolean enableTrace) {
+        Constraint constraint = new Constraint();
+        constraint.setName("trace-security");
+        //If enableTrace is true, then we want to set authenticate to false to allow it
+        constraint.setAuthenticate(!enableTrace);
+        ConstraintMapping mapping = new ConstraintMapping();
+        mapping.setConstraint(constraint);
+        mapping.setMethod("TRACE");
+        mapping.setPathSpec("/");
+        securityHandler.addConstraintMapping(mapping);
+    }
+
+    public void setHttpOptions(Map<String, Object> options) {
+        if (options != null) {
+            IntrospectionSupport.setProperties(this.httpOptions, options);
+        }
+    }
+
+    protected static class HttpOptions {
+        private boolean enableTrace = false;
+
+        public boolean isEnableTrace() {
+            return enableTrace;
+        }
+
+        public void setEnableTrace(boolean enableTrace) {
+            this.enableTrace = enableTrace;
+        }
+    }
 }

activemq-http/src/main/java/org/apache/activemq/transport/http/HttpTransportFactory.java

@@ -41,12 +41,15 @@ public class HttpTransportFactory extends TransportFactory {
 
     private static final Logger LOG = LoggerFactory.getLogger(HttpTransportFactory.class);
 
+    @Override
     public TransportServer doBind(URI location) throws IOException {
         try {
             Map<String, String> options = new HashMap<String, String>(URISupport.parseParameters(location));
             HttpTransportServer result = new HttpTransportServer(location, this);
+            Map<String, Object> httpOptions = IntrospectionSupport.extractProperties(options, "http.");
             Map<String, Object> transportOptions = IntrospectionSupport.extractProperties(options, "transport.");
             result.setTransportOption(transportOptions);
+            result.setHttpOptions(httpOptions);
             return result;
         } catch (URISyntaxException e) {
             throw IOExceptionSupport.create(e);
@@ -61,10 +64,12 @@ public class HttpTransportFactory extends TransportFactory {
         return new XStreamWireFormat();
     }
 
+    @Override
     protected String getDefaultWireFormatType() {
         return "xstream";
     }
 
+    @Override
     protected Transport createTransport(URI location, WireFormat wf) throws IOException {
         TextWireFormat textWireFormat = asTextWireFormat(wf);
         // need to remove options from uri
@@ -79,11 +84,13 @@ public class HttpTransportFactory extends TransportFactory {
         return new HttpClientTransport(textWireFormat, uri);
     }
 
+    @Override
     @SuppressWarnings("rawtypes")
     public Transport serverConfigure(Transport transport, WireFormat format, HashMap options) throws Exception {
         return compositeConfigure(transport, format, options);
     }
 
+    @Override
     @SuppressWarnings("rawtypes")
     public Transport compositeConfigure(Transport transport, WireFormat format, Map options) {
         transport = super.compositeConfigure(transport, format, options);

activemq-http/src/main/java/org/apache/activemq/transport/http/HttpTransportServer.java

@@ -26,6 +26,7 @@ import org.apache.activemq.transport.WebTransportServerSupport;
 import org.apache.activemq.transport.util.TextWireFormat;
 import org.apache.activemq.transport.xstream.XStreamWireFormat;
 import org.apache.activemq.util.ServiceStopper;
+import org.eclipse.jetty.security.ConstraintSecurityHandler;
 import org.eclipse.jetty.server.Connector;
 import org.eclipse.jetty.server.Handler;
 import org.eclipse.jetty.server.Server;
@@ -82,7 +83,7 @@ public class HttpTransportServer extends WebTransportServerSupport {
         URI boundTo = bind();
 
         ServletContextHandler contextHandler =
-            new ServletContextHandler(server, "/", ServletContextHandler.NO_SECURITY);
+            new ServletContextHandler(server, "/", ServletContextHandler.SECURITY);
 
         ServletHolder holder = new ServletHolder();
         holder.setServlet(new HttpTunnelServlet());
@@ -93,6 +94,10 @@ public class HttpTransportServer extends WebTransportServerSupport {
         contextHandler.setAttribute("transportFactory", transportFactory);
         contextHandler.setAttribute("transportOptions", transportOptions);
 
+        //AMQ-6182 - disabling trace by default
+        configureTraceMethod((ConstraintSecurityHandler) contextHandler.getSecurityHandler(),
+                httpOptions.isEnableTrace());
+
         addGzipHandler(contextHandler);
 
         server.start();

activemq-http/src/main/java/org/apache/activemq/transport/https/HttpsTransportFactory.java

@@ -41,18 +41,22 @@ public class HttpsTransportFactory extends HttpTransportFactory {
         return doBind(location);
     }
 
+    @Override
     public TransportServer doBind(URI location) throws IOException {
         try {
             Map<String, String> options = new HashMap<String, String>(URISupport.parseParameters(location));
             HttpsTransportServer result = new HttpsTransportServer(location, this, SslContext.getCurrentSslContext());
+            Map<String, Object> httpOptions = IntrospectionSupport.extractProperties(options, "http.");
             Map<String, Object> transportOptions = IntrospectionSupport.extractProperties(options, "transport.");
             result.setTransportOption(transportOptions);
+            result.setHttpOptions(httpOptions);
             return result;
         } catch (URISyntaxException e) {
             throw IOExceptionSupport.create(e);
         }
     }
 
+    @Override
     protected Transport createTransport(URI location, WireFormat wf) throws MalformedURLException {
         // need to remove options from uri
         URI uri;

activemq-http/src/main/java/org/apache/activemq/transport/ws/WSTransportFactory.java

@@ -39,9 +39,11 @@ public class WSTransportFactory extends TransportFactory {
         try {
             Map<String, String> options = new HashMap<String, String>(URISupport.parseParameters(location));
             WSTransportServer result = new WSTransportServer(location);
+            Map<String, Object> httpOptions = IntrospectionSupport.extractProperties(options, "http.");
             Map<String, Object> transportOptions = IntrospectionSupport.extractProperties(options, "");
             IntrospectionSupport.setProperties(result, transportOptions);
             result.setTransportOption(transportOptions);
+            result.setHttpOptions(httpOptions);
             return result;
         } catch (URISyntaxException e) {
             throw IOExceptionSupport.create(e);

activemq-http/src/main/java/org/apache/activemq/transport/ws/WSTransportServer.java

@@ -29,13 +29,11 @@ import org.apache.activemq.transport.WebTransportServerSupport;
 import org.apache.activemq.transport.ws.jetty9.WSServlet;
 import org.apache.activemq.util.IntrospectionSupport;
 import org.apache.activemq.util.ServiceStopper;
-import org.eclipse.jetty.security.ConstraintMapping;
 import org.eclipse.jetty.security.ConstraintSecurityHandler;
 import org.eclipse.jetty.server.Connector;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.servlet.ServletContextHandler;
 import org.eclipse.jetty.servlet.ServletHolder;
-import org.eclipse.jetty.util.security.Constraint;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -70,7 +68,7 @@ public class WSTransportServer extends WebTransportServerSupport {
 
         //AMQ-6182 - disabling trace by default
         configureTraceMethod((ConstraintSecurityHandler) contextHandler.getSecurityHandler(),
-                getHttpOptions().isEnableTrace());
+                httpOptions.isEnableTrace());
 
         Map<String, Object> webSocketOptions = IntrospectionSupport.extractProperties(transportOptions, "websocket.");
         for(Map.Entry<String,Object> webSocketEntry : webSocketOptions.entrySet()) {
@@ -114,31 +112,6 @@ public class WSTransportServer extends WebTransportServerSupport {
         return (Integer)connector.getClass().getMethod("getLocalPort").invoke(connector);
     }
 
-    private void configureTraceMethod(ConstraintSecurityHandler securityHandler,
-            boolean enableTrace) {
-        Constraint constraint = new Constraint();
-        constraint.setName("trace-security");
-        //If enableTrace is true, then we want to set authenticate to false to allow it
-        constraint.setAuthenticate(!enableTrace);
-        ConstraintMapping mapping = new ConstraintMapping();
-        mapping.setConstraint(constraint);
-        mapping.setMethod("TRACE");
-        mapping.setPathSpec("/");
-        securityHandler.addConstraintMapping(mapping);
-    }
-
-    protected static class HttpOptions {
-        private boolean enableTrace = false;
-
-        public boolean isEnableTrace() {
-            return enableTrace;
-        }
-
-        public void setEnableTrace(boolean enableTrace) {
-            this.enableTrace = enableTrace;
-        }
-    }
-
     @Override
     protected void doStop(ServiceStopper stopper) throws Exception {
         Server temp = server;
@@ -161,20 +134,11 @@ public class WSTransportServer extends WebTransportServerSupport {
         this.connector = connector;
     }
 
-    protected HttpOptions getHttpOptions() {
-        HttpOptions httpOptions = new HttpOptions();
-        if (transportOptions != null) {
-            Map<String, Object> httpOptionsMap = IntrospectionSupport.extractProperties(transportOptions, "http.");
-            IntrospectionSupport.setProperties(httpOptions, httpOptionsMap);
-        }
-        return httpOptions;
-    }
-
     @Override
     public void setTransportOption(Map<String, Object> transportOptions) {
         Map<String, Object> socketOptions = IntrospectionSupport.extractProperties(transportOptions, "transport.");
         socketConnectorFactory.setTransportOptions(socketOptions);
-        super.setTransportOption(transportOptions);
+        super.setTransportOption(socketOptions);
     }
 
     @Override

activemq-http/src/main/java/org/apache/activemq/transport/wss/WSSTransportFactory.java

@@ -39,9 +39,11 @@ public class WSSTransportFactory extends TransportFactory {
         try {
             Map<String, String> options = new HashMap<String, String>(URISupport.parseParameters(location));
             WSSTransportServer result = new WSSTransportServer(location, SslContext.getCurrentSslContext());
+            Map<String, Object> httpOptions = IntrospectionSupport.extractProperties(options, "http.");
             Map<String, Object> transportOptions = IntrospectionSupport.extractProperties(options, "");
             IntrospectionSupport.setProperties(result, transportOptions);
             result.setTransportOption(transportOptions);
+            result.setHttpOptions(httpOptions);
             return result;
         } catch (URISyntaxException e) {
             throw IOExceptionSupport.create(e);

activemq-http/src/test/java/org/apache/activemq/transport/http/HttpTraceTestSupport.java

@@ -0,0 +1,56 @@
+package org.apache.activemq.transport.http;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.api.Request;
+import org.eclipse.jetty.client.api.Result;
+import org.eclipse.jetty.client.util.BufferingResponseListener;
+import org.eclipse.jetty.http.HttpMethod;
+import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+
+public class HttpTraceTestSupport {
+
+    public static List<Object[]> getTestParameters() {
+        return Arrays.asList(new Object[][] {
+            //value is empty
+            {"http.enableTrace=", HttpStatus.FORBIDDEN_403},
+            //default, trace method not specified
+            {null, HttpStatus.FORBIDDEN_403},
+            // enable http trace method
+            {"http.enableTrace=true", HttpStatus.OK_200},
+            // disable trace method
+            {"http.enableTrace=false", HttpStatus.FORBIDDEN_403}
+        });
+    }
+
+    public static void testHttpTraceEnabled(final String uri, final int expectedStatus) throws Exception {
+        testHttpTraceEnabled(uri, expectedStatus, new SslContextFactory());
+    }
+
+    public static void testHttpTraceEnabled(final String uri, final int expectedStatus, SslContextFactory
+            sslContextFactory) throws Exception {
+        HttpClient httpClient = sslContextFactory != null ? new HttpClient(sslContextFactory) :
+            new HttpClient(new SslContextFactory());
+        httpClient.start();
+
+        final CountDownLatch latch = new CountDownLatch(1);
+        Request request = httpClient.newRequest(uri).method(HttpMethod.TRACE);
+        final AtomicInteger status = new AtomicInteger();
+        request.send(new BufferingResponseListener() {
+            @Override
+            public void onComplete(Result result) {
+                status.set(result.getResponse().getStatus());
+                latch.countDown();
+            }
+        });
+        latch.await();
+        assertEquals(expectedStatus, status.get());
+    }
+}

activemq-http/src/test/java/org/apache/activemq/transport/http/HttpTransportHttpTraceTest.java

@@ -0,0 +1,88 @@
+/**
+ * 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.activemq.transport.http;
+
+import java.util.Collection;
+
+import org.apache.activemq.broker.BrokerService;
+import org.apache.activemq.broker.TransportConnector;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class HttpTransportHttpTraceTest {
+
+    private BrokerService broker;
+    private String uri;
+
+    protected String enableTraceParam;
+    private int expectedStatus;
+
+
+    @Before
+    public void setUp() throws Exception {
+        additionalConfig();
+
+        broker = new BrokerService();
+        TransportConnector connector = broker.addConnector(getConnectorUri());
+        broker.setPersistent(false);
+        broker.setUseJmx(false);
+        broker.deleteAllMessages();
+        broker.addConnector(connector);
+        broker.start();
+
+        uri = connector.getPublishableConnectString();
+    }
+
+    protected String getConnectorUri() {
+        return "http://localhost:0?" + enableTraceParam;
+    }
+
+    protected void additionalConfig() {
+
+    }
+
+    @After
+    public void tearDown() throws Exception {
+        broker.stop();
+    }
+
+    @Parameters
+    public static Collection<Object[]> data() {
+        return HttpTraceTestSupport.getTestParameters();
+    }
+
+    public HttpTransportHttpTraceTest(final String enableTraceParam, final int expectedStatus) {
+        this.enableTraceParam = enableTraceParam;
+        this.expectedStatus = expectedStatus;
+    }
+
+    /**
+     * This tests whether the TRACE method is enabled or not
+     * @throws Exception
+     */
+    @Test(timeout=10000)
+    public void testHttpTraceEnabled() throws Exception {
+        HttpTraceTestSupport.testHttpTraceEnabled(uri, expectedStatus);
+    }
+
+}

activemq-http/src/test/java/org/apache/activemq/transport/https/HttpsTransportHttpTraceTest.java

@@ -0,0 +1,45 @@
+/**
+ * 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.activemq.transport.https;
+
+import org.apache.activemq.transport.http.HttpTransportHttpTraceTest;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class HttpsTransportHttpTraceTest extends HttpTransportHttpTraceTest {
+
+    public HttpsTransportHttpTraceTest(String enableTraceParam, int expectedStatus) {
+        super(enableTraceParam, expectedStatus);
+    }
+
+    @Override
+    protected String getConnectorUri() {
+        return "https://localhost:0?" + enableTraceParam;
+    }
+
+    @Override
+    public void additionalConfig() {
+        System.setProperty("javax.net.ssl.trustStore", "src/test/resources/client.keystore");
+        System.setProperty("javax.net.ssl.trustStorePassword", "password");
+        System.setProperty("javax.net.ssl.trustStoreType", "jks");
+        System.setProperty("javax.net.ssl.keyStore", "src/test/resources/server.keystore");
+        System.setProperty("javax.net.ssl.keyStorePassword", "password");
+        System.setProperty("javax.net.ssl.keyStoreType", "jks");
+    }
+}

activemq-http/src/test/java/org/apache/activemq/transport/ws/WSTransportHttpTraceTest.java

@@ -17,19 +17,9 @@
 
 package org.apache.activemq.transport.ws;
 
-import static org.junit.Assert.assertEquals;
-
-import java.util.Arrays;
 import java.util.Collection;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.atomic.AtomicInteger;
 
-import org.eclipse.jetty.client.HttpClient;
+import org.apache.activemq.transport.http.HttpTraceTestSupport;
-import org.eclipse.jetty.client.api.Request;
-import org.eclipse.jetty.client.api.Result;
-import org.eclipse.jetty.client.util.BufferingResponseListener;
-import org.eclipse.jetty.http.HttpMethod;
-import org.eclipse.jetty.http.HttpStatus;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -39,21 +29,12 @@ import org.junit.runners.Parameterized.Parameters;
 @RunWith(Parameterized.class)
 public class WSTransportHttpTraceTest extends WSTransportTest {
 
-    private String enableTraceParam;
+    protected String enableTraceParam;
-    private int expectedStatus;
+    protected int expectedStatus;
 
     @Parameters
     public static Collection<Object[]> data() {
-        return Arrays.asList(new Object[][] {
+        return HttpTraceTestSupport.getTestParameters();
-                //value is empty
-                {"http.enableTrace=", HttpStatus.FORBIDDEN_403},
-                //default, trace method not specified
-                {null, HttpStatus.FORBIDDEN_403},
-                // enable http trace method
-                {"http.enableTrace=true", HttpStatus.OK_200},
-                // disable trace method
-                {"http.enableTrace=false", HttpStatus.FORBIDDEN_403}
-        });
     }
 
     public WSTransportHttpTraceTest(final String enableTraceParam, final int expectedStatus) {
@@ -74,21 +55,7 @@ public class WSTransportHttpTraceTest extends WSTransportTest {
      */
     @Test(timeout=10000)
     public void testHttpTraceEnabled() throws Exception {
-        HttpClient httpClient = new HttpClient();
+        HttpTraceTestSupport.testHttpTraceEnabled("http://127.0.0.1:61623", expectedStatus, null);
-        httpClient.start();
-
-        final CountDownLatch latch = new CountDownLatch(1);
-        Request request = httpClient.newRequest("http://127.0.0.1:61623").method(HttpMethod.TRACE);
-        final AtomicInteger status = new AtomicInteger();
-        request.send(new BufferingResponseListener() {
-            @Override
-            public void onComplete(Result result) {
-                status.set(result.getResponse().getStatus());
-                latch.countDown();
-            }
-        });
-        latch.await();
-        assertEquals(expectedStatus, status.get());
     }
 
     @Override

activemq-http/src/test/java/org/apache/activemq/transport/wss/WSSTransportHttpTraceTest.java

@@ -0,0 +1,53 @@
+/**
+ * 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.activemq.transport.wss;
+
+import org.apache.activemq.transport.http.HttpTraceTestSupport;
+import org.apache.activemq.transport.ws.WSTransportHttpTraceTest;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class WSSTransportHttpTraceTest extends WSTransportHttpTraceTest {
+
+    public WSSTransportHttpTraceTest(String enableTraceParam, int expectedStatus) {
+        super(enableTraceParam, expectedStatus);
+    }
+
+    /**
+     * This tests whether the TRACE method is enabled or not
+     * @throws Exception
+     */
+    @Override
+    @Test(timeout=10000)
+    public void testHttpTraceEnabled() throws Exception {
+        SslContextFactory factory = new SslContextFactory();
+        factory.setSslContext(broker.getSslContext().getSSLContext());
+
+        HttpTraceTestSupport.testHttpTraceEnabled("https://127.0.0.1:61623", expectedStatus, factory);
+    }
+
+    @Override
+    protected String getWSConnectorURI() {
+        String uri = "wss://127.0.0.1:61623?websocket.maxTextMessageSize=99999&transport.maxIdleTime=1001";
+        uri = enableTraceParam != null ? uri + "&" + enableTraceParam : uri;
+        return uri;
+    }
+}