8000 gh-135993: Fix IPv6 bug in `set_ok_port` and `return_ok_port` by LamentXU123 · Pull Request #136076 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-135993: Fix IPv6 bug in set_ok_port and return_ok_port #136076

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.

Sign up for GitHub

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

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

gh-135993: Fix IPv6 bug in set_ok_port and return_ok_port #136076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
8680fa4
Update uuid.rst
LamentXU123 Jun 18, 2025
b911f1e
Update uuid.rst
LamentXU123 Jun 18, 2025
c69a050
Update Doc/library/uuid.rst
LamentXU123 Jun 19, 2025
b55a920
Update uuid.rst
LamentXU123 Jun 20, 2025
f129053
sync uuid1 & uuid6 docs
picnixz Jun 20, 2025
48064f2
Update cookiejar.py
LamentXU123 Jun 28, 2025
e1fe548
Update test_http_cookiejar.py
LamentXU123 Jun 28, 2025
f6859dd
Merge branch 'python:main' into Fix-IPv6-bug-in-set&return-port-ok
LamentXU123 Jun 28, 2025
05597f8
📜🤖 Added by blurb_it.
blurb-it[bot] Jun 28, 2025
c5ca056
add more test for HDNs
LamentXU123 Jun 28, 2025
854422d
add False test
LamentXU123 Jun 28, 2025
428096f
Update test_http_cookiejar.py
LamentXU123 Jun 28, 2025
312f66f
Update Misc/NEWS.d/next/Library/2025-06-28-14-10-07.gh-issue-135993.G…
LamentXU123 Jun 29, 2025
94bb9ab
remove useless `c.clear()`
LamentXU123 Jun 29, 2025
24eb5d9
Update test_http_cookiejar.py
LamentXU123 Jun 29, 2025
0415d3f
Update 2025-06-28-14-10-07.gh-issue-135993.Gmyux9.rst
LamentXU123 Jun 29, 2025
357b9d1
Update test_http_cookiejar.py
LamentXU123 Jul 11, 2025
1034647
change to match[0]
LamentXU123 Jul 11, 2025
9d75634
Update cookiejar.py
LamentXU123 Jul 12, 2025
b7e859f
Update cookiejar.py
LamentXU123 Jul 12, 2025
b7d6e4e
Update cookiejar.py
LamentXU123 Jul 12, 2025
d197796
prevent to remove the debug message
LamentXU123 Jul 14, 2025
bc8bad0
Update 2025-06-28-14-10-07.gh-issue-135993.Gmyux9.rst
LamentXU123 Jul 14, 2025
7e3237a
Fix CI
LamentXU123 Jul 14, 2025
cdda526
Add stricter check for IPv6 case
LamentXU123 Jul 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 9 additions & 16 deletions Lib/http/cookiejar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -656,16 +656,15 @@ def request_path(request):
return path

def request_port(request):
host = request.host
i = host.find(':')
if i >= 0:
port = host[i+1:]
try:
int(port)
except ValueError:
_debug("nonnumeric port: '%s'", port)
return None
match = cut_port_re.search(request.host)
if match:
port = match[0].removeprefix(':')
else:
i = request.host.rfind(':')
if (i >= 0
and not ']' in request.host[i+1:]
and not request.host.startswith('[')): # to prevent IPv6 addresses
_debug("nonnumeric port: '%s'", request.host[i+1:])
Copy link
Contributor Author
@LamentXU123 LamentXU123 Jul 14, 2025 •
edited 8000
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is added to print debug message in the original code. ValueError is no longer to be raised, but we still need debug message here.

