diff --git a/core/src/main/java/apoc/load/util/JdbcUtil.java b/core/src/main/java/apoc/load/util/JdbcUtil.java index 7858bdcd6b..1bbb555bb3 100644 --- a/core/src/main/java/apoc/load/util/JdbcUtil.java +++ b/core/src/main/java/apoc/load/util/JdbcUtil.java @@ -90,4 +90,8 @@ public static String getSqlOrKey(String sqlOrKey) { ? sqlOrKey : Util.getLoadUrlByConfigFile(LOAD_TYPE, sqlOrKey, "sql").orElse("SELECT * FROM " + sqlOrKey); } + + public static String obfuscateJdbcUrl(String query) { + return query.replaceAll("(jdbc:[^:]+://)([^\\s\\\"']+)", "$1*******"); + } } diff --git a/full/src/main/java/apoc/load/Jdbc.java b/full/src/main/java/apoc/load/Jdbc.java index 2bd2d4cd2d..6204a91770 100644 --- a/full/src/main/java/apoc/load/Jdbc.java +++ b/full/src/main/java/apoc/load/Jdbc.java @@ -131,17 +131,7 @@ private Stream executeQuery( throw sqle; } } catch (Exception e) { - log.error(String.format("Cannot execute SQL statement `%s`.%nError:%n%s", query, e.getMessage()), e); - String errorMessage = "Cannot execute SQL statement `%s`.%nError:%n%s"; - if (e.getMessage().contains("No suitable driver")) - errorMessage = "Cannot execute SQL statement `%s`.%nError:%n%s%n%s"; - throw new RuntimeException( - String.format( - errorMessage, - query, - e.getMessage(), - "Please download and copy the JDBC driver into $NEO4J_HOME/plugins,more details at https://neo4j-contrib.github.io/neo4j-apoc-procedures/#_load_jdbc_resources"), - e); + throw logsErrorAndThrowsException(e, query, log); } } @@ -182,20 +172,28 @@ private Stream executeUpdate( throw sqle; } } catch (Exception e) { - log.error(String.format("Cannot execute SQL statement `%s`.%nError:%n%s", query, e.getMessage()), e); - String errorMessage = "Cannot execute SQL statement `%s`.%nError:%n%s"; - if (e.getMessage().contains("No suitable driver")) - errorMessage = "Cannot execute SQL statement `%s`.%nError:%n%s%n%s"; - throw new RuntimeException( - String.format( - errorMessage, - query, - e.getMessage(), - "Please download and copy the JDBC driver into $NEO4J_HOME/plugins,more details at https://neo4j-contrib.github.io/neo4j-apoc-procedures/#_load_jdbc_resources"), - e); + throw logsErrorAndThrowsException(e, query, log); } } + private static RuntimeException logsErrorAndThrowsException(Exception e, String query, Log log) { + String errorMessage = "Cannot execute SQL statement `%s`.%nError:%n%s"; + String exceptionMsg = e.getMessage(); + if (e.getMessage().contains("No suitable driver")) { + errorMessage = "Cannot execute SQL statement `%s`.%nError:%n%s%n%s"; + exceptionMsg = obfuscateJdbcUrl(e.getMessage()); + } + Exception ex = new Exception(exceptionMsg); + log.error(String.format("Cannot execute SQL statement `%s`.%nError:%n%s", query, exceptionMsg), ex); + return new RuntimeException( + String.format( + errorMessage, + query, + exceptionMsg, + "Please download and copy the JDBC driver into $NEO4J_HOME/plugins, more details at https://neo4j-contrib.github.io/neo4j-apoc-procedures/#_load_jdbc_resources"), + ex); + } + static void closeIt(Log log, AutoCloseable... closeables) { for (AutoCloseable c : closeables) { try { diff --git a/full/src/test/java/apoc/load/JdbcTest.java b/full/src/test/java/apoc/load/JdbcTest.java index 43d3fd798a..f7db48046a 100644 --- a/full/src/test/java/apoc/load/JdbcTest.java +++ b/full/src/test/java/apoc/load/JdbcTest.java @@ -19,10 +19,13 @@ package apoc.load; import static apoc.ApocConfig.apocConfig; +import static apoc.util.ExtendedTestUtil.assertFails; +import static apoc.util.ExtendedTestUtil.getLogFileContent; import static apoc.util.MapUtil.map; import static apoc.util.TestUtil.testCall; import static apoc.util.TestUtil.testResult; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import apoc.periodic.Periodic; import apoc.util.TestUtil; @@ -39,17 +42,23 @@ import java.util.Map; import org.junit.*; import org.junit.rules.ExpectedException; +import org.junit.rules.TemporaryFolder; import org.junit.rules.TestName; +import org.neo4j.configuration.GraphDatabaseSettings; +import org.neo4j.dbms.api.DatabaseManagementService; +import org.neo4j.graphdb.GraphDatabaseService; import org.neo4j.graphdb.QueryExecutionException; import org.neo4j.internal.helpers.collection.Iterators; import org.neo4j.internal.helpers.collection.MapUtil; -import org.neo4j.test.rule.DbmsRule; -import org.neo4j.test.rule.ImpermanentDbmsRule; +import org.neo4j.test.TestDatabaseManagementServiceBuilder; public class JdbcTest extends AbstractJdbcTest { @Rule - public DbmsRule db = new ImpermanentDbmsRule(); + public TemporaryFolder STORE_DIR = new TemporaryFolder(); + + private GraphDatabaseService db; + private DatabaseManagementService dbms; private Connection conn; @@ -63,6 +72,9 @@ public class JdbcTest extends AbstractJdbcTest { @Before public void setUp() throws Exception { + dbms = new TestDatabaseManagementServiceBuilder(STORE_DIR.getRoot().toPath()).build(); + db = dbms.database(GraphDatabaseSettings.DEFAULT_DATABASE_NAME); + apocConfig().setProperty("apoc.jdbc.derby.url", "jdbc:derby:derbyDB"); apocConfig().setProperty("apoc.jdbc.test.sql", "SELECT * FROM PERSON"); apocConfig().setProperty("apoc.jdbc.testparams.sql", "SELECT * FROM PERSON WHERE NAME = ?"); @@ -92,7 +104,7 @@ public void tearDown() throws SQLException { System.clearProperty("derby.connection.requireAuthentication"); System.clearProperty("derby.user.apoc"); - db.shutdown(); + dbms.shutdown(); } @Test @@ -133,6 +145,23 @@ public void testLoadJdbcParams() throws Exception { (row) -> assertResult(row)); } + @Test + public void testExceptionAndLogWithObfuscatedUrl() { + String url = "jdbc:ajeje://localhost:3306/data_mart?user=root&password=root"; + String errorMsgWithObfuscatedUrl = "No suitable driver found for jdbc:ajeje://*******"; + + // obfuscated exception + assertFails( + db, + "CALL apoc.load.jdbc($url,'SELECT * FROM PERSON WHERE NAME = ?',['John'])", + Map.of("url", url), + errorMsgWithObfuscatedUrl); + + // obfuscated log in `debug.log` + String fileContent = getLogFileContent(); + assertTrue("Actual log content is: " + fileContent, fileContent.contains(errorMsgWithObfuscatedUrl)); + } + @Test public void testLoadJdbcParamsWithConfigLocalDateTime() throws Exception { testCall( diff --git a/full/src/test/java/apoc/load/LoadLdapTest.java b/full/src/test/java/apoc/load/LoadLdapTest.java index ec6e6ebe49..e7b8e04f16 100644 --- a/full/src/test/java/apoc/load/LoadLdapTest.java +++ b/full/src/test/java/apoc/load/LoadLdapTest.java @@ -19,20 +19,17 @@ package apoc.load; import static apoc.ApocConfig.apocConfig; +import static apoc.util.ExtendedTestUtil.getLogFileContent; import static apoc.util.TestUtil.testCall; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import apoc.util.FileUtils; import apoc.util.TestUtil; import com.novell.ldap.LDAPEntry; import com.novell.ldap.LDAPSearchResults; import com.unboundid.ldap.sdk.LDAPConnection; -import java.io.File; -import java.io.IOException; -import java.nio.file.Files; import java.util.List; import java.util.Map; import org.junit.AfterClass; @@ -117,15 +114,6 @@ public void testLoadLDAPWithApocConfigWithoutDotBeforeLdapKey() { assertTrue(getLogFileContent().contains(logWarn)); } - private static String getLogFileContent() { - try { - File logFile = new File(FileUtils.getLogDirectory(), "debug.log"); - return Files.readString(logFile.toPath()); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - private void testWithStringConfigCommon(String key) { // set a config `key=localhost:port dns pwd` String ldapValue = diff --git a/full/src/test/java/apoc/periodic/PeriodicExtendedTest.java b/full/src/test/java/apoc/periodic/PeriodicExtendedTest.java index 5bfd1aab08..d706b725ad 100644 --- a/full/src/test/java/apoc/periodic/PeriodicExtendedTest.java +++ b/full/src/test/java/apoc/periodic/PeriodicExtendedTest.java @@ -32,7 +32,6 @@ import apoc.nlp.gcp.GCPProcedures; import apoc.nodes.NodesExtended; import apoc.util.TestUtil; -import java.sql.SQLException; import java.util.Collections; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -222,27 +221,6 @@ public void testIterateErrors() { }); } - @Test - public void testIterateJDBC() { - TestUtil.ignoreException( - () -> { - testResult( - db, - "CALL apoc.periodic.iterate('call apoc.load.jdbc(\"jdbc:mysql://localhost:3306/northwind?user=root\",\"customers\")', 'create (c:Customer) SET c += $row', {batchSize:10,parallel:true})", - result -> { - Map row = Iterators.single(result); - assertEquals(3L, row.get("batches")); - assertEquals(29L, row.get("total")); - }); - - testCall( - db, - "MATCH (p:Customer) return count(p) as count", - row -> assertEquals(29L, row.get("count"))); - }, - SQLException.class); - } - @Test public void testRock_n_roll_while() { // setup diff --git a/full/src/test/java/apoc/util/ExtendedTestUtil.java b/full/src/test/java/apoc/util/ExtendedTestUtil.java index dc3a8ba34c..91f712677d 100644 --- a/full/src/test/java/apoc/util/ExtendedTestUtil.java +++ b/full/src/test/java/apoc/util/ExtendedTestUtil.java @@ -1,8 +1,14 @@ package apoc.util; +import static apoc.util.TestUtil.testCall; import static apoc.util.TestUtil.testCallAssertions; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.neo4j.test.assertion.Assert.assertEventually; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; import java.util.Collections; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -67,4 +73,23 @@ public static void testResultEventually( timeout, TimeUnit.SECONDS); } + + public static void assertFails( + GraphDatabaseService db, String query, Map params, String expectedErrMsg) { + try { + testCall(db, query, params, r -> fail("Should fail due to " + expectedErrMsg)); + } catch (Exception e) { + String actualErrMsg = e.getMessage(); + assertTrue("Actual err. message is: " + actualErrMsg, actualErrMsg.contains(expectedErrMsg)); + } + } + + public static String getLogFileContent() { + try { + File logFile = new File(FileUtils.getLogDirectory(), "debug.log"); + return Files.readString(logFile.toPath()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } }