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

Windows: fix JNI's DeletePath function #7176

Closed
wants to merge 1 commit into from
Closed

Windows: fix JNI's DeletePath function #7176

wants to merge 1 commit into from

Conversation

laszlocsomor
Copy link
Contributor

DeletePath now verifies that the input path is
Windows-style (i.e. uses backslashes) and
absolute, and adds the \\?\ prefix to the path.

Doing so ensures it can delete any file (including
degenerately-named ones like c:\foo\nul).

Fixes #7173

@laszlocsomor
Windows: fix JNI's DeletePath function
e41deab 
DeletePath now verifies that the input path is
Windows-style (i.e. uses backslashes) and
absolute, and adds the `\\?\` prefix to the path.

Doing so ensures it can delete any file (including
degenerately-named ones like `c:\foo\nul`).

Fixes bazelbuild#7173
@laszlocsomor laszlocsomor added the area-Windows Windows-specific issues and feature requests label Jan 18, 2019
@meteorcloudy
Copy link
Member

Did you consider making DeletePath compatible with mixed style Windows path? (eg. c:/foo/bar)? Maybe using the AsAbsoluteWindowsPath API?

@laszlocsomor
Copy link
Contributor Author

Yes! But doing that requires depending on //src/main/cpp/util:filesystem -- but that already depends on the JNI library, so that'd create a cycle. Instead I would have to move the AsWindowsPath logic to the JNI library, which is probably a good idea anyway but it'd make this PR much bigger and more complex.

@meteorcloudy
Copy link
Member

Ah, I see!

laszlocsomor added a commit to laszlocsomor/bazel that referenced this pull request Jan 18, 2019
@laszlocsomor
Windows: all JNI methods verify their path arg.
c5697e9 
Also remove the GetAttributesOfMaybeMissingFile
method because it's based on a false belief,
namely that FindFirstFile returns up-to-date
information. According to this OldNewThing post
that belief was wrong: https://blogs.msdn.microsoft.com/oldnewthing/20111226-00/?p=8813

This PR is a follow-up to bazelbuild#7176
laszlocsomor added a commit to laszlocsomor/bazel that referenced this pull request Jan 18, 2019
@laszlocsomor
Windows: all JNI methods verify their path arg.
69670c9 
Also remove the GetAttributesOfMaybeMissingFile
method because it's based on a false belief,
namely that FindFirstFile returns up-to-date
information. According to this OldNewThing post
that belief was wrong: https://blogs.msdn.microsoft.com/oldnewthing/20111226-00/?p=8813

This PR is a follow-up to bazelbuild#7176
@laszlocsomor laszlocsomor mentioned this pull request Jan 18, 2019
Closed
laszlocsomor added a commit to laszlocsomor/bazel that referenced this pull request Jan 18, 2019
@laszlocsomor
Windows: all JNI methods verify their path arg.
ffb1d8c 
Also remove the GetAttributesOfMaybeMissingFile
method because it's based on a false belief,
namely that FindFirstFile returns up-to-date
information. According to this OldNewThing post
that belief was wrong: https://blogs.msdn.microsoft.com/oldnewthing/20111226-00/?p=8813

This PR is a follow-up to bazelbuild#7176
@bazel-io bazel-io closed this in f3c5570 Jan 18, 2019
@laszlocsomor laszlocsomor mentioned this pull request Jan 18, 2019
Closed
@laszlocsomor laszlocsomor deleted the fix-7173 branch January 18, 2019 14:00
laszlocsomor added a commit to laszlocsomor/bazel that referenced this pull request Jan 21, 2019
@laszlocsomor
Windows: all JNI methods verify their path arg.
910d871 
Also remove the GetAttributesOfMaybeMissingFile
method because it's based on a false belief,
namely that FindFirstFile returns up-to-date
information. According to this OldNewThing post
that belief was wrong: https://blogs.msdn.microsoft.com/oldnewthing/20111226-00/?p=8813

This PR is a follow-up to bazelbuild#7176
laszlocsomor added a commit to laszlocsomor/bazel that referenced this pull request Jan 24, 2019
@laszlocsomor
Windows: all JNI methods verify their path arg.
cbe39cc 
Also remove the GetAttributesOfMaybeMissingFile
method because it's based on a false belief,
namely that FindFirstFile returns up-to-date
information. According to this OldNewThing post
that belief was wrong: https://blogs.msdn.microsoft.com/oldnewthing/20111226-00/?p=8813

This PR is a follow-up to bazelbuild#7176
bazel-io pushed a commit that referenced this pull request Jan 24, 2019
@laszlocsomor
Windows: all JNI methods verify their path arg.
84aaca1 
Also remove the GetAttributesOfMaybeMissingFile
method because it's based on a false belief,
namely that FindFirstFile returns up-to-date
information. According to this OldNewThing post
that belief was wrong: https://blogs.msdn.microsoft.com/oldnewthing/20111226-00/?p=8813

This PR is a follow-up to #7176

Closes #7178.

PiperOrigin-RevId: 230705273
weixiao-huang pushed a commit to weixiao-huang/bazel that referenced this pull request Jan 31, 2019
@laszlocsomor @weixiao-huang
Windows: fix JNI's DeletePath function
84d9e5f 
DeletePath now verifies that the input path is
Windows-style (i.e. uses backslashes) and
absolute, and adds the `\\?\` prefix to the path.

Doing so ensures it can delete any file (including
degenerately-named ones like `c:\foo\nul`).

Fixes bazelbuild#7173

Closes bazelbuild#7176.

PiperOrigin-RevId: 229913623
weixiao-huang pushed a commit to weixiao-huang/bazel that referenced this pull request Jan 31, 2019
@laszlocsomor @weixiao-huang
Windows: all JNI methods verify their path arg.
9a5bbdc 
Also remove the GetAttributesOfMaybeMissingFile
method because it's based on a false belief,
namely that FindFirstFile returns up-to-date
information. According to this OldNewThing post
that belief was wrong: https://blogs.msdn.microsoft.com/oldnewthing/20111226-00/?p=8813

This PR is a follow-up to bazelbuild#7176

Closes bazelbuild#7178.

PiperOrigin-RevId: 230705273
@philwo philwo added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meteorcloudy meteorcloudy meteorcloudy approved these changes

Assignees
No one assigned
Labels
area-Windows Windows-specific issues and feature requests cla: yes team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Windows: cannot delete file from test's temp directory
4 participants
@laszlocsomor @meteorcloudy @philwo @googlebot