PYTHON-4588 Don't include invalid port in URI parsing error message (mongodb#1753)

Author: ShaneHarvey

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:

test/test_uri_parser.py

@@ -160,10 +160,6 @@ def test_parse_uri(self):
         self.assertRaises(InvalidURI, parse_uri, "http://[email protected]")
         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 @@ def test_special_chars(self):
         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()