GeezerVison is showing github.com — gh-106242: Fix path truncation in normpath by finnagin · Pull Request #106816 · python/cpython
Conversation
finnagin
commented
Jul 17, 2023
•
edited by bedevere-bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Following the suggestion from this comment in the original issue this PR adds a new function _Py_normpathAndSize which behaves like _Py_normpath but also takes a pointer which it will use to store the length of the final normalized path. This new function is then used in os__path_normpath_impl to pass the above mentioned length into PyUnicode_FromWideChar instead of -1.
Additionally, this adds a test to check that normpath is not truncating paths containing a null character as well as changes test_addpackage_import_bad_pth_file in test_site.py which was using a null character to make a bad path, specifically it was using the string 'abc\x00def'. The problem is that before this change this was being truncated for parts of the test so the string it was actually using in those parts was 'abc'. Upon making the normpath changes it started throwing the error ValueError: embedded null character so I changed the test to use 'abc<>$$**:://def' instead.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with the minor name convention update
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
Looks like we just need the NEWS entry - click "Details" next to the failed check for info on it (or run PCbuild/blurb.bat to create it locally).
Text should be something like ``Fixes :func:os.normpath to handle embedded null characters without truncating the path.` and put it under Library.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
Thanks @finnagin for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Thanks @finnagin for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Sorry, @finnagin and @zooba, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 09322724319d4c23195300b222a1c0ea720af56b 3.11
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Sorry, @finnagin and @zooba, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 09322724319d4c23195300b222a1c0ea720af56b 3.12
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
zooba
pushed a commit
to zooba/cpython
that referenced
this pull request
Aug 15, 2023
zooba
pushed a commit
to zooba/cpython
that referenced
this pull request
Aug 15, 2023
Reviewers
eryksun
eryksun left review comments
zooba
zooba approved these changes
Uh oh!
There was an error while loading. Please reload this page.
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|