Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/urlparser improve3 pr2 #2646

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
87 changes: 79 additions & 8 deletions README.md

Large diffs are not rendered by default.

Expand Up @@ -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());

Expand Down
4 changes: 3 additions & 1 deletion packaging/rpm/postgresql-jdbc.spec.tpl
Expand Up @@ -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.
Expand Down
171 changes: 6 additions & 165 deletions pgjdbc/src/main/java/org/postgresql/Driver.java
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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++) {
Expand Down
111 changes: 109 additions & 2 deletions pgjdbc/src/main/java/org/postgresql/PGEnvironment.java
Expand Up @@ -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.
*/
Expand All @@ -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.
Expand All @@ -47,7 +127,7 @@ public enum PGEnvironment {
*/
PGSERVICEFILE(
"PGSERVICEFILE",
"pg_service.conf",
null,
"Specifies the service resource to resolve connection properties."),

/**
Expand All @@ -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;
Expand All @@ -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.
*
Expand Down Expand Up @@ -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());
}
}
}