-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Fix unblock read only file #4395
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
Conversation
@iSazonov what is the error message the user sees now? |
MSDN says that Unblock-File : Access is denied
At line:1 char:1
+ Unblock-File -Path $Testpath
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : NotSpecified: (:) [Unblock-File], Win32Exception
+ FullyQualifiedErrorId : System.ComponentModel.Win32Exception,Microsoft.PowerShell.Commands.UnblockFileCommand |
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
if (!NativeMethods.DeleteFile(resultPath)) | ||
{ | ||
int error = Marshal.GetLastWin32Error(); | ||
throw new Win32Exception(error); |
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.
Shouldn't this be a non-terminating error? The following should behave similarly:
dir | Unblock-File
dir | Remove-Item
In the Remove-Item
case, a read only file reports an error, but all other files are deleted.
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.
Good catch!
AlternateDataStreamUtilities
uses the pattern. So maybe add try-catch
in Unblock-File cmdlet file?
@lzybkr @SteveL-MSFT Please continue the code review. |
A commit has been added to the review
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.
Make sure the final commit message points out this is a new non-terminating error.
Adds a new non-terminating error
Fix #4390.
Problem
If a file is read only it is skipping silently.
Fix
Analize GetLastError() and throw if a file stream hasn't been removed.
First commit only refactor tests to use
ShouldBeErrorId
.