GeezerVison

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

GeezerVison is showing github.com — gh-82367: Use `FindFirstFile` Win32 API in `ntpath.realpath()` by moonsikpark · Pull Request #110298 · python/cpython

Conversation

moonsikpark commented Oct 3, 2023 •

edited by github-actions Bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

ntpath.realpath() breaks out of traversing a series of paths where a (handled) ERROR_ACCESS_DENIED or ERROR_SHARING_VIOLATION occurs.

Using FindFirstFile Win32 API, we can query their real name to correct the case and also resolve MS-DOS (also called 8.3) style file names such as C:\\LONGFI~1.TXT.

Issue: Use FindFirstFile in ntpath.realpath() when access is denied #82367

📚 Documentation preview 📚: https://cpython-previews--110298.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

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

All reactions

bedevere-app Bot added the awaiting review label

Oct 3, 2023

bedevere-app Bot mentioned this pull request

Oct 3, 2023

Use FindFirstFile in ntpath.realpath() when access is denied #82367

Closed

zware reviewed

Oct 3, 2023

View reviewed changes

zware 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.

A few documentation nits, but mostly this looks pretty good to me. It will need approval from either @zooba or @eryksun before I'd be comfortable merging it, though.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Doc/whatsnew/3.13.rst Resolved

Uh oh!

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

Comment thread

Misc/NEWS.d/next/Windows/2023-10-03-12-30-59.gh-issue-82367.nxwfMx.rst Resolved

Uh oh!

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

Comment thread

Doc/library/os.path.rst Resolved

Uh oh!

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

zware requested review from eryksun and zooba

October 3, 2023 17:28

zooba commented Oct 3, 2023

Copy link

Copy Markdown

Member

The code change looks great to me. Awesome job!

With Zach's requested doc changes, I think this is good to merge.

All reactions

Sorry, something went wrong.

Uh oh!

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

moonsikpark force-pushed the gh-82367-use-findfirstfile-in-nt-realpath branch from 2c49ffc to 4590347 Compare

October 4, 2023 11:38

moonsikpark added a commit to moonsikpark/cpython that referenced this pull request

