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.

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

fix(ext/node): Fix fs.access/fs.promises.access with X_OK mode parameter on Windows #27407

Merged
merged 5 commits into from
Dec 18, 2024

Conversation

filipstev
Copy link
Contributor

@filipstev filipstev commented Dec 17, 2024

  • Fixes an issue on Windows where the fs.constants.X_OK flag caused fs.access and fs.promises.access to incorrectly throw a "permission denied" error
  • Introduced formatting changes due to the formatting tool
  • Fixed the issue by always removing the X_OK bit from the mode variable m (not sure if it's necessary to check for the presence of it in the mode param first?)
  • Updated unit tests to handle the mentioned constant
  • X_OK bit is ignored in the Node implementation and should behave like F_OK

fs constants Node documentation: https://nodejs.org/api/fs.html#fsconstants

fixes #27405

@filipstev filipstev marked this pull request as ready for review December 17, 2024 23:27
@filipstev filipstev changed the title fix(ext/node): Fix fs.access/fs.promises.access on Windows fix(ext/node): Fix fs.access/fs.promises.access with X_OK mode parameter on Windows Dec 17, 2024
@filipstev
Copy link
Contributor Author

Hey @bartlomieju, not sure who I should ping for a review, happy to hear the teams opinion on this issue 🫡

Copy link
Member

@nathanwhit nathanwhit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@nathanwhit nathanwhit merged commit 8fc4796 into denoland:main Dec 18, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

node compat: fs.access/fs.promises.access seems broken
2 participants