From 7c1717fb47218c801bf2433b6781667a3cd893ec Mon Sep 17 00:00:00 2001 From: Simon Ochsenreither Date: Sun, 23 Jan 2022 01:56:21 +0100 Subject: [PATCH 1/3] Treat unsupported URLs as specified in `Driver#connect` to avoid overwriting the "real" exception --- .../postgis/jdbc/java2d/Java2DWrapper.java | 21 ++++-------- .../net/postgis/jdbc/jts/JtsGisWrapper.java | 21 ++++-------- .../java/net/postgis/jdbc/jts/JtsWrapper.java | 21 ++++-------- .../java/net/postgis/jdbc/DriverWrapper.java | 31 ++++++++---------- .../jdbc/DriverConnectBehaviorTest.java | 32 +++++++++++++++++++ 5 files changed, 66 insertions(+), 60 deletions(-) create mode 100644 postgis-jdbc/src/test/java/net/postgis/jdbc/DriverConnectBehaviorTest.java diff --git a/postgis-jdbc-java2d/src/main/java/net/postgis/jdbc/java2d/Java2DWrapper.java b/postgis-jdbc-java2d/src/main/java/net/postgis/jdbc/java2d/Java2DWrapper.java index e54fa77..e3f9653 100644 --- a/postgis-jdbc-java2d/src/main/java/net/postgis/jdbc/java2d/Java2DWrapper.java +++ b/postgis-jdbc-java2d/src/main/java/net/postgis/jdbc/java2d/Java2DWrapper.java @@ -106,15 +106,12 @@ public static void addGISTypes(PGConnection pgconn) throws SQLException { * Mangles the PostGIS URL to return the original PostGreSQL URL * * @param url String containing the url to be "mangled" - * @return "mangled" string - * @throws SQLException when a SQLException occurs + * @return "mangled" string or null if the URL is unsupported */ - public static String mangleURL(String url) throws SQLException { - if (url.startsWith(POSTGIS_PROTOCOL)) { - return POSTGRES_PROTOCOL + url.substring(POSTGIS_PROTOCOL.length()); - } else { - throw new SQLException("Unknown protocol or subprotocol in url " + url); - } + public static String mangleURL(String url) { + return url.startsWith(POSTGIS_PROTOCOL) + ? POSTGRES_PROTOCOL + url.substring(POSTGIS_PROTOCOL.length()) + : null; } /** @@ -128,12 +125,8 @@ public static String mangleURL(String url) throws SQLException { * @return true if this driver accepts the given URL */ public boolean acceptsURL(String url) { - try { - url = mangleURL(url); - } catch (SQLException e) { - return false; - } - return super.acceptsURL(url); + url = mangleURL(url); + return url != null && super.acceptsURL(url); } /** diff --git a/postgis-jdbc-jts/src/main/java/net/postgis/jdbc/jts/JtsGisWrapper.java b/postgis-jdbc-jts/src/main/java/net/postgis/jdbc/jts/JtsGisWrapper.java index 5c87b58..a71a4ec 100644 --- a/postgis-jdbc-jts/src/main/java/net/postgis/jdbc/jts/JtsGisWrapper.java +++ b/postgis-jdbc-jts/src/main/java/net/postgis/jdbc/jts/JtsGisWrapper.java @@ -100,15 +100,12 @@ public static void addGISTypes(PGConnection pgconn) throws SQLException { * Mangles the PostGIS URL to return the original PostGreSQL URL * * @param url String containing the url to be "mangled" - * @return "mangled" string - * @throws SQLException when a SQLException occurs + * @return "mangled" string or null if the URL is unsupported */ - public static String mangleURL(String url) throws SQLException { - if (url.startsWith(POSTGIS_PROTOCOL)) { - return POSTGRES_PROTOCOL + url.substring(POSTGIS_PROTOCOL.length()); - } else { - throw new SQLException("Unknown protocol or subprotocol in url " + url); - } + public static String mangleURL(String url) { + return url.startsWith(POSTGIS_PROTOCOL) + ? POSTGRES_PROTOCOL + url.substring(POSTGIS_PROTOCOL.length()) + : null; } /** @@ -122,12 +119,8 @@ public static String mangleURL(String url) throws SQLException { * @return true if this driver accepts the given URL */ public boolean acceptsURL(String url) { - try { - url = mangleURL(url); - } catch (SQLException e) { - return false; - } - return super.acceptsURL(url); + url = mangleURL(url); + return url != null && super.acceptsURL(url); } /** diff --git a/postgis-jdbc-jts/src/main/java/net/postgis/jdbc/jts/JtsWrapper.java b/postgis-jdbc-jts/src/main/java/net/postgis/jdbc/jts/JtsWrapper.java index b0ca24a..54f3de1 100644 --- a/postgis-jdbc-jts/src/main/java/net/postgis/jdbc/jts/JtsWrapper.java +++ b/postgis-jdbc-jts/src/main/java/net/postgis/jdbc/jts/JtsWrapper.java @@ -109,15 +109,12 @@ public static void addGISTypes(PGConnection pgconn) throws SQLException { * Mangles the PostGIS URL to return the original PostGreSQL URL * * @param url String containing the url to be "mangled" - * @return "mangled" string - * @throws SQLException when a SQLException occurs + * @return "mangled" string or null if the URL is unsupported */ - public static String mangleURL(String url) throws SQLException { - if (url.startsWith(POSTGIS_PROTOCOL)) { - return POSTGRES_PROTOCOL + url.substring(POSTGIS_PROTOCOL.length()); - } else { - throw new SQLException("Unknown protocol or subprotocol in url " + url); - } + public static String mangleURL(String url) { + return url.startsWith(POSTGIS_PROTOCOL) + ? POSTGRES_PROTOCOL + url.substring(POSTGIS_PROTOCOL.length()) + : null; } /** @@ -128,12 +125,8 @@ public static String mangleURL(String url) throws SQLException { * @return true if this driver accepts the given URL */ public boolean acceptsURL(String url) { - try { - url = mangleURL(url); - } catch (SQLException e) { - return false; - } - return super.acceptsURL(url); + url = mangleURL(url); + return url != null && super.acceptsURL(url); } /** diff --git a/postgis-jdbc/src/main/java/net/postgis/jdbc/DriverWrapper.java b/postgis-jdbc/src/main/java/net/postgis/jdbc/DriverWrapper.java index 01bb858..a2640b0 100644 --- a/postgis-jdbc/src/main/java/net/postgis/jdbc/DriverWrapper.java +++ b/postgis-jdbc/src/main/java/net/postgis/jdbc/DriverWrapper.java @@ -145,18 +145,20 @@ private static TypesAdder loadTypesAdder(final String version) throws SQLExcepti /** * Creates a postgresql connection, and then adds the PostGIS data types to it calling addpgtypes(). * - * A side-effect of this method is that the specified url parameter may be be changed + * A side-effect of this method is that the specified url parameter may be changed * * @param url the URL of the database to connect to (may be changed as a side-effect of this method) * @param info a list of arbitrary tag/value pairs as connection arguments - * @return a connection to the URL or null if it isnt us + * @return a connection to the URL or null if the driver does not support the subprotocol specified in the URL * @exception SQLException if a database access error occurs * * @see java.sql.Driver#connect * @see org.postgresql.Driver */ public java.sql.Connection connect(String url, final Properties info) throws SQLException { - url = mangleURL(url); + if (!acceptsURL(url)) { + return null; + } Connection result = super.connect(url, info); typesAdder.addGT(result, useLW(result)); return result; @@ -188,12 +190,8 @@ protected boolean useLW(final Connection result) { * @return true if this driver accepts the given URL */ public boolean acceptsURL(String url) { - try { - url = mangleURL(url); - } catch (SQLException e) { - return false; - } - return super.acceptsURL(url); + url = mangleURL(url); + return url != null && super.acceptsURL(url); } @@ -254,16 +252,13 @@ public static void addGISTypes72(final org.postgresql.PGConnection pgconn) throw * Mangles the PostGIS URL to return the original PostGreSQL URL * * @param url String containing the url to be "mangled" - * @return "mangled" string - * @throws SQLException when a SQLException occurs + * @return "mangled" string or null if the URL is unsupported */ - protected String mangleURL(final String url) throws SQLException { - String myProgo = getProtoString(); - if (url.startsWith(myProgo)) { - return POSTGRES_PROTOCOL + url.substring(myProgo.length()); - } else { - throw new SQLException("Unknown protocol or subprotocol in url " + url); - } + protected String mangleURL(final String url) { + String myProto = getProtoString(); + return url.startsWith(myProto) + ? POSTGRES_PROTOCOL + url.substring(myProto.length()) + : null; } diff --git a/postgis-jdbc/src/test/java/net/postgis/jdbc/DriverConnectBehaviorTest.java b/postgis-jdbc/src/test/java/net/postgis/jdbc/DriverConnectBehaviorTest.java new file mode 100644 index 0000000..a81aa47 --- /dev/null +++ b/postgis-jdbc/src/test/java/net/postgis/jdbc/DriverConnectBehaviorTest.java @@ -0,0 +1,32 @@ +package net.postgis.jdbc; + +import org.junit.Assert; +import org.junit.Test; + +import java.sql.DriverManager; +import java.sql.SQLException; + +public class DriverConnectBehaviorTest { + + @Test + public void testThatPostGisDoesNotOverwriteSavedExceptionForUnsupportedConnectionString() { + try { + DriverManager.getConnection("jdbc:missing"); + } catch (SQLException e) { + // This should not be "Unknown protocol or subprotocol in url jdbc:missing", which + // would indicate that PostGIS threw an exception instead of returning `null` from + // the `connect` method for an unsupported connection string. + // (This is documented in `java.sql.Driver.connect`.) + // + // The former behavior is not desirable as throwing an exception causes a previously + // saved exception from a "better fitting" driver to be overwritten by PostGis, despite + // PostGis not actually being able to handle the connection. + // + // (Imagine an Oracle connection string with a wrong password, in which the Oracle + // driver's exception regarding the wrong password would be replaced with a generic + // nonsensical PostGis exception.) + Assert.assertEquals("No suitable driver found for jdbc:missing", e.getMessage()); + } + } + +} From 19a1c461acf9da5dcf5b459a81f4f0412c14cb40 Mon Sep 17 00:00:00 2001 From: Simon Ochsenreither Date: Sun, 23 Jan 2022 01:58:35 +0100 Subject: [PATCH 2/3] Drop documentation on URL being changed --- .../src/main/java/net/postgis/jdbc/DriverWrapper.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/postgis-jdbc/src/main/java/net/postgis/jdbc/DriverWrapper.java b/postgis-jdbc/src/main/java/net/postgis/jdbc/DriverWrapper.java index a2640b0..1324416 100644 --- a/postgis-jdbc/src/main/java/net/postgis/jdbc/DriverWrapper.java +++ b/postgis-jdbc/src/main/java/net/postgis/jdbc/DriverWrapper.java @@ -145,9 +145,7 @@ private static TypesAdder loadTypesAdder(final String version) throws SQLExcepti /** * Creates a postgresql connection, and then adds the PostGIS data types to it calling addpgtypes(). * - * A side-effect of this method is that the specified url parameter may be changed - * - * @param url the URL of the database to connect to (may be changed as a side-effect of this method) + * @param url the URL of the database to connect to * @param info a list of arbitrary tag/value pairs as connection arguments * @return a connection to the URL or null if the driver does not support the subprotocol specified in the URL * @exception SQLException if a database access error occurs @@ -183,10 +181,8 @@ protected boolean useLW(final Connection result) { /** * Check whether the driver thinks he can handle the given URL. * - * A side-effect of this method is that the specified url parameter may be be changed - * * @see java.sql.Driver#acceptsURL - * @param url the URL of the driver (may be changed as a side-effect of this method) + * @param url the URL of the driver * @return true if this driver accepts the given URL */ public boolean acceptsURL(String url) { From d8bed3b0df18fdb573b76f777b43e80a900897ca Mon Sep 17 00:00:00 2001 From: Phillip Ross Date: Thu, 7 Dec 2023 17:08:47 -0500 Subject: [PATCH 3/3] - FIX: connect() method modification to retain "mangled" url for connection - FIX: acceptsURL() method rework to avoid potentially invoking super method with null param Signed-off-by: Phillip Ross --- .../main/java/net/postgis/jdbc/DriverWrapper.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/postgis-jdbc/src/main/java/net/postgis/jdbc/DriverWrapper.java b/postgis-jdbc/src/main/java/net/postgis/jdbc/DriverWrapper.java index 1324416..615d049 100644 --- a/postgis-jdbc/src/main/java/net/postgis/jdbc/DriverWrapper.java +++ b/postgis-jdbc/src/main/java/net/postgis/jdbc/DriverWrapper.java @@ -154,7 +154,9 @@ private static TypesAdder loadTypesAdder(final String version) throws SQLExcepti * @see org.postgresql.Driver */ public java.sql.Connection connect(String url, final Properties info) throws SQLException { - if (!acceptsURL(url)) { + if (acceptsURL(url)) { + url = mangleURL(url); + } else { return null; } Connection result = super.connect(url, info); @@ -186,8 +188,12 @@ protected boolean useLW(final Connection result) { * @return true if this driver accepts the given URL */ public boolean acceptsURL(String url) { - url = mangleURL(url); - return url != null && super.acceptsURL(url); + String mangledURL = mangleURL(url); + if (mangledURL == null) { + return false; + } else { + return super.acceptsURL(mangledURL); + } }