Skip to content

Correct MAX_N in Lib/zipfile ZipExtFile#144973

Open
jberg5 wants to merge 1 commit intopython:mainfrom
jberg5:fix-max-N
Open

Correct MAX_N in Lib/zipfile ZipExtFile#144973
jberg5 wants to merge 1 commit 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

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

Comments