From bf12e975e38c7b9724282ffee2cc961eb7359059 Mon Sep 17 00:00:00 2001 From: Shane Harvey Date: Wed, 31 Jul 2024 09:41:43 -0700 Subject: [PATCH] PYTHON-4588 Don't include invalid port in URI parsing error message (#1753) --- pymongo/uri_parser.py | 17 +++++++++++++++-- test/test_uri_parser.py | 26 ++++++++++++++++++++++---- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/pymongo/uri_parser.py b/pymongo/uri_parser.py index 3607e7d79..7018dad7d 100644 --- a/pymongo/uri_parser.py +++ b/pymongo/uri_parser.py @@ -146,8 +146,21 @@ def parse_host(entity: str, default_port: Optional[int] = DEFAULT_PORT) -> _Addr ) host, port = host.split(":", 1) if isinstance(port, str): - if not port.isdigit() or int(port) > 65535 or int(port) <= 0: - raise ValueError(f"Port must be an integer between 0 and 65535: {port!r}") + if not port.isdigit(): + # Special case check for mistakes like "mongodb://localhost:27017 ". + if all(c.isspace() or c.isdigit() for c in port): + for c in port: + if c.isspace(): + raise ValueError(f"Port contains whitespace character: {c!r}") + + # A non-digit port indicates that the URI is invalid, likely because the password + # or username were not escaped. + raise ValueError( + "Port contains non-digit characters. Hint: username and password must be escaped according to " + "RFC 3986, use urllib.parse.quote_plus" + ) + if int(port) > 65535 or int(port) <= 0: + raise ValueError("Port must be an integer between 0 and 65535") port = int(port) # Normalize hostname to lowercase, since DNS is case-insensitive: diff --git a/test/test_uri_parser.py b/test/test_uri_parser.py index 27f5fd2fb..2a68e9a2c 100644 --- a/test/test_uri_parser.py +++ b/test/test_uri_parser.py @@ -160,10 +160,6 @@ class TestURI(unittest.TestCase): self.assertRaises(InvalidURI, parse_uri, "http://foo@foobar.com") self.assertRaises(ValueError, parse_uri, "mongodb://::1", 27017) - # Extra whitespace should be visible in error message. - with self.assertRaisesRegex(ValueError, "'27017 '"): - parse_uri("mongodb://localhost:27017 ") - orig: dict = { "nodelist": [("localhost", 27017)], "username": None, @@ -536,6 +532,28 @@ class TestURI(unittest.TestCase): self.assertEqual(user, res["username"]) self.assertEqual(pwd, res["password"]) + def test_do_not_include_password_in_port_message(self): + with self.assertRaisesRegex(ValueError, "Port must be an integer between 0 and 65535"): + parse_uri("mongodb://localhost:65536") + with self.assertRaisesRegex( + ValueError, "Port contains non-digit characters. Hint: username " + ) as ctx: + parse_uri("mongodb://user:PASS /@localhost:27017") + self.assertNotIn("PASS", str(ctx.exception)) + + # This "invalid" case is technically a valid URI: + res = parse_uri("mongodb://user:1234/@localhost:27017") + self.assertEqual([("user", 1234)], res["nodelist"]) + self.assertEqual("@localhost:27017", res["database"]) + + def test_port_with_whitespace(self): + with self.assertRaisesRegex(ValueError, "Port contains whitespace character: ' '"): + parse_uri("mongodb://localhost:27017 ") + with self.assertRaisesRegex(ValueError, "Port contains whitespace character: ' '"): + parse_uri("mongodb://localhost: 27017") + with self.assertRaisesRegex(ValueError, r"Port contains whitespace character: '\\n'"): + parse_uri("mongodb://localhost:27\n017") + if __name__ == "__main__": unittest.main()