From 0c55471ff857cbe6c6a12c73e6e5a7514ab7ba7b Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Wed, 29 Mar 2017 08:57:42 +0200 Subject: [PATCH] SONAR-9082 H2 should bind to localhost interface --- .../application/config/JdbcSettings.java | 18 ++++++-- .../application/config/JdbcSettingsTest.java | 3 +- .../server/platform/db/EmbeddedDatabase.java | 20 ++++++--- .../platform/db/EmbeddedDatabaseFactory.java | 9 +++- .../db/EmbeddedDatabaseFactoryTest.java | 8 ++-- .../platform/db/EmbeddedDatabaseTest.java | 45 +++++++++++-------- .../java/org/sonar/api/utils/System2.java | 9 ++++ .../java/org/sonar/api/utils/System2Test.java | 33 +++++++++----- 8 files changed, 99 insertions(+), 46 deletions(-) diff --git a/server/sonar-process-monitor/src/main/java/org/sonar/application/config/JdbcSettings.java b/server/sonar-process-monitor/src/main/java/org/sonar/application/config/JdbcSettings.java index 52f772958fc..9011f167c8e 100644 --- a/server/sonar-process-monitor/src/main/java/org/sonar/application/config/JdbcSettings.java +++ b/server/sonar-process-monitor/src/main/java/org/sonar/application/config/JdbcSettings.java @@ -20,6 +20,8 @@ package org.sonar.application.config; import java.io.File; +import java.net.Inet6Address; +import java.net.InetAddress; import java.util.ArrayList; import java.util.List; import java.util.function.Consumer; @@ -33,6 +35,7 @@ import org.sonar.process.MessageException; import org.sonar.process.ProcessProperties; import org.sonar.process.Props; +import static java.lang.String.format; import static org.apache.commons.lang.StringUtils.isEmpty; import static org.apache.commons.lang.StringUtils.isNotEmpty; import static org.sonar.process.ProcessProperties.JDBC_EMBEDDED_PORT; @@ -99,18 +102,25 @@ public class JdbcSettings implements Consumer { Pattern pattern = Pattern.compile("jdbc:(\\w+):.+"); Matcher matcher = pattern.matcher(url); if (!matcher.find()) { - throw new MessageException(String.format("Bad format of JDBC URL: %s", url)); + throw new MessageException(format("Bad format of JDBC URL: %s", url)); } String key = matcher.group(1); try { return Provider.valueOf(StringUtils.upperCase(key)); } catch (IllegalArgumentException e) { - throw new MessageException(String.format("Unsupported JDBC driver provider: %s", key)); + throw new MessageException(format("Unsupported JDBC driver provider: %s", key)); } } private static String buildH2JdbcUrl(int embeddedDatabasePort) { - return "jdbc:h2:tcp://localhost:" + embeddedDatabasePort + "/sonar"; + InetAddress ip = InetAddress.getLoopbackAddress(); + String host; + if (ip instanceof Inet6Address) { + host = "[" + ip.getHostAddress() + "]"; + } else { + host = ip.getHostAddress(); + } + return format("jdbc:h2:tcp://%s:%d/sonar", host, embeddedDatabasePort); } void checkUrlParameters(Provider provider, String url) { @@ -145,7 +155,7 @@ public class JdbcSettings implements Consumer { private static void checkRequiredParameter(String url, String val) { if (!url.contains(val)) { - throw new MessageException(String.format("JDBC URL must have the property '%s'", val)); + throw new MessageException(format("JDBC URL must have the property '%s'", val)); } } diff --git a/server/sonar-process-monitor/src/test/java/org/sonar/application/config/JdbcSettingsTest.java b/server/sonar-process-monitor/src/test/java/org/sonar/application/config/JdbcSettingsTest.java index 45510230779..5b4eb945364 100644 --- a/server/sonar-process-monitor/src/test/java/org/sonar/application/config/JdbcSettingsTest.java +++ b/server/sonar-process-monitor/src/test/java/org/sonar/application/config/JdbcSettingsTest.java @@ -20,6 +20,7 @@ package org.sonar.application.config; import java.io.File; +import java.net.InetAddress; import java.util.Properties; import org.apache.commons.io.FileUtils; import org.junit.Before; @@ -56,7 +57,7 @@ public class JdbcSettingsTest { assertThat(underTest.resolveProviderAndEnforceNonnullJdbcUrl(props)) .isEqualTo(Provider.H2); - assertThat(props.nonNullValue(JDBC_URL)).isEqualTo("jdbc:h2:tcp://localhost:9092/sonar"); + assertThat(props.nonNullValue(JDBC_URL)).isEqualTo(String.format("jdbc:h2:tcp://%s:9092/sonar", InetAddress.getLoopbackAddress().getHostAddress())); } @Test diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/db/EmbeddedDatabase.java b/server/sonar-server/src/main/java/org/sonar/server/platform/db/EmbeddedDatabase.java index e2fc4250a7e..33d1f62d637 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/db/EmbeddedDatabase.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/db/EmbeddedDatabase.java @@ -20,6 +20,7 @@ package org.sonar.server.platform.db; import java.io.File; +import java.net.InetAddress; import java.sql.DriverManager; import java.sql.SQLException; import org.apache.commons.lang.StringUtils; @@ -27,7 +28,7 @@ import org.h2.Driver; import org.h2.tools.Server; import org.picocontainer.Startable; import org.sonar.api.config.Settings; -import org.sonar.api.utils.SonarException; +import org.sonar.api.utils.System2; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; @@ -44,11 +45,14 @@ import static org.sonar.process.ProcessProperties.PATH_DATA; public class EmbeddedDatabase implements Startable { private static final Logger LOG = Loggers.get(EmbeddedDatabase.class); + private final Settings settings; + private final System2 system2; private Server server; - public EmbeddedDatabase(Settings settings) { + public EmbeddedDatabase(Settings settings, System2 system2) { this.settings = settings; + this.system2 = system2; } @Override @@ -67,19 +71,23 @@ public class EmbeddedDatabase implements Startable { String user = getSetting(PROP_USER, PROP_USER_DEFAULT_VALUE); String password = getSetting(PROP_PASSWORD, PROP_PASSWORD_DEFAULT_VALUE); try { + // Db is used only by web server and compute engine. No need + // to make it accessible from outside. + system2.setProperty("h2.bindAddress", InetAddress.getLoopbackAddress().getHostAddress()); + if (url.contains("/mem:")) { - server = Server.createTcpServer("-tcpPort", port, "-tcpAllowOthers", "-baseDir", dbHome.getAbsolutePath()); + server = Server.createTcpServer("-tcpPort", port, "-baseDir", dbHome.getAbsolutePath()); } else { createDatabase(dbHome, user, password); - server = Server.createTcpServer("-tcpPort", port, "-tcpAllowOthers", "-ifExists", "-baseDir", dbHome.getAbsolutePath()); + server = Server.createTcpServer("-tcpPort", port, "-ifExists", "-baseDir", dbHome.getAbsolutePath()); } LOG.info("Starting embedded database on port " + server.getPort() + " with url " + url); server.start(); LOG.info("Embedded database started. Data stored in: " + dbHome.getAbsolutePath()); - } catch (Exception e) { - throw new SonarException("Unable to start database", e); + } catch (SQLException e) { + throw new IllegalStateException("Unable to start database", e); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/db/EmbeddedDatabaseFactory.java b/server/sonar-server/src/main/java/org/sonar/server/platform/db/EmbeddedDatabaseFactory.java index 86968d427dd..91c69ff80fc 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/db/EmbeddedDatabaseFactory.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/db/EmbeddedDatabaseFactory.java @@ -23,16 +23,21 @@ import com.google.common.annotations.VisibleForTesting; import org.picocontainer.Startable; import org.sonar.api.config.Settings; import org.sonar.api.database.DatabaseProperties; +import org.sonar.api.utils.System2; import static org.apache.commons.lang.StringUtils.startsWith; public class EmbeddedDatabaseFactory implements Startable { + private static final String URL_PREFIX = "jdbc:h2:tcp:"; + private final Settings settings; + private final System2 system2; private EmbeddedDatabase embeddedDatabase; - public EmbeddedDatabaseFactory(Settings settings) { + public EmbeddedDatabaseFactory(Settings settings, System2 system2) { this.settings = settings; + this.system2 = system2; } @Override @@ -56,6 +61,6 @@ public class EmbeddedDatabaseFactory implements Startable { @VisibleForTesting EmbeddedDatabase createEmbeddedDatabase() { - return new EmbeddedDatabase(settings); + return new EmbeddedDatabase(settings, system2); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/platform/db/EmbeddedDatabaseFactoryTest.java b/server/sonar-server/src/test/java/org/sonar/server/platform/db/EmbeddedDatabaseFactoryTest.java index e904bb5400f..0d48ed030c7 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/platform/db/EmbeddedDatabaseFactoryTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/platform/db/EmbeddedDatabaseFactoryTest.java @@ -23,6 +23,7 @@ import org.junit.Test; import org.sonar.api.config.Settings; import org.sonar.api.config.MapSettings; import org.sonar.api.database.DatabaseProperties; +import org.sonar.api.utils.System2; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -30,7 +31,8 @@ import static org.mockito.Mockito.verify; public class EmbeddedDatabaseFactoryTest { - Settings settings = new MapSettings(); + private Settings settings = new MapSettings(); + private System2 system2 = mock(System2.class); @Test public void should_start_and_stop_tcp_h2_database() { @@ -38,7 +40,7 @@ public class EmbeddedDatabaseFactoryTest { EmbeddedDatabase embeddedDatabase = mock(EmbeddedDatabase.class); - EmbeddedDatabaseFactory databaseFactory = new EmbeddedDatabaseFactory(settings) { + EmbeddedDatabaseFactory databaseFactory = new EmbeddedDatabaseFactory(settings, system2) { @Override EmbeddedDatabase createEmbeddedDatabase() { return embeddedDatabase; @@ -57,7 +59,7 @@ public class EmbeddedDatabaseFactoryTest { EmbeddedDatabase embeddedDatabase = mock(EmbeddedDatabase.class); - EmbeddedDatabaseFactory databaseFactory = new EmbeddedDatabaseFactory(settings) { + EmbeddedDatabaseFactory databaseFactory = new EmbeddedDatabaseFactory(settings, system2) { @Override EmbeddedDatabase createEmbeddedDatabase() { return embeddedDatabase; diff --git a/server/sonar-server/src/test/java/org/sonar/server/platform/db/EmbeddedDatabaseTest.java b/server/sonar-server/src/test/java/org/sonar/server/platform/db/EmbeddedDatabaseTest.java index e780f99092f..676431e04dd 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/platform/db/EmbeddedDatabaseTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/platform/db/EmbeddedDatabaseTest.java @@ -20,19 +20,25 @@ package org.sonar.server.platform.db; import java.io.IOException; +import java.net.InetAddress; import java.sql.DriverManager; import org.h2.Driver; import org.junit.After; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.DisableOnDebug; import org.junit.rules.ExpectedException; import org.junit.rules.TemporaryFolder; +import org.junit.rules.TestRule; import org.junit.rules.Timeout; import org.sonar.api.config.MapSettings; +import org.sonar.api.utils.System2; import org.sonar.api.utils.log.LogTester; import org.sonar.process.NetworkUtils; import static junit.framework.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.sonar.api.database.DatabaseProperties.PROP_EMBEDDED_PORT; import static org.sonar.api.database.DatabaseProperties.PROP_PASSWORD; import static org.sonar.api.database.DatabaseProperties.PROP_PASSWORD_DEFAULT_VALUE; @@ -43,6 +49,8 @@ import static org.sonar.process.ProcessProperties.PATH_DATA; public class EmbeddedDatabaseTest { + private static final String LOOPBACK_ADDRESS = InetAddress.getLoopbackAddress().getHostAddress(); + @Rule public ExpectedException expectedException = ExpectedException.none(); @Rule @@ -50,9 +58,11 @@ public class EmbeddedDatabaseTest { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); @Rule - public Timeout timeout = Timeout.seconds(30); + public TestRule safeguard = new DisableOnDebug(Timeout.seconds(30)); - private EmbeddedDatabase underTest; + private MapSettings settings = new MapSettings(); + private System2 system2 = mock(System2.class); + private EmbeddedDatabase underTest = new EmbeddedDatabase(settings, system2); @After public void tearDown() throws Exception { @@ -63,8 +73,6 @@ public class EmbeddedDatabaseTest { @Test public void start_fails_with_IAE_if_property_Data_Path_is_not_set() { - EmbeddedDatabase underTest = new EmbeddedDatabase(new MapSettings()); - expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Missing property " + PATH_DATA); @@ -73,8 +81,7 @@ public class EmbeddedDatabaseTest { @Test public void start_fails_with_IAE_if_property_Data_Path_is_empty() { - EmbeddedDatabase underTest = new EmbeddedDatabase(new MapSettings() - .setProperty(PATH_DATA, "")); + settings.setProperty(PATH_DATA, ""); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Missing property " + PATH_DATA); @@ -84,8 +91,7 @@ public class EmbeddedDatabaseTest { @Test public void start_fails_with_IAE_if_JDBC_URL_settings_is_not_set() throws IOException { - EmbeddedDatabase underTest = new EmbeddedDatabase(new MapSettings() - .setProperty(PATH_DATA, temporaryFolder.newFolder().getAbsolutePath())); + settings.setProperty(PATH_DATA, temporaryFolder.newFolder().getAbsolutePath()); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Missing property " + PROP_URL); @@ -95,9 +101,9 @@ public class EmbeddedDatabaseTest { @Test public void start_fails_with_IAE_if_embedded_port_settings_is_not_set() throws IOException { - EmbeddedDatabase underTest = new EmbeddedDatabase(new MapSettings() + settings .setProperty(PATH_DATA, temporaryFolder.newFolder().getAbsolutePath()) - .setProperty(PROP_URL, "jdbc url")); + .setProperty(PROP_URL, "jdbc url"); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Missing property " + PROP_EMBEDDED_PORT); @@ -108,10 +114,10 @@ public class EmbeddedDatabaseTest { @Test public void start_ignores_URL_to_create_database_and_uses_default_username_and_password_when_then_are_not_set() throws IOException { int port = NetworkUtils.freePort(); - underTest = new EmbeddedDatabase(new MapSettings() + settings .setProperty(PATH_DATA, temporaryFolder.newFolder().getAbsolutePath()) .setProperty(PROP_URL, "jdbc url") - .setProperty(PROP_EMBEDDED_PORT, "" + port)); + .setProperty(PROP_EMBEDDED_PORT, "" + port); underTest.start(); @@ -119,29 +125,32 @@ public class EmbeddedDatabaseTest { } @Test - public void start_creates_db_with_specified_user_and_password() throws IOException { + public void start_creates_db_and_adds_tcp_listener() throws IOException { int port = NetworkUtils.freePort(); - underTest = new EmbeddedDatabase(new MapSettings() + settings .setProperty(PATH_DATA, temporaryFolder.newFolder().getAbsolutePath()) .setProperty(PROP_URL, "jdbc url") .setProperty(PROP_EMBEDDED_PORT, "" + port) .setProperty(PROP_USER, "foo") - .setProperty(PROP_PASSWORD, "bar")); + .setProperty(PROP_PASSWORD, "bar"); underTest.start(); checkDbIsUp(port, "foo", "bar"); + + // H2 listens on loopback address only + verify(system2).setProperty("h2.bindAddress", LOOPBACK_ADDRESS); } @Test public void start_supports_in_memory_H2_JDBC_URL() throws IOException { int port = NetworkUtils.freePort(); - underTest = new EmbeddedDatabase(new MapSettings() + settings .setProperty(PATH_DATA, temporaryFolder.newFolder().getAbsolutePath()) .setProperty(PROP_URL, "jdbc:h2:mem:sonar") .setProperty(PROP_EMBEDDED_PORT, "" + port) .setProperty(PROP_USER, "foo") - .setProperty(PROP_PASSWORD, "bar")); + .setProperty(PROP_PASSWORD, "bar"); underTest.start(); @@ -150,7 +159,7 @@ public class EmbeddedDatabaseTest { private void checkDbIsUp(int port, String user, String password) { try { - String driverUrl = String.format("jdbc:h2:tcp://localhost:%d/sonar;USER=%s;PASSWORD=%s", port, user, password); + String driverUrl = String.format("jdbc:h2:tcp://%s:%d/sonar;USER=%s;PASSWORD=%s", LOOPBACK_ADDRESS, port, user, password); DriverManager.registerDriver(new Driver()); DriverManager.getConnection(driverUrl).close(); } catch (Exception ex) { diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/utils/System2.java b/sonar-plugin-api/src/main/java/org/sonar/api/utils/System2.java index 90010346158..ef8fa8e878d 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/utils/System2.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/utils/System2.java @@ -92,6 +92,15 @@ public class System2 { return System.getProperty(key); } + /** + * Shortcut for {@code System{@link #setProperty(String, String)}} + * @since 6.4 + */ + public System2 setProperty(String key, String value) { + System.setProperty(key, value); + return this; + } + /** * Shortcut for {@link System#getenv()} */ diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/utils/System2Test.java b/sonar-plugin-api/src/test/java/org/sonar/api/utils/System2Test.java index a1c658bf60a..f1e1c7b24fd 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/utils/System2Test.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/utils/System2Test.java @@ -19,21 +19,21 @@ */ package org.sonar.api.utils; -import org.apache.commons.lang.SystemUtils; -import org.junit.Test; - import java.io.Closeable; import java.io.IOException; import java.util.Map; import java.util.Properties; import java.util.TimeZone; +import org.apache.commons.lang.SystemUtils; +import org.junit.Test; +import static java.util.UUID.randomUUID; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; public class System2Test { @Test - public void testNow() throws Exception { + public void testNow() { long start = System.currentTimeMillis(); long now = System2.INSTANCE.now(); long end = System.currentTimeMillis(); @@ -41,25 +41,33 @@ public class System2Test { } @Test - public void testProperties() throws Exception { + public void testProperties() { Properties expected = System.getProperties(); assertThat(System2.INSTANCE.properties()).isNotNull().isEqualTo(expected); } @Test - public void testProperty() throws Exception { + public void testProperty() { String expected = System.getProperty("java.version"); assertThat(System2.INSTANCE.property("java.version")).isNotNull().isEqualTo(expected); } @Test - public void testEnvVariables() throws Exception { + public void testSetProperty() { + String key = "System2Test.testSetProperty"; + String value = randomUUID().toString(); + System2.INSTANCE.setProperty(key, value); + assertThat(System2.INSTANCE.property(key)).isEqualTo(value); + } + + @Test + public void testEnvVariables() { Map expected = System.getenv(); assertThat(System2.INSTANCE.envVariables()).isNotNull().isEqualTo(expected); } @Test - public void testEnvVariable() throws Exception { + public void testEnvVariable() { // assume that there's at least one env variable if (System.getenv().isEmpty()) { fail("Test can't succeed because there are no env variables. How is it possible ?"); @@ -71,12 +79,12 @@ public class System2Test { } @Test - public void testIsOsWindows() throws Exception { + public void testIsOsWindows() { assertThat(System2.INSTANCE.isOsWindows()).isEqualTo(SystemUtils.IS_OS_WINDOWS); } @Test - public void testIsJavaAtLeast17() throws Exception { + public void testIsJavaAtLeast17() { if (SystemUtils.IS_JAVA_1_6) { assertThat(System2.INSTANCE.isJavaAtLeast17()).isFalse(); } else { @@ -85,13 +93,13 @@ public class System2Test { } @Test - public void testPrintln() throws Exception { + public void testPrintln() { // well, how to assert that ? Adding a System3 dependency to System2 ? :-) System2.INSTANCE.println("foo"); } @Test - public void testGetResource() throws Exception { + public void testGetResource() { String name = "META-INF/MANIFEST.MF"; assertThat(System2.INSTANCE.getResource(name)).isEqualTo(getClass().getResource(name)); } @@ -100,6 +108,7 @@ public class System2Test { public void close() { class MyCloseable implements Closeable { boolean isClosed = false; + @Override public void close() throws IOException { isClosed = true; -- 2.39.5