GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-106242: Fix path truncation in normpath by finnagin · Pull Request #106816 · python/cpython

Conversation

@finnagin

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

  • Issue: os.path.normpath truncates input on null bytes in 3.11, but not 3.10 #106242

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
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 added the awaiting review label Jul 17, 2023
@bedevere-bot bedevere-bot mentioned this pull request Jul 17, 2023
os.path.normpath truncates input on null bytes in 3.11, but not 3.10 #106242
Closed
@AlexWaygood
AlexWaygood requested review from eryksun and zooba July 17, 2023 00:04
zooba
zooba reviewed Jul 17, 2023
View reviewed changes

@zooba zooba left a comment

Copy link
Copy Markdown
Member

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.

All reactions
Comment thread Include/internal/pycore_fileutils.h Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Include/internal/pycore_fileutils.h Resolved

Uh oh!

There was an error while loading. Please reload this page.

eryksun
eryksun reviewed Jul 17, 2023
View reviewed changes
Comment thread Lib/test/test_site.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@finnagin
_Py_normpathAndSize -> _Py_normpath_and_size
3ef9e3b
@finnagin finnagin mentioned this pull request Jul 17, 2023
_winapi.LCMapStringEx fails when encountering a string containing null characters #106844
Closed
finnagin added 2 commits July 17, 2023 16:19
@finnagin
revert test_addpackage_import_bad_pth_file change
c885d13
@finnagin
merge upstream/main, fix merge conflicts
d896d45
@zooba

zooba commented Aug 14, 2023

Copy link
Copy Markdown
Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@blurb-it
📜🤖 Added by blurb_it.
0959e2b
zooba
zooba reviewed Aug 14, 2023
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2023-08-14-23-11-11.gh-issue-106242.71HMym.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@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 <[email protected]>
zooba
zooba approved these changes Aug 14, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Aug 14, 2023
@zooba
zooba merged commit 0932272 into python:main Aug 15, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Aug 15, 2023
@zooba zooba added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Aug 15, 2023
@miss-islington

miss-islington commented Aug 15, 2023

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington

miss-islington commented Aug 15, 2023

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington

miss-islington commented Aug 15, 2023

Copy link
Copy Markdown
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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington miss-islington assigned zooba Aug 15, 2023
@miss-islington

miss-islington commented Aug 15, 2023

Copy link
Copy Markdown
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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot

bedevere-bot commented Aug 15, 2023

Copy link
Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the needs backport to 3.12 only security fixes label Aug 15, 2023
@bedevere-bot

bedevere-bot commented Aug 15, 2023

Copy link
Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@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 <[email protected]>
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 <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
@vstinner vstinner mentioned this pull request Nov 3, 2023
Embedded null characters can lead to bugs or even security vulnerabilities #111656
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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@zooba zooba

Labels

None yet

Projects

None yet

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.

5 participants

@finnagin @zooba @miss-islington @bedevere-bot @eryksun
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.