Skip to content

gh-84353: Change the first-attempt zipfile encoding to cp437 #104347

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

Jump to bottom
Closed
arhadthedev wants to merge 1 commit into from
Closed

gh-84353: Change the first-attempt zipfile encoding to cp437 #104347

arhadthedev wants to merge 1 commit into from

Conversation

@arhadthedev
Copy link
Member

@arhadthedev arhadthedev commented May 10, 2023 •
edited by bedevere-bot
Loading

@bedevere-bot bedevere-bot mentioned this pull request May 10, 2023
Open
@arhadthedev
Copy link
Member Author

arhadthedev commented May 10, 2023

This test fails:

cpython/Lib/test/test_zipfile/test_core.py

Lines 2983 to 2984 in 22f3425

with zipfile.ZipFile(TESTFN, "r", metadata_encoding='shift_jis') as zipfp:
self.assertEqual(sorted(zipfp.namelist()), sorted(expected_names)) 

======================================================================
FAIL: test_read_after_append (test.test_zipfile.test_core.EncodedMetadataTests.test_read_after_append)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vsts/work/1/s/Lib/test/test_zipfile/test_core.py", line 2984, in test_read_after_append
    self.assertEqual(sorted(zipfp.namelist()), sorted(expected_names))
AssertionError: Lists differ: ['一', '三', '二', '四'] != ['êΩ', 'ô±', '三', '四']

First differing element 0:
'一'
'êΩ'

Relevant declarations:

cpython/Lib/test/test_zipfile/test_core.py

Lines 2970 to 2973 in 22f3425

newname = '\u56db' # Han 'four'
expected_names = [name.encode('shift_jis').decode('cp437')
for name in self.file_names[:2]] + self.file_names[2:]
expected_names.append(newname)

cpython/Lib/test/test_zipfile/test_core.py

Line 2896 in 22f3425

file_names = ['\u4e00', '\u4e8c', '\u4e09'] # Han 'one', 'two', 'three'

@arhadthedev arhadthedev mentioned this pull request May 10, 2023
Closed
gpshead
gpshead reviewed May 10, 2023
View reviewed changes
Lib/zipfile/__init__.py
def _encodeFilenameFlags(self):
try:
return self.filename.encode('ascii'), self.flag_bits
return self.filename.encode('cp437'), self.flag_bits
Copy link
Member

@gpshead gpshead May 10, 2023

Choose a reason for hiding this comment

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

This would need to add the removal of the UTF-8 bit from flag_bits again... which would re-create this regression. #95463

I'll follow up on the issue.

@gpshead gpshead marked this pull request as draft May 10, 2023 05:26
@arhadthedev
Copy link
Member Author

arhadthedev commented May 10, 2023

Should I close this PR since the parent issue is closed?

@arhadthedev
Copy link
Member Author

arhadthedev commented May 18, 2023

From #84353 (comment):

If we went forward with that PR #104347, we'd be re-creating the since fixed #95463 regression from 3.11 again.

Thus closing the PR.

@arhadthedev arhadthedev deleted the zipfile-cp437 branch May 18, 2023 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead left review comments

Assignees

No one assigned

Labels

awaiting review DO-NOT-MERGE

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@arhadthedev @gpshead @bedevere-bot