Merge branch 'refs/heads/feature/urlparser-improve3-pr2' into feature…

…/urlparser-improve3-pr4 # Conflicts: # README.md # pgjdbc/src/main/java/org/postgresql/Driver.java # pgjdbc/src/test/java/org/postgresql/jdbcurlresolver/PgPassParserTest.java # pgjdbc/src/test/java/org/postgresql/test/jdbc2/PGPropertyTest.java # pgjdbc/src/test/java/org/postgresql/test/jdbc2/optional/BaseDataSourceFailoverUrlsTest.java # pgjdbc/src/testFixtures/java/org/postgresql/test/TestUtil.java
pgjdbc · May 11, 2024 · 9813dbf · 9813dbf
2 parents 4956902 + a913205
commit 9813dbf
Show file tree

Hide file tree

Showing 32 changed files with 2,026 additions and 905 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 ## [Unreleased]
 
 ### Changed
+- Parsing and resolving JDBC URL is more compliant with libpq. "user" and "password" are supported: "jdbc:postgresql://[[user][:password]@][host1]...". Connection properties come from different sources. Override order is more clear: 1) URL arguments (values after "?" mark) 2) URL values (values before "?" mark) 3) Properties given to DriverManager.getConnection() 4) values provided by "service" (from resource .pg_service.conf) 5) values in Java System Properties 6) values in Operating System environment 7) values from driverconfig file(s) (org/postgresql/driverconfig.properties) 8) global defaults (four: host, port, user, dbname)
+
 ### Added
 ### Fixed
 

