From 5fd66f15797c45c9bab7b59f9e55e0a2f7ad5cd0 Mon Sep 17 00:00:00 2001 From: Jason Carey Date: Mon, 4 Feb 2019 10:32:18 -0500 Subject: SERVER-39317 Make uri options case insensitive --- src/mongo/client/mongo_uri.cpp | 11 +- src/mongo/client/mongo_uri.h | 28 ++- src/mongo/client/mongo_uri_test.cpp | 232 +++++++++++++-------- .../mongo_uri_tests/mongo-uri-valid-auth.json | 7 +- src/mongo/scripting/mozjs/uri.cpp | 2 +- 5 files changed, 183 insertions(+), 97 deletions(-) diff --git a/src/mongo/client/mongo_uri.cpp b/src/mongo/client/mongo_uri.cpp index 99468784adb..1a52948cc51 100644 --- a/src/mongo/client/mongo_uri.cpp +++ b/src/mongo/client/mongo_uri.cpp @@ -148,8 +148,8 @@ std::pair partitionBackward(StringData str, const char c * on multiple parsed option sources. STL setwise operations require sorted lists. A map is used * instead of a vector of pairs to permit insertion-is-not-overwrite behavior. */ -std::map parseOptions(StringData options, StringData url) { - std::map ret; +MongoURI::OptionsMap parseOptions(StringData options, StringData url) { + MongoURI::OptionsMap ret; if (options.empty()) { return ret; } @@ -206,7 +206,7 @@ std::map parseOptions(StringData options, StringData u return ret; } -MongoURI::OptionsMap addTXTOptions(std::map options, +MongoURI::OptionsMap addTXTOptions(MongoURI::OptionsMap options, const std::string& host, const StringData url, const bool isSeedlist) { @@ -488,7 +488,8 @@ MongoURI MongoURI::parseImpl(const std::string& url) { transport::ConnectSSLMode sslMode = transport::kGlobalSSLMode; auto sslModeIter = std::find_if(options.begin(), options.end(), [](auto pred) { - return pred.first == "ssl"_sd || pred.first == "tls"_sd; + return pred.first == CaseInsensitiveString("ssl") || + pred.first == CaseInsensitiveString("tls"); }); if (sslModeIter != options.end()) { const auto& val = sslModeIter->second; @@ -565,7 +566,7 @@ std::string MongoURI::canonicalizeURIAsString() const { auto delimeter = ""; uri << "?"; for (const auto& pair : _options) { - uri << delimeter << uriEncode(pair.first) << "=" << uriEncode(pair.second); + uri << delimeter << uriEncode(pair.first.original()) << "=" << uriEncode(pair.second); delimeter = "&"; } } diff --git a/src/mongo/client/mongo_uri.h b/src/mongo/client/mongo_uri.h index e600522b463..a5fb3492b0d 100644 --- a/src/mongo/client/mongo_uri.h +++ b/src/mongo/client/mongo_uri.h @@ -30,6 +30,7 @@ #pragma once +#include #include #include #include @@ -103,12 +104,37 @@ StatusWith uriDecode(StringData str); */ class MongoURI { public: + class CaseInsensitiveString { + public: + CaseInsensitiveString(std::string str) + : _original(std::move(str)), _lowercase(boost::algorithm::to_lower_copy(_original)) {} + + CaseInsensitiveString(StringData sd) : CaseInsensitiveString(std::string(sd)) {} + CaseInsensitiveString(const char* str) : CaseInsensitiveString(std::string(str)) {} + + friend bool operator<(const CaseInsensitiveString& lhs, const CaseInsensitiveString& rhs) { + return lhs._lowercase < rhs._lowercase; + } + + friend bool operator==(const CaseInsensitiveString& lhs, const CaseInsensitiveString& rhs) { + return lhs._lowercase == rhs._lowercase; + } + + const std::string& original() const noexcept { + return _original; + } + + private: + std::string _original; + std::string _lowercase; + }; + // Note that, because this map is used for DNS TXT record injection on options, there is a // requirement on its behavior for `insert`: insert must not replace or update existing values // -- this gives the desired behavior that user-specified values override TXT record specified // values. `std::map` and `std::unordered_map` satisfy this requirement. Make sure that // whichever map type is used provides that guarantee. - using OptionsMap = std::map; + using OptionsMap = std::map; static StatusWith parse(const std::string& url); diff --git a/src/mongo/client/mongo_uri_test.cpp b/src/mongo/client/mongo_uri_test.cpp index c8136d3198e..c9aba2294ac 100644 --- a/src/mongo/client/mongo_uri_test.cpp +++ b/src/mongo/client/mongo_uri_test.cpp @@ -58,7 +58,7 @@ struct URITestCase { ConnectionString::ConnectionType type; std::string setname; size_t numservers; - size_t numOptions; + MongoURI::OptionsMap options; std::string database; ConnectSSLMode sslMode; }; @@ -72,22 +72,49 @@ struct InvalidURITestCase { } }; +void compareOptions(size_t lineNumber, + StringData uri, + const MongoURI::OptionsMap& connection, + const MongoURI::OptionsMap& expected) { + std::vector> options(begin(connection), + end(connection)); + std::sort(begin(options), end(options)); + std::vector> expectedOptions( + begin(expected), end(expected)); + std::sort(begin(expectedOptions), end(expectedOptions)); + + for (std::size_t i = 0; i < std::min(options.size(), expectedOptions.size()); ++i) { + if (options[i] != expectedOptions[i]) { + unittest::log() << "Option: \"tolower(" << options[i].first.original() + << ")=" << options[i].second << "\" doesn't equal: \"tolower(" + << expectedOptions[i].first.original() + << ")=" << expectedOptions[i].second << "\"" + << " data on line: " << lineNumber << std::endl; + std::cerr << "Failing URI: \"" << uri << "\"" + << " data on line: " << lineNumber << std::endl; + ASSERT(false); + } + } + ASSERT_EQ(options.size(), expectedOptions.size()) << "Failing URI: " + << " data on line: " << lineNumber << uri; +} + const ConnectionString::ConnectionType kMaster = ConnectionString::MASTER; const ConnectionString::ConnectionType kSet = ConnectionString::SET; const URITestCase validCases[] = { - {"mongodb://user:pwd@127.0.0.1", "user", "pwd", kMaster, "", 1, 0, "", kGlobalSSLMode}, + {"mongodb://user:pwd@127.0.0.1", "user", "pwd", kMaster, "", 1, {}, "", kGlobalSSLMode}, - {"mongodb://user@127.0.0.1", "user", "", kMaster, "", 1, 0, "", kGlobalSSLMode}, + {"mongodb://user@127.0.0.1", "user", "", kMaster, "", 1, {}, "", kGlobalSSLMode}, - {"mongodb://localhost/?foo=bar", "", "", kMaster, "", 1, 1, "", kGlobalSSLMode}, + {"mongodb://localhost/?foo=bar", "", "", kMaster, "", 1, {{"foo", "bar"}}, "", kGlobalSSLMode}, - {"mongodb://localhost,/?foo=bar", "", "", kMaster, "", 1, 1, "", kGlobalSSLMode}, + {"mongodb://localhost,/?foo=bar", "", "", kMaster, "", 1, {{"foo", "bar"}}, "", kGlobalSSLMode}, - {"mongodb://user:pwd@127.0.0.1:1234", "user", "pwd", kMaster, "", 1, 0, "", kGlobalSSLMode}, + {"mongodb://user:pwd@127.0.0.1:1234", "user", "pwd", kMaster, "", 1, {}, "", kGlobalSSLMode}, - {"mongodb://user@127.0.0.1:1234", "user", "", kMaster, "", 1, 0, "", kGlobalSSLMode}, + {"mongodb://user@127.0.0.1:1234", "user", "", kMaster, "", 1, {}, "", kGlobalSSLMode}, {"mongodb://127.0.0.1:1234/dbName?foo=a&c=b", "", @@ -95,11 +122,19 @@ const URITestCase validCases[] = { kMaster, "", 1, - 2, + {{"foo", "a"}, {"c", "b"}}, "dbName", kGlobalSSLMode}, - {"mongodb://127.0.0.1/dbName?foo=a&c=b", "", "", kMaster, "", 1, 2, "dbName", kGlobalSSLMode}, + {"mongodb://127.0.0.1/dbName?foo=a&c=b", + "", + "", + kMaster, + "", + 1, + {{"foo", "a"}, {"c", "b"}}, + "dbName", + kGlobalSSLMode}, {"mongodb://user:pwd@127.0.0.1,/dbName?foo=a&c=b", "user", @@ -107,7 +142,7 @@ const URITestCase validCases[] = { kMaster, "", 1, - 2, + {{"foo", "a"}, {"c", "b"}}, "dbName", kGlobalSSLMode}, @@ -117,7 +152,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 2, + {{"a", "b"}, {"replicaSet", "replName"}}, "dbname", kGlobalSSLMode}, @@ -128,7 +163,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 2, + {{"a", "b"}, {"replicaSet", "replName"}}, "dbname", kGlobalSSLMode}, @@ -139,7 +174,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 2, + {{"a", "b"}, {"replicaSet", "replName"}}, "db@name", kGlobalSSLMode}, @@ -150,7 +185,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 2, + {{"a", "b"}, {"replicaSet", "replName"}}, "dbname", kGlobalSSLMode}, @@ -160,7 +195,7 @@ const URITestCase validCases[] = { kSet, "needs encoding%#!<>", 2, - 2, + {{"a", "b"}, {"replicaSet", "needs encoding%#!<>"}}, "dbname", kGlobalSSLMode}, @@ -170,7 +205,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 2, + {{"a", "b"}, {"replicaSet", "replName"}}, "needsencoding@hello", kGlobalSSLMode}, @@ -180,7 +215,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 1, + {{"replicaSet", "replName"}}, "", kGlobalSSLMode}, @@ -190,7 +225,17 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 1, + {{"replicaSet", "replName"}}, + "", + kGlobalSSLMode}, + + {"mongodb://user@127.0.0.1,127.0.0.2/?replicaset=replName", + "user", + "", + kSet, + "replName", + 2, + {{"replicaSet", "replName"}}, "", kGlobalSSLMode}, @@ -200,7 +245,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 3, + {{"foo", "a"}, {"c", "b"}, {"replicaSet", "replName"}}, "dbName", kGlobalSSLMode}, @@ -210,7 +255,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 1, + {{"replicaSet", "replName"}}, "", kGlobalSSLMode}, @@ -220,7 +265,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 1, + {{"replicaSet", "replName"}}, "", kGlobalSSLMode}, @@ -230,21 +275,37 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 3, + {{"foo", "a"}, {"c", "b"}, {"replicaSet", "replName"}}, "dbName", kGlobalSSLMode}, - {"mongodb://user:pwd@[::1]", "user", "pwd", kMaster, "", 1, 0, "", kGlobalSSLMode}, + {"mongodb://user:pwd@[::1]", "user", "pwd", kMaster, "", 1, {}, "", kGlobalSSLMode}, - {"mongodb://user@[::1]", "user", "", kMaster, "", 1, 0, "", kGlobalSSLMode}, + {"mongodb://user@[::1]", "user", "", kMaster, "", 1, {}, "", kGlobalSSLMode}, - {"mongodb://[::1]/dbName?foo=a&c=b", "", "", kMaster, "", 1, 2, "dbName", kGlobalSSLMode}, + {"mongodb://[::1]/dbName?foo=a&c=b", + "", + "", + kMaster, + "", + 1, + {{"foo", "a"}, {"c", "b"}}, + "dbName", + kGlobalSSLMode}, - {"mongodb://user:pwd@[::1]:1234", "user", "pwd", kMaster, "", 1, 0, "", kGlobalSSLMode}, + {"mongodb://user:pwd@[::1]:1234", "user", "pwd", kMaster, "", 1, {}, "", kGlobalSSLMode}, - {"mongodb://user@[::1]:1234", "user", "", kMaster, "", 1, 0, "", kGlobalSSLMode}, + {"mongodb://user@[::1]:1234", "user", "", kMaster, "", 1, {}, "", kGlobalSSLMode}, - {"mongodb://[::1]:1234/dbName?foo=a&c=b", "", "", kMaster, "", 1, 2, "dbName", kGlobalSSLMode}, + {"mongodb://[::1]:1234/dbName?foo=a&c=b", + "", + "", + kMaster, + "", + 1, + {{"foo", "a"}, {"c", "b"}}, + "dbName", + kGlobalSSLMode}, {"mongodb://user:pwd@[::1],127.0.0.2/?replicaSet=replName", "user", @@ -252,7 +313,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 1, + {{"replicaSet", "replName"}}, "", kGlobalSSLMode}, @@ -262,7 +323,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 1, + {{"replicaSet", "replName"}}, "", kGlobalSSLMode}, @@ -272,7 +333,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 3, + {{"foo", "a"}, {"c", "b"}, {"replicaSet", "replName"}}, "dbName", kGlobalSSLMode}, @@ -282,7 +343,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 1, + {{"replicaSet", "replName"}}, "", kGlobalSSLMode}, @@ -292,7 +353,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 1, + {{"replicaSet", "replName"}}, "", kGlobalSSLMode}, @@ -302,21 +363,37 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 3, + {{"foo", "a"}, {"c", "b"}, {"replicaSet", "replName"}}, "dbName", kGlobalSSLMode}, - {"mongodb://user:pwd@[::1]", "user", "pwd", kMaster, "", 1, 0, "", kGlobalSSLMode}, + {"mongodb://user:pwd@[::1]", "user", "pwd", kMaster, "", 1, {}, "", kGlobalSSLMode}, - {"mongodb://user@[::1]", "user", "", kMaster, "", 1, 0, "", kGlobalSSLMode}, + {"mongodb://user@[::1]", "user", "", kMaster, "", 1, {}, "", kGlobalSSLMode}, - {"mongodb://[::1]/dbName?foo=a&c=b", "", "", kMaster, "", 1, 2, "dbName", kGlobalSSLMode}, + {"mongodb://[::1]/dbName?foo=a&c=b", + "", + "", + kMaster, + "", + 1, + {{"foo", "a"}, {"c", "b"}}, + "dbName", + kGlobalSSLMode}, - {"mongodb://user:pwd@[::1]:1234", "user", "pwd", kMaster, "", 1, 0, "", kGlobalSSLMode}, + {"mongodb://user:pwd@[::1]:1234", "user", "pwd", kMaster, "", 1, {}, "", kGlobalSSLMode}, - {"mongodb://user@[::1]:1234", "user", "", kMaster, "", 1, 0, "", kGlobalSSLMode}, + {"mongodb://user@[::1]:1234", "user", "", kMaster, "", 1, {}, "", kGlobalSSLMode}, - {"mongodb://[::1]:1234/dbName?foo=a&c=b", "", "", kMaster, "", 1, 2, "dbName", kGlobalSSLMode}, + {"mongodb://[::1]:1234/dbName?foo=a&c=b", + "", + "", + kMaster, + "", + 1, + {{"foo", "a"}, {"c", "b"}}, + "dbName", + kGlobalSSLMode}, {"mongodb://user:pwd@[::1],127.0.0.2/?replicaSet=replName", "user", @@ -324,7 +401,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 1, + {{"replicaSet", "replName"}}, "", kGlobalSSLMode}, @@ -334,7 +411,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 1, + {{"replicaSet", "replName"}}, "", kGlobalSSLMode}, @@ -344,7 +421,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 3, + {{"foo", "a"}, {"c", "b"}, {"replicaSet", "replName"}}, "dbName", kGlobalSSLMode}, @@ -354,7 +431,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 1, + {{"replicaSet", "replName"}}, "", kGlobalSSLMode}, @@ -364,7 +441,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 1, + {{"replicaSet", "replName"}}, "", kGlobalSSLMode}, @@ -374,7 +451,7 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 3, + {{"foo", "a"}, {"c", "b"}, {"replicaSet", "replName"}}, "dbName", kGlobalSSLMode}, @@ -384,7 +461,7 @@ const URITestCase validCases[] = { kMaster, "", 1, - 2, + {{"authmechanism", "GSSAPI"}, {"authmechanismproperties", "SERVICE_NAME:foobar"}}, "", kGlobalSSLMode}, @@ -394,11 +471,11 @@ const URITestCase validCases[] = { kMaster, "", 1, - 2, + {{"authmechanism", "GSSAPI"}, {"gssapiServiceName", "foobar"}}, "", kGlobalSSLMode}, - {"mongodb://%2Ftmp%2Fmongodb-27017.sock", "", "", kMaster, "", 1, 0, "", kGlobalSSLMode}, + {"mongodb://%2Ftmp%2Fmongodb-27017.sock", "", "", kMaster, "", 1, {}, "", kGlobalSSLMode}, {"mongodb://%2Ftmp%2Fmongodb-27017.sock,%2Ftmp%2Fmongodb-27018.sock/?replicaSet=replName", "", @@ -406,14 +483,14 @@ const URITestCase validCases[] = { kSet, "replName", 2, - 1, + {{"replicaSet", "replName"}}, "", kGlobalSSLMode}, - {"mongodb://localhost/?ssl=true", "", "", kMaster, "", 1, 1, "", kEnableSSL}, - {"mongodb://localhost/?ssl=false", "", "", kMaster, "", 1, 1, "", kDisableSSL}, - {"mongodb://localhost/?tls=true", "", "", kMaster, "", 1, 1, "", kEnableSSL}, - {"mongodb://localhost/?tls=false", "", "", kMaster, "", 1, 1, "", kDisableSSL}, + {"mongodb://localhost/?ssl=true", "", "", kMaster, "", 1, {{"ssl", "true"}}, "", kEnableSSL}, + {"mongodb://localhost/?ssl=false", "", "", kMaster, "", 1, {{"ssl", "false"}}, "", kDisableSSL}, + {"mongodb://localhost/?tls=true", "", "", kMaster, "", 1, {{"tls", "true"}}, "", kEnableSSL}, + {"mongodb://localhost/?tls=false", "", "", kMaster, "", 1, {{"tls", "false"}}, "", kDisableSSL}, }; const InvalidURITestCase invalidCases[] = { @@ -528,7 +605,7 @@ void testValidURIFormat(URITestCase testCase) { ASSERT_EQ(testCase.type, result.type()); ASSERT_EQ(testCase.setname, result.getSetName()); ASSERT_EQ(testCase.numservers, result.getServers().size()); - ASSERT_EQ(testCase.numOptions, result.getOptions().size()); + compareOptions(0, testCase.URI, result.getOptions(), testCase.options); ASSERT_EQ(testCase.database, result.getDatabase()); } @@ -659,6 +736,7 @@ TEST(MongoURI, specTests) { std::string setName; const auto optionsElement = test.getField("options"); ASSERT_FALSE(optionsElement.eoo()); + MongoURI::OptionsMap options; if (optionsElement.type() != jstNULL) { ASSERT_EQ(optionsElement.type(), Object); const auto optionsObj = optionsElement.Obj(); @@ -669,17 +747,21 @@ TEST(MongoURI, specTests) { setName = replsetElement.String(); connectionType = kSet; } + + for (auto&& field : optionsElement.Obj()) { + if (field.type() == String) { + options[field.fieldNameStringData()] = field.String(); + } else if (field.isNumber()) { + options[field.fieldNameStringData()] = std::to_string(field.Int()); + } else { + MONGO_UNREACHABLE; + } + } } // Create the URITestCase abnd - const URITestCase testCase = {uri, - username, - password, - connectionType, - setName, - numHosts, - numOptions, - database}; + const URITestCase testCase = { + uri, username, password, connectionType, setName, numHosts, options, database}; testValidURIFormat(testCase); } } @@ -695,7 +777,7 @@ TEST(MongoURI, srvRecordTest) { std::string password; std::string database; std::vector hosts; - std::map options; + std::map options; Expectation expectation; } tests[] = { // Test some non-SRV URIs to make sure that they do not perform expansions @@ -920,27 +1002,7 @@ TEST(MongoURI, srvRecordTest) { << " data on line : " << test.lineNumber; ASSERT_EQ(rv.getDatabase(), test.database) << "Failed on URI: " << test.uri << " data on line : " << test.lineNumber; - std::vector> options(begin(rv.getOptions()), - end(rv.getOptions())); - std::sort(begin(options), end(options)); - std::vector> expectedOptions(begin(test.options), - end(test.options)); - std::sort(begin(expectedOptions), end(expectedOptions)); - - for (std::size_t i = 0; i < std::min(options.size(), expectedOptions.size()); ++i) { - if (options[i] != expectedOptions[i]) { - unittest::log() << "Option: \"" << options[i].first << "=" << options[i].second - << "\" doesn't equal: \"" << expectedOptions[i].first << "=" - << expectedOptions[i].second << "\"" - << " data on line: " << test.lineNumber << std::endl; - std::cerr << "Failing URI: \"" << test.uri << "\"" - << " data on line: " << test.lineNumber << std::endl; - ASSERT(false); - } - } - ASSERT_EQ(options.size(), expectedOptions.size()) << "Failing URI: " - << " data on line: " << test.lineNumber - << test.uri; + compareOptions(test.lineNumber, test.uri, rv.getOptions(), test.options); std::vector hosts(begin(rv.getServers()), end(rv.getServers())); std::sort(begin(hosts), end(hosts)); diff --git a/src/mongo/client/mongo_uri_tests/mongo-uri-valid-auth.json b/src/mongo/client/mongo_uri_tests/mongo-uri-valid-auth.json index 68228c6ddec..516edd8be6e 100644 --- a/src/mongo/client/mongo_uri_tests/mongo-uri-valid-auth.json +++ b/src/mongo/client/mongo_uri_tests/mongo-uri-valid-auth.json @@ -267,7 +267,7 @@ "password": "secret", "username": "user@EXAMPLE.COM" }, - "description": "Escaped username (GSSAPI)", + "description": "Escaped username (GSSAPI) - SERVER NOTE: This test case has been doctored to return authmechanismproperties flattened, because that's all we support", "hosts": [ { "host": "localhost", @@ -277,10 +277,7 @@ ], "options": { "authmechanism": "GSSAPI", - "authmechanismproperties": { - "CANONICALIZE_HOST_NAME": true, - "SERVICE_NAME": "other" - } + "authmechanismproperties": "SERVICE_NAME:other,CANONICALIZE_HOST_NAME:true" }, "uri": "mongodb://user%40EXAMPLE.COM:secret@localhost/?authMechanismProperties=SERVICE_NAME:other,CANONICALIZE_HOST_NAME:true&authMechanism=GSSAPI", "valid": true, diff --git a/src/mongo/scripting/mozjs/uri.cpp b/src/mongo/scripting/mozjs/uri.cpp index 92cb0053b17..011d201918a 100644 --- a/src/mongo/scripting/mozjs/uri.cpp +++ b/src/mongo/scripting/mozjs/uri.cpp @@ -83,7 +83,7 @@ void URIInfo::construct(JSContext* cx, JS::CallArgs args) { BSONObjBuilder optsBuilder; for (const auto& kvpair : parsed.getOptions()) { - optsBuilder.append(kvpair.first, kvpair.second); + optsBuilder.append(kvpair.first.original(), kvpair.second); } JS::RootedObject thisv(cx); -- cgit v1.2.1