Oct 4, 2023


          pythongh-82367: Use FindFirstFile Win32 API in ntpath.realpath() (p…

f675a4f

…ython#110298)

* Use `FindFirstFile` Win32 API to fix a bug where `ntpath.realpath()`
breaks out of traversing a series of paths where a (handled)
`ERROR_ACCESS_DENIED` or `ERROR_SHARING_VIOLATION` occurs.
* Update docs to reflect that `ntpath.realpath()` eliminates MS-DOS
style names.

moonsikpark force-pushed the gh-82367-use-findfirstfile-in-nt-realpath branch from 4590347 to f675a4f Compare

October 4, 2023 11:50

ghost commented Oct 4, 2023 •

edited by ghost

Loading

Uh oh!

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

Copy link

Copy Markdown

All commit authors signed the Contributor License Agreement.

All reactions

Sorry, something went wrong.

Uh oh!

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

moonsikpark added a commit to moonsikpark/cpython that referenced this pull request

Oct 4, 2023


          pythongh-82367: Use FindFirstFile Win32 API in ntpath.realpath() (p…

e17852c

…ython#110298)

* Use `FindFirstFile` Win32 API to fix a bug where `ntpath.realpath()`
breaks out of traversing a series of paths where a (handled)
`ERROR_ACCESS_DENIED` or `ERROR_SHARING_VIOLATION` occurs.
* Update docs to reflect that `ntpath.realpath()` eliminates MS-DOS
style names.

moonsikpark force-pushed the gh-82367-use-findfirstfile-in-nt-realpath branch from d88c757 to e17852c Compare

October 4, 2023 14:41

eryksun reviewed

Oct 4, 2023

View reviewed changes

Comment thread

Lib/ntpath.py Resolved

Uh oh!

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

eryksun reviewed

Oct 5, 2023

View reviewed changes

Comment thread

Lib/test/test_ntpath.py Resolved

Uh oh!

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

eryksun reviewed

Oct 5, 2023

View reviewed changes

Comment thread

Lib/test/test_ntpath.py Resolved

Uh oh!

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

eryksun reviewed

Oct 5, 2023

View reviewed changes

Comment thread

Lib/test/test_ntpath.py Resolved

Uh oh!

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

eryksun reviewed

Oct 5, 2023

View reviewed changes

Comment thread

Lib/test/test_ntpath.py Resolved

Uh oh!

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

eryksun reviewed

Oct 5, 2023

View reviewed changes

Comment thread

Lib/test/test_ntpath.py Resolved

Uh oh!

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

eryksun reviewed

Oct 5, 2023

View reviewed changes

Comment thread

Lib/test/test_ntpath.py Resolved

Uh oh!

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

eryksun approved these changes

Oct 5, 2023

View reviewed changes

eryksun left a comment

Copy link

Copy Markdown

Contributor

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. Thank you, Moonsik, and thanks also for taking the extra time to make the test more robust.

Sorry, something went wrong.

Uh oh!

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

moonsikpark and zware reacted with thumbs up emoji

zware reacted with heart emoji

All reactions

👍 2 reactions
❤️ 1 reaction

bedevere-app Bot added awaiting core review and removed awaiting review labels

Oct 5, 2023


          pythongh-82367: Use FindFirstFile Win32 API in ntpath.realpath() (p…

2b5a29e

…ython#110298)

* Use `FindFirstFile` Win32 API to fix a bug where `ntpath.realpath()`
breaks out of traversing a series of paths where a (handled)
`ERROR_ACCESS_DENIED` or `ERROR_SHARING_VIOLATION` occurs.
* Update docs to reflect that `ntpath.realpath()` eliminates MS-DOS
style names.

moonsikpark force-pushed the gh-82367-use-findfirstfile-in-nt-realpath branch from 1766bc3 to 2b5a29e Compare

October 5, 2023 08:38

zware merged commit d33aa18 into python:main

Oct 5, 2023

bedevere-app Bot removed the awaiting core review label

Oct 5, 2023

zware commented Oct 5, 2023

Copy link

Copy Markdown

Member

Thanks Steve and Eryk for the reviews, and Moonsik for the patch and excellent collaboration!

moonsikpark reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

moonsikpark deleted the gh-82367-use-findfirstfile-in-nt-realpath branch

October 5, 2023 17:10

moonsikpark restored the gh-82367-use-findfirstfile-in-nt-realpath branch

October 6, 2023 02:17

moonsikpark deleted the gh-82367-use-findfirstfile-in-nt-realpath branch

October 6, 2023 02:20

moonsikpark mentioned this pull request

Oct 7, 2023

Ensure ntpath.realpath() result is a final, normalized pathname. #110495

Open

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request

Sep 2, 2024


          pythongh-82367: Use FindFirstFile Win32 API in ntpath.realpath() (p…

5140b4f

…ythonGH-110298)

* Use `FindFirstFile` Win32 API to fix a bug where `ntpath.realpath()`
breaks out of traversing a series of paths where a (handled)
`ERROR_ACCESS_DENIED` or `ERROR_SHARING_VIOLATION` occurs.
* Update docs to reflect that `ntpath.realpath()` eliminates MS-DOS
style names.

encukou added a commit to encukou/cpython that referenced this pull request

Jun 3, 2025


          [3.12] pythongh-135034: Remove test_realpath_permission

ac560d1

The test was added in pythongh-110298, with a fix that was never backported
to 3.12 and below.
It was most likely skipped in the GHA run.

encukou mentioned this pull request

Jun 3, 2025

[3.12] gh-135034: Remove test_realpath_permission #135093

Merged

ambv pushed a commit that referenced this pull request

Jun 3, 2025


          [3.12] gh-135034: Remove test_realpath_permission (GH-135093)

28463db

The test was added in gh-110298, with a fix that was never backported
to 3.12 and below.
It was most likely skipped in the GHA run.

Reviewers

zware

zware left review comments

eryksun

eryksun approved these changes

zooba

Awaiting requested review from zooba

Uh oh!

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

Assignees

No one assigned

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.

4 participants

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.

GeezerVison

Conversation

moonsikpark commented Oct 3, 2023 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zware left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zooba commented Oct 3, 2023

Uh oh!

ghost commented Oct 4, 2023 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eryksun left a comment

Choose a reason for hiding this comment

Uh oh!

zware commented Oct 5, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

moonsikpark commented Oct 3, 2023 •

edited by github-actions Bot

Loading

Uh oh!

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

ghost commented Oct 4, 2023 •

edited by ghost

Loading

Uh oh!

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