diff --git a/README.md b/README.md
diff --git a/benchmarks/src/jmh/java/org/postgresql/util/ConnectionUtil.java b/benchmarks/src/jmh/java/org/postgresql/util/ConnectionUtil.java
@@ -61,7 +61,7 @@ public static Properties getProperties() {
 
     PGProperty.USER.set(properties, getUser());
     PGProperty.PASSWORD.set(properties, getPassword());
-    PGProperty.PG_PORT.set(properties, getPort());
+    PGProperty.PORT.set(properties, getPort());
     properties.setProperty("database", getDatabase());
     properties.setProperty("server", getServer());
 

diff --git a/packaging/rpm/postgresql-jdbc.spec.tpl b/packaging/rpm/postgresql-jdbc.spec.tpl
@@ -113,8 +113,10 @@ find -type f \( -name "*.jar" -or -name "*.class" \) | xargs rm -f
 rm src/test/java/org/postgresql/test/jdbc2/DriverTest.java \
    src/test/java/org/postgresql/util/OSUtilTest.java \
    src/test/java/org/postgresql/util/StubEnvironmentAndProperties.java \
+   src/test/java/org/postgresql/jdbcurlresolver/JdbcUrlResolverTest.java \
    src/test/java/org/postgresql/jdbcurlresolver/PgPassParserTest.java \
-   src/test/java/org/postgresql/jdbcurlresolver/PgServiceConfParserTest.java
+   src/test/java/org/postgresql/jdbcurlresolver/PgServiceConfParserTest.java \
+   src/test/java/org/postgresql/jdbcurlresolver/Utils.java
 
 # compat symlink: requested by dtardon (libreoffice), reverts part of
 # 0af97ce32de877 commit.

diff --git a/pgjdbc/src/main/java/org/postgresql/Driver.java b/pgjdbc/src/main/java/org/postgresql/Driver.java
@@ -9,12 +9,10 @@
 
 import org.postgresql.jdbc.PgConnection;
 import org.postgresql.jdbc.ResourceLock;
-import org.postgresql.jdbcurlresolver.PgPassParser;
-import org.postgresql.jdbcurlresolver.PgServiceConfParser;
+import org.postgresql.jdbcurlresolver.JdbcUrlResolver;
 import org.postgresql.util.DriverInfo;
 import org.postgresql.util.GT;
 import org.postgresql.util.HostSpec;
-import org.postgresql.util.PGPropertyUtil;
 import org.postgresql.util.PSQLException;
 import org.postgresql.util.PSQLState;
 import org.postgresql.util.SharedTimer;
@@ -140,13 +138,6 @@ private static <T> T doPrivileged(PrivilegedExceptionAction<T> action) throws Th
   private Properties loadDefaultProperties() throws IOException {
     Properties merged = new Properties();
 
-    try {
-      PGProperty.USER.set(merged, System.getProperty("user.name"));
-    } catch (SecurityException se) {
-      // We're just trying to set a default, so if we can't
-      // it's not a big deal.
-    }
-
     // If we are loaded by the bootstrap classloader, getClassLoader()
     // may return null. In that case, try to fall back to the system
     // classloader.
@@ -531,159 +522,9 @@ public boolean jdbcCompliant() {
    * @return Properties with elements added from the url
    */
   public static @Nullable Properties parseURL(String url, @Nullable Properties defaults) {
-    // priority 1 - URL values
-    Properties priority1Url = new Properties();
-    // priority 2 - Properties given as argument to DriverManager.getConnection()
-    // argument "defaults" EXCLUDING defaults
-    // priority 3 - Values retrieved by "service"
-    Properties priority3Service = new Properties();
-    // priority 4 - Properties loaded by Driver.loadDefaultProperties() (user, org/postgresql/driverconfig.properties)
-    // argument "defaults" INCLUDING defaults
-    // priority 5 - PGProperty defaults for PGHOST, PGPORT, PGDBNAME
-
-    String urlServer = url;
-    String urlArgs = "";
-
-    int qPos = url.indexOf('?');
-    if (qPos != -1) {
-      urlServer = url.substring(0, qPos);
-      urlArgs = url.substring(qPos + 1);
-    }
-
-    if (!urlServer.startsWith("jdbc:postgresql:")) {
-      LOGGER.log(Level.FINE, "JDBC URL must start with \"jdbc:postgresql:\" but was: {0}", url);
-      return null;
-    }
-    urlServer = urlServer.substring("jdbc:postgresql:".length());
-
-    if ("//".equals(urlServer) || "///".equals(urlServer)) {
-      urlServer = "";
-    } else if (urlServer.startsWith("//")) {
-      urlServer = urlServer.substring(2);
-      long slashCount = urlServer.chars().filter(ch -> ch == '/').count();
-      if (slashCount > 1) {
-        LOGGER.log(Level.WARNING, "JDBC URL contains too many / characters: {0}", url);
-        return null;
-      }
-      int slash = urlServer.indexOf('/');
-      if (slash == -1) {
-        LOGGER.log(Level.WARNING, "JDBC URL must contain a / at the end of the host or port: {0}", url);
-        return null;
-      }
-      if (!urlServer.endsWith("/")) {
-        String value = urlDecode(urlServer.substring(slash + 1));
-        if (value == null) {
-          return null;
-        }
-        PGProperty.PG_DBNAME.set(priority1Url, value);
-      }
-      urlServer = urlServer.substring(0, slash);
-
-      String[] addresses = urlServer.split(",");
-      StringBuilder hosts = new StringBuilder();
-      StringBuilder ports = new StringBuilder();
-      for (String address : addresses) {
-        int portIdx = address.lastIndexOf(':');
-        if (portIdx != -1 && address.lastIndexOf(']') < portIdx) {
-          String portStr = address.substring(portIdx + 1);
-          ports.append(portStr);
-          CharSequence hostStr = address.subSequence(0, portIdx);
-          if (hostStr.length() == 0) {
-            hosts.append(PGProperty.PG_HOST.getDefaultValue());
-          } else {
-            hosts.append(hostStr);
-          }
-        } else {
-          ports.append(PGProperty.PG_PORT.getDefaultValue());
-          hosts.append(address);
-        }
-        ports.append(',');
-        hosts.append(',');
-      }
-      ports.setLength(ports.length() - 1);
-      hosts.setLength(hosts.length() - 1);
-      PGProperty.PG_HOST.set(priority1Url, hosts.toString());
-      PGProperty.PG_PORT.set(priority1Url, ports.toString());
-    } else if (urlServer.startsWith("/")) {
-      return null;
-    } else {
-      String value = urlDecode(urlServer);
-      if (value == null) {
-        return null;
-      }
-      priority1Url.setProperty(PGProperty.PG_DBNAME.getName(), value);
-    }
-
-    // parse the args part of the url
-    String[] args = urlArgs.split("&");
-    String serviceName = null;
-    for (String token : args) {
-      if (token.isEmpty()) {
-        continue;
-      }
-      int pos = token.indexOf('=');
-      if (pos == -1) {
-        priority1Url.setProperty(token, "");
-      } else {
-        String pName = PGPropertyUtil.translatePGServiceToPGProperty(token.substring(0, pos));
-        String pValue = urlDecode(token.substring(pos + 1));
-        if (pValue == null) {
-          return null;
-        }
-        if (PGProperty.SERVICE.getName().equals(pName)) {
-          serviceName = pValue;
-        } else {
-          priority1Url.setProperty(pName, pValue);
-        }
-      }
-    }
-
-    // load pg_service.conf
-    if (serviceName != null) {
-      LOGGER.log(Level.FINE, "Processing option [?service={0}]", serviceName);
-      Properties result = PgServiceConfParser.getServiceProperties(serviceName);
-      if (result == null) {
-        LOGGER.log(Level.WARNING, "Definition of service [{0}] not found", serviceName);
-        return null;
-      }
-      priority3Service.putAll(result);
-    }
-
-    // combine result based on order of priority
-    Properties result = new Properties();
-    result.putAll(priority1Url);
-    if (defaults != null) {
-      // priority 2 - forEach() returns all entries EXCEPT defaults
-      defaults.forEach(result::putIfAbsent);
-    }
-    priority3Service.forEach(result::putIfAbsent);
-    if (defaults != null) {
-      // priority 4 - stringPropertyNames() returns all entries INCLUDING defaults
-      defaults.stringPropertyNames().forEach(s -> result.putIfAbsent(s, castNonNull(defaults.getProperty(s))));
-    }
-    // priority 5 - PGProperty defaults for PGHOST, PGPORT, PGDBNAME
-    result.putIfAbsent(PGProperty.PG_PORT.getName(), castNonNull(PGProperty.PG_PORT.getDefaultValue()));
-    result.putIfAbsent(PGProperty.PG_HOST.getName(), castNonNull(PGProperty.PG_HOST.getDefaultValue()));
-    if (PGProperty.USER.getOrDefault(result) != null) {
-      result.putIfAbsent(PGProperty.PG_DBNAME.getName(), castNonNull(PGProperty.USER.getOrDefault(result)));
-    }
-
-    // consistency check
-    if (!PGPropertyUtil.propertiesConsistencyCheck(result)) {
-      return null;
-    }
-
-    // try to load .pgpass if password is missing
-    if (PGProperty.PASSWORD.getOrDefault(result) == null) {
-      String password = PgPassParser.getPassword(
-          PGProperty.PG_HOST.getOrDefault(result), PGProperty.PG_PORT.getOrDefault(result), PGProperty.PG_DBNAME.getOrDefault(result), PGProperty.USER.getOrDefault(result)
-      );
-      if (password != null && !password.isEmpty()) {
-        PGProperty.PASSWORD.set(result, password);
-      }
-    }
-    //
-    return result;
+    // resolve url (including "?service=" syntax and pgpass file)
+    JdbcUrlResolver resolver = new JdbcUrlResolver(url, defaults);
+    return resolver.getResult();
   }
 
   // decode url, on failure log and return null
@@ -700,8 +541,8 @@ public boolean jdbcCompliant() {
    * @return the address portion of the URL
    */
   private static HostSpec[] hostSpecs(Properties props) {
-    String[] hosts = castNonNull(PGProperty.PG_HOST.getOrDefault(props)).split(",");
-    String[] ports = castNonNull(PGProperty.PG_PORT.getOrDefault(props)).split(",");
+    String[] hosts = castNonNull(PGProperty.HOST.getOrDefault(props)).split(",");
+    String[] ports = castNonNull(PGProperty.PORT.getOrDefault(props)).split(",");
     String localSocketAddress = PGProperty.LOCAL_SOCKET_ADDRESS.getOrDefault(props);
     HostSpec[] hostSpecs = new HostSpec[hosts.length];
     for (int i = 0; i < hostSpecs.length; i++) {

diff --git a/pgjdbc/src/main/java/org/postgresql/PGEnvironment.java b/pgjdbc/src/main/java/org/postgresql/PGEnvironment.java
@@ -16,6 +16,38 @@
  */
 public enum PGEnvironment {
 
+  /**
+   * The database name.
+   */
+  ORG_POSTGRESQL_PGDATABASE(
+      "org.postgresql.pgdatabase",
+      null,
+      "Specifies the database of instance."),
+
+  /**
+   * The database name.
+   */
+  PGDATABASE(
+      "PGDATABASE",
+      null,
+      "Specifies the database of instance."),
+
+  /**
+   * Name of host to connect to.
+   */
+  ORG_POSTGRESQL_PGHOST(
+      "org.postgresql.pghost",
+      null,
+      "Specifies the host of instance."),
+
+  /**
+   * Name of host to connect to.
+   */
+  PGHOST(
+      "PGHOST",
+      null,
+      "Specifies the host of instance."),
+
   /**
    * Specified location of password file.
    */
@@ -29,9 +61,57 @@ public enum PGEnvironment {
    */
   PGPASSFILE(
       "PGPASSFILE",
-      "pgpass",
+      null,
       "Specified location of password file."),
 
+  /**
+   * The password of user.
+   */
+  ORG_POSTGRESQL_PGPASSWORD(
+      "org.postgresql.pgpassword",
+      null,
+      "Specifies the password of instance."),
+
+  /**
+   * The password of user.
+   */
+  PGPASSWORD(
+      "PGPASSWORD",
+      null,
+      "Specifies the password of instance."),
+
+  /**
+   * Port number to connect to at the server host.
+   */
+  ORG_POSTGRESQL_PGPORT(
+      "org.postgresql.pgport",
+      null,
+      "Specifies the port of instance."),
+
+  /**
+   * Port number to connect to at the server host.
+   */
+  PGPORT(
+      "PGPORT",
+      null,
+      "Specifies the port of instance."),
+
+  /**
+   * The connection service name to be found in PGSERVICEFILE.
+   */
+  ORG_POSTGRESQL_PGSERVICE(
+      "org.postgresql.pgservice",
+      null,
+      "Specifies the service name."),
+
+  /**
+   * The connection service name to be found in PGSERVICEFILE.
+   */
+  PGSERVICE(
+      "PGSERVICE",
+      null,
+      "Specifies the service name."),
+
   /**
    * The connection service resource (file, url) allows connection parameters to be associated
    * with a single service name.
@@ -47,7 +127,7 @@ public enum PGEnvironment {
    */
   PGSERVICEFILE(
       "PGSERVICEFILE",
-      "pg_service.conf",
+      null,
       "Specifies the service resource to resolve connection properties."),
 
   /**
@@ -58,6 +138,22 @@ public enum PGEnvironment {
       "PGSYSCONFDIR",
       null,
       "Specifies the directory containing the PGSERVICEFILE file"),
+
+  /**
+   * The connection user.
+   */
+  ORG_POSTGRESQL_PGUSER(
+      "org.postgresql.pguser",
+      null,
+      "Specifies the user of instance."),
+
+  /**
+   * The connection user.
+   */
+  PGUSER(
+      "PGUSER",
+      null,
+      "Specifies the user of instance."),
   ;
 
   private final String name;
@@ -80,6 +176,10 @@ public enum PGEnvironment {
     }
   }
 
+  public static @Nullable PGEnvironment forName(String name) {
+    return PROPS_BY_NAME.get(name);
+  }
+
   /**
    * Returns the name of the parameter.
    *
@@ -107,4 +207,11 @@ public String getDescription() {
     return description;
   }
 
+  public @Nullable String readStringValue() {
+    if (this.getName().startsWith("org.postgresql.")) {
+      return System.getProperty(this.getName());
+    } else {
+      return System.getenv().get(this.getName());
+    }
+  }
 }