Skip to content

fs: set path when mkdir recursive called on file - #31607

Closed
bcoe wants to merge 1 commit into
nodejs:masterfrom
bcoe:fix-28015
Closed

fs: set path when mkdir recursive called on file#31607
bcoe wants to merge 1 commit into
nodejs:masterfrom
bcoe:fix-28015

Conversation

@bcoe

@bcoebcoe commented Feb 1, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

Similar fix to #31505, we should bail early if mkdir recursive is called on a file path.

Fixes: #28015

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. labels Feb 1, 2020
@bcoebcoe added the fast-track PRs that do not need to wait for 48 hours to land. label Feb 1, 2020
@bcoe

bcoe commented Feb 1, 2020

Copy link
Copy Markdown
ContributorAuthor

If folks are open to it, I would love to fast track this fix.

@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

This comment has been minimized.

Sign in to view
richardlau
richardlau reviewed Feb 1, 2020
View reviewed changes
Comment threadtest/parallel/test-fs-mkdir.js Outdated
@bcoebcoe mentioned this pull request Feb 2, 2020
Closed
bcoe
bcoe commented Feb 2, 2020
View reviewed changes
Comment threadsrc/node_file.cc
}
// verify that the path pointed to is actually a directory.
if (err == 0 && !S_ISDIR(req->statbuf.st_mode)) err = UV_EEXIST;
uv_fs_req_cleanup(req);

@bcoebcoeFeb 2, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

We were calling uv_fs_req_cleanup prematurely on Windows systems, and freeing req->path.

uv_fs_req_cleanup is called in FSReqAfterScope::~FSReqAfterScope().

@nodejs-github-bot

This comment has been minimized.

Sign in to view
@bcoebcoe added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 2, 2020
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot

nodejs-github-bot commented Feb 3, 2020•
edited by bcoe
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28889/ ✅

@bcoe

bcoe commented Feb 3, 2020

Copy link
Copy Markdown
ContributorAuthor

@richardlau mind if I land?

@richardlau

richardlau commented Feb 3, 2020

Copy link
Copy Markdown
Member

@richardlau mind if I land?

Go for it 😀.

bcoe pushed a commit that referenced this pull request Feb 3, 2020
bcoe
fs: set path when mkdir recursive called on file
3f70d77
PR-URL: #31607Fixes: #28015
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@bcoe

bcoe commented Feb 3, 2020

Copy link
Copy Markdown
ContributorAuthor

Landed in 3f70d77

@bcoebcoe closed this Feb 3, 2020
@bcoe
bcoe deleted the fix-28015 branch February 3, 2020 22:33
codebytere pushed a commit that referenced this pull request Feb 17, 2020
bcoe@codebytere
fs: set path when mkdir recursive called on file
21fc818
PR-URL: #31607Fixes: #28015
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@codebyterecodebytere mentioned this pull request Feb 17, 2020
Merged
codebytere pushed a commit that referenced this pull request Mar 15, 2020
bcoe@codebytere
fs: set path when mkdir recursive called on file
7a88f7e
PR-URL: #31607Fixes: #28015
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
codebytere pushed a commit that referenced this pull request Mar 17, 2020
bcoe@codebytere
fs: set path when mkdir recursive called on file
8622ee0
PR-URL: #31607Fixes: #28015
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@codebyterecodebytere mentioned this pull request Mar 17, 2020
Merged
codebytere pushed a commit that referenced this pull request Mar 30, 2020
bcoe@codebytere
fs: set path when mkdir recursive called on file
d366996
PR-URL: #31607Fixes: #28015
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TrottTrottTrott approved these changes
@addaleaxaddaleaxaddaleax approved these changes
@lpincalpincalpinca approved these changes
@richardlaurichardlaurichardlau approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.fast-trackPRs that do not need to wait for 48 hours to land.fsIssues and PRs related to the fs subsystem / file system.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

missing error.path at fs.mkdir callback on last call

6 participants

@bcoe@nodejs-github-bot@richardlau@Trott@addaleax@lpinca