When attempting to open a ZIP file withZipFile.open(), a BadZipFile exception is raised. The origin of the exception is an unnecessary file name encoding test.

ifzinfo.flag_bits&0x800:# UTF-8 filenamefname_str=fname.decode("utf-8")else:fname_str=fname.decode("cp437")iffname_str!=zinfo.orig_filename:raiseBadZipFile('File name in directory %r and header %r differ.'% (zinfo.orig_filename,fname))

Two clear problems with this test:

The test supports only the UTF-8 andCP437 encodings.
On Windows, the test always fails due to path separator differences.

Two risks with this solution:

This test might be relevant to whomever is using CP437.
This test might be relevant on non-Windows platforms.

This issue has been open and unresolved for nearly 10 years, requiring users to subclass ZipFile when working with ZIP files on Windows platforms. This issue affects every version of Python since at least 2.7.

https://bugs.python.org/issue6839

bpo-6839: removed unnecessary file name encoding test from ZipFile.open

804aaec

Copy link

the-knights-who-say-ni commentedJun 18, 2019

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed thePSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username onbugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please followthe steps outlined in the CPython devguide to rectify this issue.

You cancheck yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

the-knights-who-say-ni added the CLA not signed label

Jun 18, 2019

bedevere-bot added the awaiting review label

Jun 18, 2019

Copy link

Author

MorganRamsay commentedJun 18, 2019

I signed the CLA. My BPO account reflects that, but the Heroku app isn't sync'd. In any case, this PR removes code; it doesn't add any.

Copy link

Contributor

mangrisano commentedJun 19, 2019

/cc@serhiy-storchaka @Yhg1s

the-knights-who-say-ni added CLA signed and removed CLA not signed labels

Jun 20, 2019

Copy link

Member

serhiy-storchaka commentedJun 23, 2019

The test supports only the UTF-8 andCP437 encodings.

Because they are the only official encodings. I do not see a problem here.

On Windows, the test always fails due to path separator differences.

Do you say that reading the ZIP file always fails on Windows? Why our tests are passed and we did not have bug reports about such major bug?

Copy link

Author

MorganRamsay commentedJun 23, 2019•
edited
Loading

Why our tests are passed?

Bad tests?

and we did not have bug reports about such major bug?

So, this doesn't count?https://bugs.python.org/issue6839

FYI: Using test data generated from CryEngine's Resource Compiler, this is what the exception looks like:

Traceback (most recent call last):<snip>  File "D:\dev\Python3_7_2\Lib\zipfile.py", line 1599, in extract    return self._extract_member(member, path, pwd)  File "D:\dev\Python3_7_2\Lib\zipfile.py", line 1669, in _extract_member    with self.open(member, pwd=pwd) as source, \  File "D:\dev\Python3_7_2\Lib\zipfile.py", line 1511, in open    % (zinfo.orig_filename, fname))zipfile.BadZipFile: File name in directory 'Animations/animals/cow/cow.animevents' and header b'Animations\\animals\\cow\\cow.animevents' differ.

Copy link

Member

serhiy-storchaka commentedJun 24, 2019•
edited by bedevere-bot
Loading

Why our tests are passed?
Bad tests?

There are tests for reading ZIP archives containing files in subdirectories. They are passed on Windows. If you think that some tests are missed, could you please provide them?

and we did not have bug reports about such major bug?
So, this doesn't count?https://bugs.python.org/issue6839

Seems this is just one case of ZIP archives generated by some broken software. That software should be fixed.

Please read the discussion onbpo-6839. Even if we makezipfile more lenient, the check should not be just removed, but an exception can be replaced with a warning.