Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gh-106242: Fix path truncation in normpath #106816

Merged
merged 27 commits into from
Aug 15, 2023
Merged

gh-106242: Fix path truncation in normpath #106816

merged 27 commits into from
Aug 15, 2023

Conversation

finnagin
Copy link
Contributor

@finnagin finnagin commented Jul 17, 2023 •
edited by bedevere-bot
Loading

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.

finnagin and others added 22 commits July 3, 2023 12:38
@finnagin
added _Py_normpathAndSize
d1fe8fa
@finnagin
Merge remote-tracking branch 'upstream/main' into 106242
726da04
@finnagin
PyAPI_FUNC -> extern
8cab42c
@finnagin
remove prinfs
82a9d36
@finnagin
fix length calculation
6c18bf8
@finnagin
length -> norm_size, fix begining null char case
c90249d
@finnagin
fix norm_size calculation
8e1f1ea
@finnagin
add test cases
3003891
@finnagin
Merge remote-tracking branch 'upstream/main' into 106242
2fcae77
@finnagin
move tests to test_genericpath.py
30fa3a4
@finnagin
handle edge cases
f0afe44
@finnagin
decrement p2 when == minp2 & update minp2 handling with leading ./
6412901
@finnagin
change test case to no longer include null character
78192de
@finnagin
Merge remote-tracking branch 'upstream/main' into 106242
199abac
@finnagin
Merge branch 'python:main' into 106242
cbe57c4
@finnagin
Add norm_size variable description, add normpath description
4b9de2c
@finnagin
update normpathAndSize description
f1731f1
@finnagin
norm_size->normsize
d7f7045
@finnagin
Merge remote-tracking branch 'upstream/main' into 106242
184fde9
@finnagin
remove trailing whitespace
9e0d52a
@finnagin
Merge remote-tracking branch 'upstream/main' into 106242
3815967
@finnagin
Merge branch 'python:main' into 106242
d9d78cc
@bedevere-bot bedevere-bot mentioned this pull request Jul 17, 2023
Closed
zooba
zooba reviewed Jul 17, 2023
View reviewed changes
Copy link
Member

@zooba zooba left a 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

Include/internal/pycore_fileutils.h Outdated Show resolved Hide resolved
Include/internal/pycore_fileutils.h Outdated Show resolved Hide resolved
eryksun
eryksun reviewed Jul 17, 2023
View reviewed changes
Lib/test/test_site.py Outdated Show resolved Hide resolved
@finnagin finnagin mentioned this pull request Jul 17, 2023
Closed
@zooba
Copy link
Member

zooba commented Aug 14, 2023

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.

@finnagin @zooba
Update Misc/NEWS.d/next/Library/2023-08-14-23-11-11.gh-issue-106242.7…
aecae84 
…1HMym.rst

Co-authored-by: Steve Dower <steve.dower@microsoft.com>
@zooba zooba merged commit 0932272 into python:main Aug 15, 2023
19 checks passed
@zooba zooba added needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes labels Aug 15, 2023
@miss-islington
Copy link
Contributor

Thanks @finnagin for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @finnagin for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

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

@miss-islington
Copy link
Contributor

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

@bedevere-bot
Copy link

GH-107981 is a backport of this pull request to the 3.12 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.12 bug and security fixes label Aug 15, 2023
@bedevere-bot
Copy link

GH-107982 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Aug 15, 2023
zooba pushed a commit to zooba/cpython that referenced this pull request Aug 15, 2023
@finnagin @zooba
pythongh-106242: Fix path truncation in os.path.normpath (pythonGH-10…
828d1ef 
…6816)
zooba pushed a commit to zooba/cpython that referenced this pull request Aug 15, 2023
@finnagin @zooba
pythongh-106242: Fix path truncation in os.path.normpath (pythonGH-10…
ae5e111 
…6816)
ambv pushed a commit that referenced this pull request Aug 15, 2023
@zooba @finnagin
[3.11] gh-106242: Fix path truncation in os.path.normpath (GH-106816) (…
ccf81e1 
…#107982)

Co-authored-by: Finn Womack <flan313@gmail.com>
iritkatriel pushed a commit to iritkatriel/cpython that referenced this pull request Aug 16, 2023
@finnagin @iritkatriel
pythongh-106242: Fix path truncation in os.path.normpath (pythonGH-10…
3a22494 
…6816)
Yhg1s pushed a commit that referenced this pull request Aug 16, 2023
@zooba @finnagin @serhiy-storchaka
[3.12] gh-106242: Fix path truncation in os.path.normpath (GH-106816) (…
ede9895 
…#107981)

* gh-106242: Fix path truncation in os.path.normpath (GH-106816)
* gh-106242: Minor fixup to avoid compiler warnings

---------

Co-authored-by: Finn Womack <flan313@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@vstinner vstinner mentioned this pull request Nov 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eryksun eryksun eryksun left review comments

@zooba zooba zooba approved these changes

Assignees

@zooba zooba

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@finnagin @zooba @miss-islington @bedevere-bot @eryksun