GeezerVison

The tested assertions are failing in the backport on Windows:

======================================================================
FAIL: test_backslash_not_separator (test.test_zipfile.TestPath)
In a zip file, backslashes are not separators.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\cpython\cpython\lib\test\test_zipfile.py", line 3329, in test_backslash_not_separator
    assert first.name == 'foo\\bar'
AssertionError

======================================================================
FAIL: test_unsupported_names (test.test_zipfile.TestPath)
Path segments with special characters are readable.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\cpython\cpython\lib\test\test_zipfile.py", line 3315, in test_unsupported_names
    assert next(contents).name == 'V: NMS.flac'
AssertionError

----------------------------------------------------------------------

Since these same tests pass on zipp on Windows+Python 3.10, it must be the case that something else has changed in zipp that wasn't backported to Python 3.10. But what? I applied the latest code from zipp to CPython, and the diff doesn't reveal anything that would explain the failing tests.

Adding some additional information to the troubleshooting, it's slightly more clear what's happening:

======================================================================
FAIL: test_backslash_not_separator (test.test_zipfile.TestPath)
In a zip file, backslashes are not separators.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\cpython\cpython\lib\test\test_zipfile.py", line 3330, in test_backslash_not_separator
    assert first.name == 'foo\\bar', first.name
AssertionError: bar
======================================================================
FAIL: test_unsupported_names (test.test_zipfile.TestPath)
Path segments with special characters are readable.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\cpython\cpython\lib\test\test_zipfile.py", line 3316, in test_unsupported_names
    assert item.name == 'V: NMS.flac', item.name
AssertionError:  NMS.flac
----------------------------------------------------------------------

Aha! That diff is relevant in how it handles .name on Windows (pathlib.Path vs. pathlib.PurePosixPath) as applied in jaraco/zipp@9d6e639. So these tests can't be used in the backport. That's fine, as these tests are there to guarantee expectation in main and aren't pertinent to the security issue.

@pablogsal In addition to approving/merging this security fix, I'd like your opinion.

In troubleshooting why the two new tests were failing, I learned that on Python 3.10 only, zipfile.Path.name is incorrect for names containing characters specific to the platform's file system (i.e. : and \ on Windows). Probably nobody is going to care about it, but it's conceivable it could lead to a security issue. Trivially, files that are present in the zipfile will appear to be missing on Windows, and potentially someone could craft a zipfile that could cause more serious harm, all because pathlib.Path is used to derive the .name of the path, producing invalid and unexpected results.

I can think of a few options:

• track the issue as a separate security issue and patch it separately.
• patch the issue as part of this fix, making the behavior consistent with Python 3.9 and 3.11.
• (default) disregard the concern for now, leaving out the tests that caught the concern.

How would you like me to proceed?

• track the issue as a separate security issue and patch it separately.

I would prefer to do that. The reason is that it's unclear if that would affect anything else and I would like more eyes on this because 3.10 it's long into security-only so I don't want to risk this. I personally would prefer to keep it consistent with 3.9 and 3.11 if it's related to the security fix but I want to see what others think

@ambv @Yhg1s what are your thoughts?

Agree this should be patched separately. @jaraco, the security releases will go out on Friday. Would it be possible to get a fix before then?

No problem. I'll get it done today.

I'll revise this PR to be acceptable as-is with the expectation that the additional issue will be addressed to re-enable the tests.

This patch now retains the tests but marks the two new ones as skipped on Windows, allowing the test suite to pass. A follow up GH-123694 promises to fix the emergent issue and restore the tests.