Copy link
Contributor Author
@LamentXU123 LamentXU123 Jul 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm adding stricter check to this. There may be case like www.acme.com:1]34, and 1]34 is not a normal port and needs debug message, so I think I would add a check that the host name must starts with [

port = DEFAULT_HTTP_PORT
return port

Expand Down Expand Up @@ -1075,11 +1074,7 @@ def set_ok_domain(self, cookie, request):

def set_ok_port(self, cookie, request):
if cookie.port_specified:
req_port = request_port(request)
if req_port is None:
Copy link
Contributor Author
@LamentXU123 LamentXU123 Jul 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remove this also, since request_port() will no longer return None.

Note: In fact the original code here is also wrong. the req_port should be DEFAULT_HTTP_PORT instead of 80. nvm, we just remove this logic. I will create a issue and PR later for this. This and the case below are the only case of using 80 instead of the const.

req_port = "80"
else:
req_port = str(req_port)
req_port = str(request_port(request))
for p in cookie.port.split(","):
try:
int(p)
Expand Down Expand Up @@ -1148,8 +1143,6 @@ def return_ok_expires(self, cookie, request):
def return_ok_port(self, cookie, request):
if cookie.port:
req_port = request_port(request)
if req_port is None:
req_port = "80"
for p in cookie.port.split(","):
if p == req_port:
break
Expand Down
79 changes: 79 additions & 0 deletions Lib/test/test_http_cookiejar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,19 @@ def test_request_port(self):
req = urllib.request.Request("http://www.acme.com/",
headers={"Host": "www.acme.com:4321"})
self.assertEqual(request_port(req), DEFAULT_HTTP_PORT)
req = urllib.request.Request("http://www.acme.com:",
headers={"Host": "www.acme.com:4321"})
self.assertEqual(request_port(req), DEFAULT_HTTP_PORT)
req = urllib.request.Request("http://www.acme.com:not_a_port",
headers={"Host": "www.acme.com:4321"})
self.assertEqual(request_port(req), DEFAULT_HTTP_PORT)
req = urllib.request.Request("http://[::1]:1234",
headers={"Host": "[::1]:4321"})
self.assertEqual(request_port(req), "1234")
req = urllib.request.Request("http://[::1]",
headers={"Host": "[::1]:4321"})
self.assertEqual(request_port(req), DEFAULT_HTTP_PORT)


def test_request_host(self):
# this request is illegal (RFC2616, 14.2.3)
Expand Down Expand Up @@ -1262,6 +1275,72 @@ def test_missing_final_slash(self):
c.add_cookie_header(req)
self.assertTrue(req.has_header("Cookie"))

def test_set_ok_port(self):
pol = DefaultCookiePolicy()
c = CookieJar(policy=pol)
headers_with_port_1234 = ["Set-Cookie: CUSTOMER=WILE_E_COYOTE; path=/; port=1234"]
headers_with_default_port = ["Set-Cookie: CUSTOMER=WILE_E_COYOTE; path=/; port=80"]
req = urllib.request.Request("http://127.0.0.1:1234")
res = FakeResponse(headers_with_port_1234, "http://127.0.0.1:1234")
self.assertTrue(pol.set_ok_port(c.make_cookies(res, req)[0], req))

req = urllib.request.Request("http://acme.com:1234")
res = FakeResponse(headers_with_port_1234, "http://acme.com:1234")
self.assertTrue(pol.set_ok_port(c.make_cookies(res, req)[0], req))

req = urllib.request.Request("http://[::1]:1234")
res = FakeResponse(headers_with_port_1234, "http://[::1]:1234")
self.assertTrue(pol.set_ok_port(c.make_cookies(res, req)[0], req))

req = urllib.request.Request("http://[::1]:1235")
res = FakeResponse(headers_with_port_1234, "http://[::1]:1235")
self.assertFalse(pol.set_ok_port(c.make_cookies(res, req)[0], req))

req = urllib.request.Request("http://acme.com:")
res = FakeResponse(headers_with_default_port, "http://acme.com:")
self.assertTrue(pol.set_ok_port(c.make_cookies(res, req)[0], req))
res = FakeResponse(headers_with_port_1234, "http://acme.com:")
self.assertFalse(pol.set_ok_port(c.make_cookies(res, req)[0], req))

req = urllib.request.Request("http://acme.com:not_a_port")
res = FakeResponse(headers_with_default_port, "http://acme.com:not_a_port")
self.assertTrue(pol.set_ok_port(c.make_cookies(res, req)[0], req))
res = FakeResponse(headers_with_port_1234, "http://acme.com:not_a_port")
self.assertFalse(pol.set_ok_port(c.make_cookies(res, req)[0], req))

def test_return_ok_port(self):
pol = DefaultCookiePolicy()
c = CookieJar(policy=pol)
headers_with_port_1234 = ["Set-Cookie: CUSTOMER=WILE_E_COYOTE; path=/; port=1234"]
headers_with_default_port = ["Set-Cookie: CUSTOMER=WILE_E_COYOTE; path=/; port=80"]
req = urllib.request.Request("http://127.0.0.1:1234")
res = FakeResponse(headers_with_port_1234, "http://127.0.0.1:1234")
self.assertTrue(pol.return_ok_port(c.make_cookies(res, req)[0], req))

req = urllib.request.Request("http://acme.com:1234")
res = FakeResponse(headers_with_port_1234, "http://acme.com:1234")
self.assertTrue(pol.return_ok_port(c.make_cookies(res, req)[0], req))

req = urllib.request.Request("http://[::1]:1234")
res = FakeResponse(headers_with_port_1234, "http://[::1]:1234")
self.assertTrue(pol.return_ok_port(c.make_cookies(res, req)[0], req))

req = urllib.request.Request("http://[::1]:1235")
res = FakeResponse(headers_with_port_1234, "http://[::1]:1235")
self.assertFalse(pol.return_ok_port(c.make_cookies(res, req)[0], req))

req = urllib.request.Request("http://acme.com:")
res = FakeResponse(headers_with_default_port, "http://acme.com:")
self.assertTrue(pol.return_ok_port(c.make_cookies(res, req)[0], req))
res = FakeResponse(headers_with_port_1234, "http://acme.com:")
self.assertFalse(pol.return_ok_port(c.make_cookies(res, req)[0], req))

req = urllib.request.Request("http://acme.com:not_a_port")
res = FakeResponse(headers_with_default_port, "http://acme.com:not_a_port")
self.assertTrue(pol.return_ok_port(c.make_cookies(res, req)[0], req))
res = FakeResponse(headers_with_port_1234, "http://acme.com:not_a_port")
self.assertFalse(pol.return_ok_port(c.make_cookies(res, req)[0], req))

def test_domain_mirror(self):
pol = DefaultCookiePolicy(rfc2965=True)

Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2025-06-28-14-10-07.gh-issue-135993.Gmyux9.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
:mod:`http.cookiejar`: Fix a bug that occurs when :class:`~http.cookiejar.DefaultCookiePolicy`
attempts to parse the port of an IPv6 address in func ``request_port()``. Patch by Weilin Du.
Loading
0