Treat unsupported URLs as specified in Driver#connect to avoid overwr…

…iting the "real" exception (#115) * Treat unsupported URLs as specified in `Driver#connect` to avoid overwriting the "real" exception * Drop documentation on URL being changed * - 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 <[email protected]> --------- Signed-off-by: Phillip Ross <[email protected]> Co-authored-by: Phillip Ross <[email protected]>
postgis · Dec 7, 2023 · c2284e7 · c2284e7
1 parent f6ac473
commit c2284e7
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 63 deletions.
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
@@ -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
@@ -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
@@ -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
@@ -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
-     * 
-     * @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 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)) {
+            url = mangleURL(url);
+        } else {
+            return null;
+        }
         Connection result = super.connect(url, info);
         typesAdder.addGT(result, useLW(result));
         return result;
@@ -181,19 +183,17 @@ 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) {
-        try {
-            url = mangleURL(url);
-        } catch (SQLException e) {
+        String mangledURL = mangleURL(url);
+        if (mangledURL == null) {
             return false;
+        } else {
+            return super.acceptsURL(mangledURL);
         }
-        return super.acceptsURL(url);
     }
 
 
@@ -254,16 +254,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
@@ -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());
+        }
+    }
+
+}