Skip to content

Comments

Correct MAX_N in Lib/zipfile ZipExtFile#144973

Merged
serhiy-storchaka merged 2 commits intopython:mainfrom
jberg5:fix-max-N
Feb 19, 2026
Merged

Correct MAX_N in Lib/zipfile ZipExtFile#144973
serhiy-storchaka merged 2 commits intopython:mainfrom
jberg5:fix-max-N

Conversation

@jberg5
Copy link
Contributor

@jberg5 jberg5 commented Feb 18, 2026

This is so small that I didn't think it was worth making an issue, but let me know if you disagree!

Basically it looks like we've had this (extremely minor) operator precedence bug since 2010. I am 99% sure the original implementation intended to set MAX_N to be (2^31)-1, but ended up with (2^30), since

>>> 1 << 31 - 1
1073741824
>>> (1 << 31) -1
2147483647

It's hard to imagine a scenario where this would have made a difference. Maybe a highly compressed file that expands to 2gb previously was taking two iterations to process when it could have been done in one? But at that point I imagine you wouldn't really notice the additional overhead.

That said, it feels like fixing this is the right thing to do.

AI use disclaimer: I had claude (with Opus 4.6) act as a sort of code reviewer and scan the standard library for issues, and it immediately spotted this one.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. 👍 Good catch! 🐟

Could you please also fix other cases in tests (even in comments)?

Lib/test/test_compile.py:            g = +9223372036854775807  # 1 << 63 - 1
Lib/test/test_compile.py:            h = -9223372036854775807  # 1 << 63 - 1
Lib/test/test_unpack_ex.py:    >>> s = ", ".join("a%d" % i for i in range(1<<8)) + ", *rest = range(1<<8 + 1)"
Lib/test/test_unpack_ex.py:    >>> s = ", ".join("a%d" % i for i in range(1<<8 + 1)) + ", *rest = range(1<<8 + 2)"
Lib/zipfile/__init__.py:    MAX_N = 1 << 31 - 1

@jberg5
Copy link
Contributor Author

jberg5 commented Feb 19, 2026

Thanks @serhiy-storchaka ! Have addressed the others that you mentioned.

@serhiy-storchaka serhiy-storchaka merged commit 4141f0a into python:main Feb 19, 2026
47 of 48 checks passed
@miss-islington-app
Copy link

Thanks @jberg5 for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 19, 2026
"<<" has lower precedence than "-".
(cherry picked from commit 4141f0a)

Co-authored-by: J Berg <j.berg2349@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 19, 2026
"<<" has lower precedence than "-".
(cherry picked from commit 4141f0a)

Co-authored-by: J Berg <j.berg2349@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Feb 19, 2026

GH-145022 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Feb 19, 2026
@bedevere-app
Copy link

bedevere-app bot commented Feb 19, 2026

GH-145023 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Feb 19, 2026
serhiy-storchaka pushed a commit that referenced this pull request Feb 19, 2026
"<<" has lower precedence than "-".
(cherry picked from commit 4141f0a)

Co-authored-by: J Berg <j.berg2349@gmail.com>
serhiy-storchaka pushed a commit that referenced this pull request Feb 19, 2026
"<<" has lower precedence than "-".
(cherry picked from commit 4141f0a)

Co-authored-by: J Berg <j.berg2349@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants