From 9939db4720436cda5436ca506a92e06954796e5b Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Wed, 24 Aug 2016 14:14:55 +0200 Subject: [PATCH] SONAR-7989 remove AJP support in embedded tomcat --- .../process/ProcessTest/sonar.properties | 5 -- .../process/ProcessTest/sonar.properties | 5 -- .../sonar/server/app/TomcatConnectors.java | 69 ++++--------------- .../sonar/server/app/TomcatStartupLogs.java | 8 +-- .../org/sonar/server/app/StartupLogsTest.java | 22 +++--- .../server/app/TomcatConnectorsTest.java | 51 +++----------- .../src/main/assembly/conf/sonar.properties | 3 - 7 files changed, 38 insertions(+), 125 deletions(-) diff --git a/server/sonar-process-monitor/src/test/resources/org/sonar/process/ProcessTest/sonar.properties b/server/sonar-process-monitor/src/test/resources/org/sonar/process/ProcessTest/sonar.properties index 583c4c45f1f..1f54b2bc65c 100644 --- a/server/sonar-process-monitor/src/test/resources/org/sonar/process/ProcessTest/sonar.properties +++ b/server/sonar-process-monitor/src/test/resources/org/sonar/process/ProcessTest/sonar.properties @@ -108,11 +108,6 @@ sonar.jdbc.timeBetweenEvictionRunsMillis=30000 # Access logs are enabled by default. #sonar.web.accessLogs.enable=true -# TCP port for incoming AJP connections. Disabled when value is -1. -# sonar.ajp.port=9009 - - - #-------------------------------------------------------------------------------------------------- # UPDATE CENTER diff --git a/server/sonar-process/src/test/resources/org/sonar/process/ProcessTest/sonar.properties b/server/sonar-process/src/test/resources/org/sonar/process/ProcessTest/sonar.properties index 583c4c45f1f..1f54b2bc65c 100644 --- a/server/sonar-process/src/test/resources/org/sonar/process/ProcessTest/sonar.properties +++ b/server/sonar-process/src/test/resources/org/sonar/process/ProcessTest/sonar.properties @@ -108,11 +108,6 @@ sonar.jdbc.timeBetweenEvictionRunsMillis=30000 # Access logs are enabled by default. #sonar.web.accessLogs.enable=true -# TCP port for incoming AJP connections. Disabled when value is -1. -# sonar.ajp.port=9009 - - - #-------------------------------------------------------------------------------------------------- # UPDATE CENTER diff --git a/server/sonar-server/src/main/java/org/sonar/server/app/TomcatConnectors.java b/server/sonar-server/src/main/java/org/sonar/server/app/TomcatConnectors.java index 986f2a8a4f2..2111ccaa2aa 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/app/TomcatConnectors.java +++ b/server/sonar-server/src/main/java/org/sonar/server/app/TomcatConnectors.java @@ -19,28 +19,21 @@ */ package org.sonar.server.app; -import static com.google.common.collect.FluentIterable.from; -import static java.util.Arrays.asList; - -import com.google.common.base.Predicates; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.apache.catalina.connector.Connector; import org.apache.catalina.startup.Tomcat; import org.sonar.process.Props; +import static java.lang.String.format; + /** * Configuration of Tomcat connectors */ class TomcatConnectors { - public static final int DISABLED_PORT = -1; - public static final String HTTP_PROTOCOL = "HTTP/1.1"; - public static final String AJP_PROTOCOL = "AJP/1.3"; - public static final int MAX_HTTP_HEADER_SIZE_BYTES = 48 * 1024; + private static final int UNDEFINED_PORT = 0; + protected static final String HTTP_PROTOCOL = "HTTP/1.1"; + protected static final int MAX_HTTP_HEADER_SIZE_BYTES = 48 * 1024; private static final int MAX_POST_SIZE = -1; private TomcatConnectors() { @@ -48,55 +41,23 @@ class TomcatConnectors { } static void configure(Tomcat tomcat, Props props) { - List connectors = from(asList(newHttpConnector(props), newAjpConnector(props))) - .filter(Predicates.notNull()) - .toList(); - - verify(connectors); - - tomcat.setConnector(connectors.get(0)); - for (Connector connector : connectors) { - tomcat.getService().addConnector(connector); - } + Connector httpConnector = newHttpConnector(props); + tomcat.setConnector(httpConnector); + tomcat.getService().addConnector(httpConnector); } - private static void verify(List connectors) { - if (connectors.isEmpty()) { - throw new IllegalStateException("HTTP connectors are disabled"); - } - Set ports = new HashSet<>(); - for (Connector connector : connectors) { - int port = connector.getPort(); - if (ports.contains(port)) { - throw new IllegalStateException(String.format("HTTP and AJP must not use the same port %d", port)); - } - ports.add(port); - } - } - - @CheckForNull private static Connector newHttpConnector(Props props) { - Connector connector = null; // Not named "sonar.web.http.port" to keep backward-compatibility int port = props.valueAsInt("sonar.web.port", 9000); - if (port > DISABLED_PORT) { - connector = newConnector(props, HTTP_PROTOCOL, "http"); - configureMaxHttpHeaderSize(connector); - connector.setPort(port); - connector.setMaxPostSize(MAX_POST_SIZE); + if (port < UNDEFINED_PORT) { + throw new IllegalStateException(format("HTTP port '%s' is invalid", port)); } - return connector; - } - @CheckForNull - private static Connector newAjpConnector(Props props) { - int port = props.valueAsInt("sonar.ajp.port", DISABLED_PORT); - if (port > DISABLED_PORT) { - Connector connector = newConnector(props, AJP_PROTOCOL, "http"); - connector.setPort(port); - return connector; - } - return null; + Connector connector = newConnector(props, HTTP_PROTOCOL, "http"); + configureMaxHttpHeaderSize(connector); + connector.setPort(port); + connector.setMaxPostSize(MAX_POST_SIZE); + return connector; } /** diff --git a/server/sonar-server/src/main/java/org/sonar/server/app/TomcatStartupLogs.java b/server/sonar-server/src/main/java/org/sonar/server/app/TomcatStartupLogs.java index 321fd727a27..f470e95dbff 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/app/TomcatStartupLogs.java +++ b/server/sonar-server/src/main/java/org/sonar/server/app/TomcatStartupLogs.java @@ -35,9 +35,7 @@ class TomcatStartupLogs { void log(Tomcat tomcat) { Connector[] connectors = tomcat.getService().findConnectors(); for (Connector connector : connectors) { - if (StringUtils.containsIgnoreCase(connector.getProtocol(), "AJP")) { - logAjp(connector); - } else if (StringUtils.equalsIgnoreCase(connector.getScheme(), "http")) { + if (StringUtils.equalsIgnoreCase(connector.getScheme(), "http")) { logHttp(connector); } else { throw new IllegalArgumentException("Unsupported connector: " + connector); @@ -45,10 +43,6 @@ class TomcatStartupLogs { } } - private void logAjp(Connector connector) { - log.info(String.format("%s connector enabled on port %d", connector.getProtocol(), connector.getPort())); - } - private void logHttp(Connector connector) { log.info(String.format("HTTP connector enabled on port %d", connector.getPort())); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/app/StartupLogsTest.java b/server/sonar-server/src/test/java/org/sonar/server/app/StartupLogsTest.java index d4fe8613db1..a74d0faddd6 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/app/StartupLogsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/app/StartupLogsTest.java @@ -19,13 +19,13 @@ */ package org.sonar.server.app; -import java.util.Properties; import org.apache.catalina.connector.Connector; import org.apache.catalina.startup.Tomcat; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.mockito.Mockito; import org.sonar.api.utils.log.Logger; -import org.sonar.process.Props; import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; @@ -34,20 +34,22 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; public class StartupLogsTest { + @Rule + public ExpectedException expectedException = ExpectedException.none(); - Tomcat tomcat = mock(Tomcat.class, Mockito.RETURNS_DEEP_STUBS); - Logger logger = mock(Logger.class); - TomcatStartupLogs underTest = new TomcatStartupLogs(logger); + private Tomcat tomcat = mock(Tomcat.class, Mockito.RETURNS_DEEP_STUBS); + private Logger logger = mock(Logger.class); + private TomcatStartupLogs underTest = new TomcatStartupLogs(logger); @Test - public void logAjp() { - Connector connector = newConnector("AJP/1.3", "http"); + public void fail_with_IAE_on_unsupported_protocol() { + Connector connector = newConnector("AJP/1.3", "ajp"); when(tomcat.getService().findConnectors()).thenReturn(new Connector[] {connector}); + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Unsupported connector: Connector[AJP/1.3-1234]"); + underTest.log(tomcat); - - verify(logger).info("AJP/1.3 connector enabled on port 1234"); - verifyNoMoreInteractions(logger); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/app/TomcatConnectorsTest.java b/server/sonar-server/src/test/java/org/sonar/server/app/TomcatConnectorsTest.java index d7cc1c5e696..7380631cfcf 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/app/TomcatConnectorsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/app/TomcatConnectorsTest.java @@ -38,12 +38,6 @@ */ package org.sonar.server.app; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.fail; -import static org.mockito.Matchers.argThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; - import com.google.common.collect.ImmutableMap; import java.net.InetAddress; import java.util.Map; @@ -55,6 +49,12 @@ import org.mockito.ArgumentMatcher; import org.mockito.Mockito; import org.sonar.process.Props; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.fail; +import static org.mockito.Matchers.argThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + public class TomcatConnectorsTest { Tomcat tomcat = mock(Tomcat.class, Mockito.RETURNS_DEEP_STUBS); @@ -104,24 +104,9 @@ public class TomcatConnectorsTest { } @Test - public void fail_if_http_connectors_are_disabled() { - Properties p = new Properties(); - p.setProperty("sonar.web.port", "-1"); - Props props = new Props(p); - - try { - TomcatConnectors.configure(tomcat, props); - fail(); - } catch (IllegalStateException e) { - assertThat(e.getMessage()).isEqualTo("HTTP connectors are disabled"); - } - } - - @Test - public void all_connectors_are_enabled() { + public void http_connector_is_enabled() { Properties p = new Properties(); p.setProperty("sonar.web.port", "9000"); - p.setProperty("sonar.ajp.port", "9009"); Props props = new Props(p); TomcatConnectors.configure(tomcat, props); @@ -133,26 +118,18 @@ public class TomcatConnectorsTest { return c.getScheme().equals("http") && c.getPort() == 9000 && c.getProtocol().equals(TomcatConnectors.HTTP_PROTOCOL); } })); - verify(tomcat.getService()).addConnector(argThat(new ArgumentMatcher() { - @Override - public boolean matches(Object o) { - Connector c = (Connector) o; - return c.getScheme().equals("http") && c.getPort() == 9009 && c.getProtocol().equals(TomcatConnectors.AJP_PROTOCOL); - } - })); } @Test - public void http_and_ajp_ports_should_be_different() { + public void fail_with_ISE_if_http_port_is_invalid() { Properties p = new Properties(); - p.setProperty("sonar.web.port", "9000"); - p.setProperty("sonar.ajp.port", "9000"); + p.setProperty("sonar.web.port", "-1"); try { TomcatConnectors.configure(tomcat, new Props(p)); fail(); } catch (IllegalStateException e) { - assertThat(e).hasMessage("HTTP and AJP must not use the same port 9000"); + assertThat(e).hasMessage("HTTP port '-1' is invalid"); } } @@ -160,7 +137,6 @@ public class TomcatConnectorsTest { public void bind_to_all_addresses_by_default() { Properties p = new Properties(); p.setProperty("sonar.web.port", "9000"); - p.setProperty("sonar.ajp.port", "9009"); TomcatConnectors.configure(tomcat, new Props(p)); @@ -171,13 +147,6 @@ public class TomcatConnectorsTest { return c.getScheme().equals("http") && c.getPort() == 9000 && ((InetAddress) c.getProperty("address")).getHostAddress().equals("0.0.0.0"); } })); - verify(tomcat.getService()).addConnector(argThat(new ArgumentMatcher() { - @Override - public boolean matches(Object o) { - Connector c = (Connector) o; - return c.getScheme().equals("http") && c.getPort() == 9009 && ((InetAddress) c.getProperty("address")).getHostAddress().equals("0.0.0.0"); - } - })); } @Test diff --git a/sonar-application/src/main/assembly/conf/sonar.properties b/sonar-application/src/main/assembly/conf/sonar.properties index 3925c097c85..762c855e764 100644 --- a/sonar-application/src/main/assembly/conf/sonar.properties +++ b/sonar-application/src/main/assembly/conf/sonar.properties @@ -123,9 +123,6 @@ # The default value is 25. #sonar.web.http.acceptCount=25 -# TCP port for incoming AJP connections. Disabled if value is -1. Disabled by default. -#sonar.ajp.port=-1 - # By default users are logged out and sessions closed when server is restarted. # If you prefer keeping user sessions open, a secret should be defined. Value is # HS256 key encoded with base64. It must be unique for each installation of SonarQube. -- 2.39.5