| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
…rgical fix. (pythonGH-123354) Applies changes from zipp 3.20.1 and jaraco/zippGH-124 (cherry picked from commit 2231286) (cherry picked from commit 17b77bb) Co-authored-by: Jason R. Coombs <jaraco@jaraco.com>
|
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
----------------------------------------------------------------------
|
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
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
----------------------------------------------------------------------
|
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
@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:
How would you like me to proceed? |
Sorry, something went wrong.
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 |
Sorry, something went wrong.
|
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? |
Sorry, something went wrong.
|
No problem. I'll get it done today. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
This reverts commit d1eca1e.
|
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. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Applies changes from zipp 3.20.1 and jaraco/zippGH-124 (cherry picked from commit 2231286) (cherry picked from commit 17b77bb)