Skip to content

validate cookie Max-Age as -?digits before int()#12947

Draft
dxbjavid wants to merge 3 commits into
aio-libs:masterfrom
dxbjavid:cookie-maxage-strict-digits
Draft

validate cookie Max-Age as -?digits before int()#12947
dxbjavid wants to merge 3 commits into
aio-libs:masterfrom
dxbjavid:cookie-maxage-strict-digits

Conversation

@dxbjavid

Copy link
Copy Markdown
Contributor

What do these changes do?

CookieJar.update_cookies reads a Set-Cookie Max-Age with a bare int(). Python's int() is more permissive than :rfc:6265#section-5.2.2, which says a Max-Age is "-"? DIGIT+ and any other value must be ignored. So Max-Age=+1000, Max-Age=1_000 and whitespace-padded values are honoured and persist the cookie with the given lifetime, whereas a browser drops the attribute and keeps it as a session cookie. The jar already clears a clearly non-numeric value (Max-Age=string), so this just completes that existing intent by validating against -?[0-9]+ before parsing.

Are there changes in behavior for the user?

A Set-Cookie whose Max-Age is not -?DIGIT+ now leaves the cookie as a session cookie instead of giving it an expiry. Well-formed values, including negative ones, are unchanged.

Is it a substantial burden for the maintainers to support this?

No, it is a small contained check next to the existing parse.

Related issue number

N/A

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt
  • Add a new news fragment into the CHANGES/ folder

@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Jun 17, 2026
@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.96%. Comparing base (db5c238) to head (f5a2fcf).
⚠️ Report is 16 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #12947    +/-   ##
========================================
  Coverage   98.95%   98.96%            
========================================
  Files         131      131            
  Lines       47998    48103   +105     
  Branches     2494     2497     +3     
========================================
+ Hits        47498    47603   +105     
  Misses        376      376            
  Partials      124      124            
Flag Coverage Δ
Autobahn 22.23% <12.50%> (-0.05%) ⬇️
CI-GHA 98.90% <100.00%> (+<0.01%) ⬆️
OS-Linux 98.66% <100.00%> (+<0.01%) ⬆️
OS-Windows 97.04% <100.00%> (+<0.01%) ⬆️
OS-macOS 97.94% <100.00%> (-0.01%) ⬇️
Py-3.10 98.14% <100.00%> (+<0.01%) ⬆️
Py-3.11 98.41% <100.00%> (+<0.01%) ⬆️
Py-3.12 98.50% <100.00%> (+<0.01%) ⬆️
Py-3.13 98.47% <100.00%> (-0.01%) ⬇️
Py-3.14 98.49% <100.00%> (+<0.01%) ⬆️
Py-3.14t 97.58% <100.00%> (-0.01%) ⬇️
Py-pypy-3.11 97.44% <100.00%> (+<0.01%) ⬆️
VM-macos 97.94% <100.00%> (-0.01%) ⬇️
VM-ubuntu 98.66% <100.00%> (+<0.01%) ⬆️
VM-windows 97.04% <100.00%> (+<0.01%) ⬆️
cython-coverage 38.01% <6.25%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@codspeed-hq

codspeed-hq Bot commented Jun 17, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 83 untouched benchmarks
⏩ 83 skipped benchmarks1


Comparing dxbjavid:cookie-maxage-strict-digits (f5a2fcf) with master (7987bd2)

Open in CodSpeed

Footnotes

  1. 83 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@dxbjavid

Copy link
Copy Markdown
Contributor Author

the codecov run flagged test_morsel_with_attributes: a Morsel built in Python can carry an int in max-age (the test sets it directly), and the old int() tolerated that while my regex didn't, so it blew up with a TypeError. pushed a small fix that normalises to str before the rfc check. a programmatic int can't carry the sign/underscore oddities we're guarding against anyway, so the strictness on header-parsed values is unchanged. cookiejar suite is green locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant