Skip to content

Comments

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

Closed
arhadthedev wants to merge 1 commit intopython:mainfrom
arhadthedev:zipfile-cp437
Closed

gh-84353: Change the first-attempt zipfile encoding to cp437#104347
arhadthedev wants to merge 1 commit intopython:mainfrom
arhadthedev:zipfile-cp437

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