From f67c61b4a87e71db674e5a70c8169e1df5b17ca5 Mon Sep 17 00:00:00 2001 From: Jeff Yemin Date: Wed, 12 Jun 2024 13:26:18 -0400 Subject: [PATCH 1/4] Forward slash in connection string is optional JAVA-5166 --- .../main/com/mongodb/ConnectionString.java | 21 ++++++++------- .../connection-string/invalid-uris.json | 27 ++++++++++++------- .../connection-string/valid-options.json | 19 ++++++++++++- .../ConnectionStringSpecification.groovy | 24 ++++++++++++++++- .../MongoClientURISpecification.groovy | 8 ------ 5 files changed, 71 insertions(+), 28 deletions(-) diff --git a/driver-core/src/main/com/mongodb/ConnectionString.java b/driver-core/src/main/com/mongodb/ConnectionString.java index c4e50d88020..15aac1e1a81 100644 --- a/driver-core/src/main/com/mongodb/ConnectionString.java +++ b/driver-core/src/main/com/mongodb/ConnectionString.java @@ -368,16 +368,19 @@ public ConnectionString(final String connectionString, @Nullable final DnsClient // Split out the user and host information String userAndHostInformation; - int idx = unprocessedConnectionString.indexOf("/"); - if (idx == -1) { - if (unprocessedConnectionString.contains("?")) { - throw new IllegalArgumentException("The connection string contains options without trailing slash"); + int firstForwardSlashIdx = unprocessedConnectionString.indexOf("/"); + int firstQuestionMarkIdx = unprocessedConnectionString.indexOf("?"); + if (firstForwardSlashIdx == -1 || (firstQuestionMarkIdx != -1 && firstQuestionMarkIdx < firstForwardSlashIdx)) { + if (firstQuestionMarkIdx == -1) { + userAndHostInformation = unprocessedConnectionString; + unprocessedConnectionString = ""; + } else { + userAndHostInformation = unprocessedConnectionString.substring(0, firstQuestionMarkIdx); + unprocessedConnectionString = unprocessedConnectionString.substring(firstQuestionMarkIdx); } - userAndHostInformation = unprocessedConnectionString; - unprocessedConnectionString = ""; } else { - userAndHostInformation = unprocessedConnectionString.substring(0, idx); - unprocessedConnectionString = unprocessedConnectionString.substring(idx + 1); + userAndHostInformation = unprocessedConnectionString.substring(0, firstForwardSlashIdx); + unprocessedConnectionString = unprocessedConnectionString.substring(firstForwardSlashIdx + 1); } // Split the user and host information @@ -385,7 +388,7 @@ public ConnectionString(final String connectionString, @Nullable final DnsClient String hostIdentifier; String userName = null; char[] password = null; - idx = userAndHostInformation.lastIndexOf("@"); + int idx = userAndHostInformation.lastIndexOf("@"); if (idx > 0) { userInfo = userAndHostInformation.substring(0, idx).replace("+", "%2B"); hostIdentifier = userAndHostInformation.substring(idx + 1); diff --git a/driver-core/src/test/resources/connection-string/invalid-uris.json b/driver-core/src/test/resources/connection-string/invalid-uris.json index 2a182fac7e2..a7accbd27d6 100644 --- a/driver-core/src/test/resources/connection-string/invalid-uris.json +++ b/driver-core/src/test/resources/connection-string/invalid-uris.json @@ -162,15 +162,6 @@ "auth": null, "options": null }, - { - "description": "Missing delimiting slash between hosts and options", - "uri": "mongodb://example.com?w=1", - "valid": false, - "warning": null, - "hosts": null, - "auth": null, - "options": null - }, { "description": "Incomplete key value pair for option", "uri": "mongodb://example.com/?w", @@ -269,6 +260,24 @@ "hosts": null, "auth": null, "options": null + }, + { + "description": "Username with password containing an unescaped percent sign and an escaped one", + "uri": "mongodb://user%20%:password@localhost", + "valid": false, + "warning": null, + "hosts": null, + "auth": null, + "options": null + }, + { + "description": "Username with password containing an unescaped percent sign (non hex digit)", + "uri": "mongodb://user%w:password@localhost", + "valid": false, + "warning": null, + "hosts": null, + "auth": null, + "options": null } ] } diff --git a/driver-core/src/test/resources/connection-string/valid-options.json b/driver-core/src/test/resources/connection-string/valid-options.json index cb2027f86ff..a1826e5d342 100644 --- a/driver-core/src/test/resources/connection-string/valid-options.json +++ b/driver-core/src/test/resources/connection-string/valid-options.json @@ -21,9 +21,26 @@ "authmechanism": "MONGODB-CR" } }, + { + "description": "Missing delimiting slash between hosts and options", + "uri": "mongodb://example.com?tls=true", + "valid": true, + "warning": false, + "hosts": [ + { + "type": "hostname", + "host": "example.com", + "port": null + } + ], + "auth": null, + "options": { + "tls": true + } + }, { "description": "Colon in a key value pair", - "uri": "mongodb://example.com/?authMechanism=MONGODB-OIDC&authMechanismProperties=TOKEN_RESOURCE:mongodb://test-cluster", + "uri": "mongodb://example.com?authMechanism=MONGODB-OIDC&authMechanismProperties=TOKEN_RESOURCE:mongodb://test-cluster", "valid": true, "warning": false, "hosts": [ diff --git a/driver-core/src/test/unit/com/mongodb/ConnectionStringSpecification.groovy b/driver-core/src/test/unit/com/mongodb/ConnectionStringSpecification.groovy index d56aa8a9c7c..958818b2ef1 100644 --- a/driver-core/src/test/unit/com/mongodb/ConnectionStringSpecification.groovy +++ b/driver-core/src/test/unit/com/mongodb/ConnectionStringSpecification.groovy @@ -141,6 +141,29 @@ class ConnectionStringSpecification extends Specification { .withWTimeout(5, MILLISECONDS).withJournal(true) } + @Unroll + def 'should treat trailing slash before query parameters as optional'() { + expect: + uri.getApplicationName() == appName + uri.getDatabase() == db + + where: + uri | appName | db + new ConnectionString('mongodb://mongodb.com') | null | null + new ConnectionString('mongodb://mongodb.com?') | null | null + new ConnectionString('mongodb://mongodb.com/') | null | null + new ConnectionString('mongodb://mongodb.com/?') | null | null + new ConnectionString('mongodb://mongodb.com/test') | null | "test" + new ConnectionString('mongodb://mongodb.com/test?') | null | "test" + new ConnectionString('mongodb://mongodb.com/?appName=a1') | "a1" | null + new ConnectionString('mongodb://mongodb.com?appName=a1') | "a1" | null + new ConnectionString('mongodb://mongodb.com/?appName=a1/a2') | "a1/a2" | null + new ConnectionString('mongodb://mongodb.com?appName=a1/a2') | "a1/a2" | null + new ConnectionString('mongodb://mongodb.com/test?appName=a1/a2') | "a1/a2" | "test" + new ConnectionString('mongodb://mongodb.com/test?appName=a1') | "a1" | "test" + new ConnectionString('mongodb://mongodb.com/test?appName=a1/a2') | "a1/a2" | "test" + } + def 'should correctly parse different UUID representations'() { expect: uri.getUuidRepresentation() == uuidRepresentation @@ -473,7 +496,6 @@ class ConnectionStringSpecification extends Specification { 'has an empty host' | 'mongodb://localhost:27017,,localhost:27019' 'has an malformed IPv6 host' | 'mongodb://[::1' 'has unescaped colons' | 'mongodb://locahost::1' - 'is missing a slash' | 'mongodb://localhost?wTimeout=5' 'contains an invalid port string' | 'mongodb://localhost:twenty' 'contains an invalid port negative' | 'mongodb://localhost:-1' 'contains an invalid port out of range' | 'mongodb://localhost:1000000' diff --git a/driver-legacy/src/test/unit/com/mongodb/MongoClientURISpecification.groovy b/driver-legacy/src/test/unit/com/mongodb/MongoClientURISpecification.groovy index 3db0e1a45d8..b187df8dab8 100644 --- a/driver-legacy/src/test/unit/com/mongodb/MongoClientURISpecification.groovy +++ b/driver-legacy/src/test/unit/com/mongodb/MongoClientURISpecification.groovy @@ -34,14 +34,6 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS class MongoClientURISpecification extends Specification { - def 'should throw Exception if URI does not have a trailing slash'() { - when: - new MongoClientURI('mongodb://localhost?wtimeoutMS=5') - - then: - thrown(IllegalArgumentException) - } - def 'should not throw an Exception if URI contains an unknown option'() { when: new MongoClientURI('mongodb://localhost/?unknownOption=5') From 60963664eb1a7079c894a1926ba374b0d4e17a1c Mon Sep 17 00:00:00 2001 From: Jeff Yemin Date: Wed, 12 Jun 2024 13:46:53 -0400 Subject: [PATCH 2/4] Update the rest of the connection string spec tests --- .../connection-string/valid-auth.json | 11 ++--- .../valid-unix_socket-absolute.json | 15 +++++++ .../valid-unix_socket-relative.json | 15 +++++++ .../connection-string/valid-warnings.json | 45 +++++++++++++++++++ .../com/mongodb/ConnectionStringTest.java | 3 ++ 5 files changed, 84 insertions(+), 5 deletions(-) diff --git a/driver-core/src/test/resources/connection-string/valid-auth.json b/driver-core/src/test/resources/connection-string/valid-auth.json index d3cafb029b9..176a54a096a 100644 --- a/driver-core/src/test/resources/connection-string/valid-auth.json +++ b/driver-core/src/test/resources/connection-string/valid-auth.json @@ -242,7 +242,7 @@ }, { "description": "Subdelimiters in user/pass don't need escaping (MONGODB-CR)", - "uri": "mongodb://!$&'()*,;=:!$&'()*,;=@127.0.0.1/admin?authMechanism=MONGODB-CR", + "uri": "mongodb://!$&'()*+,;=:!$&'()*+,;=@127.0.0.1/admin?authMechanism=MONGODB-CR", "valid": true, "warning": false, "hosts": [ @@ -253,8 +253,8 @@ } ], "auth": { - "username": "!$&'()*,;=", - "password": "!$&'()*,;=", + "username": "!$&'()*+,;=", + "password": "!$&'()*+,;=", "db": "admin" }, "options": { @@ -284,7 +284,7 @@ }, { "description": "Escaped username (GSSAPI)", - "uri": "mongodb://user%40EXAMPLE.COM:secret@localhost/?authMechanismProperties=SERVICE_NAME:other,CANONICALIZE_HOST_NAME:true&authMechanism=GSSAPI", + "uri": "mongodb://user%40EXAMPLE.COM:secret@localhost/?authMechanismProperties=SERVICE_NAME:other,CANONICALIZE_HOST_NAME:forward,SERVICE_HOST:example.com&authMechanism=GSSAPI", "valid": true, "warning": false, "hosts": [ @@ -303,7 +303,8 @@ "authmechanism": "GSSAPI", "authmechanismproperties": { "SERVICE_NAME": "other", - "CANONICALIZE_HOST_NAME": true + "SERVICE_HOST": "example.com", + "CANONICALIZE_HOST_NAME": "forward" } } }, diff --git a/driver-core/src/test/resources/connection-string/valid-unix_socket-absolute.json b/driver-core/src/test/resources/connection-string/valid-unix_socket-absolute.json index 5bb02476eb7..66491db13ba 100644 --- a/driver-core/src/test/resources/connection-string/valid-unix_socket-absolute.json +++ b/driver-core/src/test/resources/connection-string/valid-unix_socket-absolute.json @@ -30,6 +30,21 @@ "auth": null, "options": null }, + { + "description": "Unix domain socket (mixed case)", + "uri": "mongodb://%2Ftmp%2FMongoDB-27017.sock", + "valid": true, + "warning": false, + "hosts": [ + { + "type": "unix", + "host": "/tmp/MongoDB-27017.sock", + "port": null + } + ], + "auth": null, + "options": null + }, { "description": "Unix domain socket (absolute path with spaces in path)", "uri": "mongodb://%2Ftmp%2F %2Fmongodb-27017.sock", diff --git a/driver-core/src/test/resources/connection-string/valid-unix_socket-relative.json b/driver-core/src/test/resources/connection-string/valid-unix_socket-relative.json index 2ce649ffc23..788720920ba 100644 --- a/driver-core/src/test/resources/connection-string/valid-unix_socket-relative.json +++ b/driver-core/src/test/resources/connection-string/valid-unix_socket-relative.json @@ -30,6 +30,21 @@ "auth": null, "options": null }, + { + "description": "Unix domain socket (mixed case)", + "uri": "mongodb://rel%2FMongoDB-27017.sock", + "valid": true, + "warning": false, + "hosts": [ + { + "type": "unix", + "host": "rel/MongoDB-27017.sock", + "port": null + } + ], + "auth": null, + "options": null + }, { "description": "Unix domain socket (relative path with spaces)", "uri": "mongodb://rel%2F %2Fmongodb-27017.sock", diff --git a/driver-core/src/test/resources/connection-string/valid-warnings.json b/driver-core/src/test/resources/connection-string/valid-warnings.json index 87f7248f21e..663652aa55e 100644 --- a/driver-core/src/test/resources/connection-string/valid-warnings.json +++ b/driver-core/src/test/resources/connection-string/valid-warnings.json @@ -63,6 +63,51 @@ "options": { "wtimeoutms": 10 } + }, + { + "description": "Empty integer option values are ignored", + "uri": "mongodb://localhost/?maxIdleTimeMS=", + "valid": true, + "warning": true, + "hosts": [ + { + "type": "hostname", + "host": "localhost", + "port": null + } + ], + "auth": null, + "options": null + }, + { + "description": "Empty boolean option value are ignored", + "uri": "mongodb://localhost/?journal=", + "valid": true, + "warning": true, + "hosts": [ + { + "type": "hostname", + "host": "localhost", + "port": null + } + ], + "auth": null, + "options": null + }, + { + "description": "Comma in a key value pair causes a warning", + "uri": "mongodb://example.com?authMechanismProperties=TOKEN_RESOURCE:mongodb://host1%2Chost2", + "valid": true, + "warning": true, + "hosts": [ + { + "type": "hostname", + "host": "example.com", + "port": null + } + ], + "auth": null, + "options": null } ] } diff --git a/driver-core/src/test/unit/com/mongodb/ConnectionStringTest.java b/driver-core/src/test/unit/com/mongodb/ConnectionStringTest.java index 3b28460e866..623238419ee 100644 --- a/driver-core/src/test/unit/com/mongodb/ConnectionStringTest.java +++ b/driver-core/src/test/unit/com/mongodb/ConnectionStringTest.java @@ -29,6 +29,8 @@ import java.util.Collection; import java.util.List; +import static org.junit.Assume.assumeFalse; + // See https://github.com/mongodb/specifications/tree/master/source/connection-string/tests public class ConnectionStringTest extends AbstractConnectionStringTest { public ConnectionStringTest(final String filename, final String description, final String input, final BsonDocument definition) { @@ -37,6 +39,7 @@ public ConnectionStringTest(final String filename, final String description, fin @Test public void shouldPassAllOutcomes() { + assumeFalse(getDescription().equals("Empty integer option values are ignored")); if (getFilename().equals("invalid-uris.json")) { testInvalidUris(); } else if (getFilename().equals("valid-auth.json")) { From 8dee3dd8b2e326e56ee03392e349d0bd363bf931 Mon Sep 17 00:00:00 2001 From: Jeff Yemin Date: Thu, 13 Jun 2024 08:44:45 -0400 Subject: [PATCH 3/4] Code review updates --- .../src/main/com/mongodb/ConnectionString.java | 15 +++++++-------- .../connection-string/valid-options.json | 2 +- .../connection-string/valid-warnings.json | 6 +++--- .../mongodb/ConnectionStringSpecification.groovy | 1 - .../unit/com/mongodb/ConnectionStringTest.java | 2 ++ 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/driver-core/src/main/com/mongodb/ConnectionString.java b/driver-core/src/main/com/mongodb/ConnectionString.java index 15aac1e1a81..82c6b3a00ec 100644 --- a/driver-core/src/main/com/mongodb/ConnectionString.java +++ b/driver-core/src/main/com/mongodb/ConnectionString.java @@ -370,14 +370,13 @@ public ConnectionString(final String connectionString, @Nullable final DnsClient String userAndHostInformation; int firstForwardSlashIdx = unprocessedConnectionString.indexOf("/"); int firstQuestionMarkIdx = unprocessedConnectionString.indexOf("?"); - if (firstForwardSlashIdx == -1 || (firstQuestionMarkIdx != -1 && firstQuestionMarkIdx < firstForwardSlashIdx)) { - if (firstQuestionMarkIdx == -1) { - userAndHostInformation = unprocessedConnectionString; - unprocessedConnectionString = ""; - } else { - userAndHostInformation = unprocessedConnectionString.substring(0, firstQuestionMarkIdx); - unprocessedConnectionString = unprocessedConnectionString.substring(firstQuestionMarkIdx); - } + if (firstQuestionMarkIdx == -1 && firstForwardSlashIdx == -1) { + userAndHostInformation = unprocessedConnectionString; + unprocessedConnectionString = ""; + } else if (firstQuestionMarkIdx != -1 && (firstForwardSlashIdx == -1 || firstQuestionMarkIdx < firstForwardSlashIdx)) { + // there is a question mark, and there is no slash or the question mark comes before any slash + userAndHostInformation = unprocessedConnectionString.substring(0, firstQuestionMarkIdx); + unprocessedConnectionString = unprocessedConnectionString.substring(firstQuestionMarkIdx); } else { userAndHostInformation = unprocessedConnectionString.substring(0, firstForwardSlashIdx); unprocessedConnectionString = unprocessedConnectionString.substring(firstForwardSlashIdx + 1); diff --git a/driver-core/src/test/resources/connection-string/valid-options.json b/driver-core/src/test/resources/connection-string/valid-options.json index a1826e5d342..3c79fe7ae55 100644 --- a/driver-core/src/test/resources/connection-string/valid-options.json +++ b/driver-core/src/test/resources/connection-string/valid-options.json @@ -40,7 +40,7 @@ }, { "description": "Colon in a key value pair", - "uri": "mongodb://example.com?authMechanism=MONGODB-OIDC&authMechanismProperties=TOKEN_RESOURCE:mongodb://test-cluster", + "uri": "mongodb://example.com/?authMechanism=MONGODB-OIDC&authMechanismProperties=TOKEN_RESOURCE:mongodb://test-cluster", "valid": true, "warning": false, "hosts": [ diff --git a/driver-core/src/test/resources/connection-string/valid-warnings.json b/driver-core/src/test/resources/connection-string/valid-warnings.json index 663652aa55e..f0e8288bc73 100644 --- a/driver-core/src/test/resources/connection-string/valid-warnings.json +++ b/driver-core/src/test/resources/connection-string/valid-warnings.json @@ -93,16 +93,16 @@ ], "auth": null, "options": null - }, + }, { "description": "Comma in a key value pair causes a warning", - "uri": "mongodb://example.com?authMechanismProperties=TOKEN_RESOURCE:mongodb://host1%2Chost2", + "uri": "mongodb://localhost?authMechanism=MONGODB-OIDC&authMechanismProperties=TOKEN_RESOURCE:mongodb://host1%2Chost2", "valid": true, "warning": true, "hosts": [ { "type": "hostname", - "host": "example.com", + "host": "localhost", "port": null } ], diff --git a/driver-core/src/test/unit/com/mongodb/ConnectionStringSpecification.groovy b/driver-core/src/test/unit/com/mongodb/ConnectionStringSpecification.groovy index 958818b2ef1..72fdf108698 100644 --- a/driver-core/src/test/unit/com/mongodb/ConnectionStringSpecification.groovy +++ b/driver-core/src/test/unit/com/mongodb/ConnectionStringSpecification.groovy @@ -159,7 +159,6 @@ class ConnectionStringSpecification extends Specification { new ConnectionString('mongodb://mongodb.com?appName=a1') | "a1" | null new ConnectionString('mongodb://mongodb.com/?appName=a1/a2') | "a1/a2" | null new ConnectionString('mongodb://mongodb.com?appName=a1/a2') | "a1/a2" | null - new ConnectionString('mongodb://mongodb.com/test?appName=a1/a2') | "a1/a2" | "test" new ConnectionString('mongodb://mongodb.com/test?appName=a1') | "a1" | "test" new ConnectionString('mongodb://mongodb.com/test?appName=a1/a2') | "a1/a2" | "test" } diff --git a/driver-core/src/test/unit/com/mongodb/ConnectionStringTest.java b/driver-core/src/test/unit/com/mongodb/ConnectionStringTest.java index 623238419ee..ca73c8f613f 100644 --- a/driver-core/src/test/unit/com/mongodb/ConnectionStringTest.java +++ b/driver-core/src/test/unit/com/mongodb/ConnectionStringTest.java @@ -40,6 +40,8 @@ public ConnectionStringTest(final String filename, final String description, fin @Test public void shouldPassAllOutcomes() { assumeFalse(getDescription().equals("Empty integer option values are ignored")); + assumeFalse(getDescription().equals("Comma in a key value pair causes a warning")); + if (getFilename().equals("invalid-uris.json")) { testInvalidUris(); } else if (getFilename().equals("valid-auth.json")) { From c39105c31734644847579a8f09a6b051da3403f4 Mon Sep 17 00:00:00 2001 From: Jeff Yemin Date: Thu, 13 Jun 2024 14:14:24 -0400 Subject: [PATCH 4/4] Add comment on why tests are ignored --- driver-core/src/test/unit/com/mongodb/ConnectionStringTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/driver-core/src/test/unit/com/mongodb/ConnectionStringTest.java b/driver-core/src/test/unit/com/mongodb/ConnectionStringTest.java index ca73c8f613f..80cc9f65e83 100644 --- a/driver-core/src/test/unit/com/mongodb/ConnectionStringTest.java +++ b/driver-core/src/test/unit/com/mongodb/ConnectionStringTest.java @@ -39,6 +39,7 @@ public ConnectionStringTest(final String filename, final String description, fin @Test public void shouldPassAllOutcomes() { + // Java driver currently throws an IllegalArgumentException for these tests assumeFalse(getDescription().equals("Empty integer option values are ignored")); assumeFalse(getDescription().equals("Comma in a key value pair causes a warning"));