{"action":"created","issue":{"url":"https://api.github.com/repos/PowerShell/PowerShell/issues/6743","repository_url":"https://api.github.com/repos/PowerShell/PowerShell","labels_url":"https://api.github.com/repos/PowerShell/PowerShell/issues/6743/labels{/name}","comments_url":"https://api.github.com/repos/PowerShell/PowerShell/issues/6743/comments","events_url":"https://api.github.com/repos/PowerShell/PowerShell/issues/6743/events","html_url":"https://github.com/PowerShell/PowerShell/issues/6743","id":318005364,"number":6743,"title":"Misuse of `SHEmptyRecycleBin` in `Clear-RecycleBin` cmdlet","user":{"login":"GeeLaw","id":4462444,"avatar_url":"https://avatars1.githubusercontent.com/u/4462444?v=4","gravatar_id":"","url":"https://api.github.com/users/GeeLaw","html_url":"https://github.com/GeeLaw","followers_url":"https://api.github.com/users/GeeLaw/followers","following_url":"https://api.github.com/users/GeeLaw/following{/other_user}","gists_url":"https://api.github.com/users/GeeLaw/gists{/gist_id}","starred_url":"https://api.github.com/users/GeeLaw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/GeeLaw/subscriptions","organizations_url":"https://api.github.com/users/GeeLaw/orgs","repos_url":"https://api.github.com/users/GeeLaw/repos","events_url":"https://api.github.com/users/GeeLaw/events{/privacy}","received_events_url":"https://api.github.com/users/GeeLaw/received_events","type":"User","site_admin":false},"labels":[{"id":617835845,"url":"https://api.github.com/repos/PowerShell/PowerShell/labels/Issue-Code%20Cleanup","name":"Issue-Code Cleanup","color":"fc2929","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":10,"created_at":"2018-04-26T12:23:42Z","updated_at":"2018-04-28T20:02:42Z","closed_at":null,"author_association":"NONE","body":"## Overview\r\n\r\n`Clear-RecycleBin` produces spurious error records when used in Windows PowerShell. (Yes I know this repo is dedicated to PowerShell Core, but since it contains the code, and UserVoice isn't quite effective, I'll just put the analysis here.)\r\n\r\nIn a freshly launched PowerShell instance, running `Clear-RecycleBin -Force` will produce the following error (whether or not the recycle bin is empty does not matter at all):\r\n\r\n```text\r\nClear-RecycleBin : The system cannot find the path specified\r\nAt line:1 char:1\r\n+ Clear-RecycleBin -Force\r\n+ ~~~~~~~~~~~~~~~~~~~~~~~\r\n    + CategoryInfo          : InvalidOperation: (RecycleBin:String) [Clear-RecycleBin], Win32Exception\r\n    + FullyQualifiedErrorId : FailedToClearRecycleBin,Microsoft.PowerShell.Commands.ClearRecycleBinCommand\r\n```\r\n\r\nHowever, subsequent invocations to the same command (with `-Force`) will not produce such error. Moreover, in a newly launched PowerShell instance, running `Clear-RecycleBin` (without `-Force`) and manually confirming the operation) will not write error.\r\n\r\n## Root Cause\r\n\r\nUpon close investigation, the root cause starts on [line 226](https://github.com/PowerShell/PowerShell/blob/ffa7e4bac0b2546c5a61fad7cd87ae353e3264dc/src/Microsoft.PowerShell.Commands.Management/commands/management/ClearRecycleBinCommand.cs#L226) of `ClearRecycleBinCommand.cs`. Relevant excerpt:\r\n\r\n```csharp\r\nuint result = NativeMethod.SHEmptyRecycleBin(IntPtr.Zero, drivePath,\r\n    NativeMethod.RecycleFlags.SHERB_NOCONFIRMATION |\r\n    NativeMethod.RecycleFlags.SHERB_NOPROGRESSUI |\r\n    NativeMethod.RecycleFlags.SHERB_NOSOUND);\r\n\r\nint lastError = Marshal.GetLastWin32Error();\r\nprogress.PercentComplete = 100;\r\nprogress.RecordType = ProgressRecordType.Completed;\r\nWriteProgress(progress);\r\n// 0 is for a successful operation\r\n// 203 comes up when trying to empty an already emptied recyclebin\r\n// 18 comes up when there are no more files in the given recyclebin\r\nif (!(lastError == 0 || lastError == 203 || lastError == 18))\r\n{\r\n    Win32Exception exception = new Win32Exception(lastError);\r\n    WriteError(new ErrorRecord(exception, \"FailedToClearRecycleBin\", ErrorCategory.InvalidOperation, \"RecycleBin\"));\r\n}\r\n```\r\n\r\nThe mistake here is to use `GetLastWin32Error` -- according to [`SHEmptyRecycleBin` function (Windows)](https://msdn.microsoft.com/en-us/library/windows/desktop/bb762160.aspx) on MSDN, the function uses `HRESULT` to indicate error status. The documentation does not mention `GetLastError`, and it is true that the function does **not** call `SetLastError` on the caller thread before returning.\r\n\r\nThe fact that `GetLastError` returns 3 on the first invocation of `Clear-RecycleBin -Force` is purely accidental.\r\n\r\nPlaying around with `SHEmptyRecycleBin` indicates that it returns `S_OK` (`0`) for success (also documented), and `E_UNEXPECTED` (`0x8000FFFF`, catastrophic/unexpected failure) when the recycle bin is already empty. At this time, I would suggest remove error detection in the cmdlet, since `SHEmptyRecycleBin` doesn't know how to handle the case when the recycle bin is already empty at all -- we can't expect it to do any reasonable failure indication.\r\n\r\nThere could have been a time when `SHEmptyRecycleBin` uses `SetLastError` to indicate failure, and the code could have been written when the documentation wasn't updated.\r\n\r\n---\r\n\r\nI wrote a blog entry on this issue, and I found that the code is wrong in every way. If you're interested, read https://geelaw.blog/entries/clear-recyclebin-shemptyrecyclebin/\r\n\r\nMistakes made summarised into a list:\r\n\r\n- not declaring `[DllImport(SetLastError = true)]` and using `Marshal.GetLastWin32Error` to retrieve the possible error code set by `SHEmptyRecycleBin`, which won't work and will get the leftover of previous marshalled calls.\r\n- documentation for `SHEmptyRecycleBin` doesn't indicate that it will call `SetLastError`, and recommends that one use the return value to determine error status.\r\n- `SHEmptyRecycleBin` doesn't expect to deal with already emptied recycle bin.\r\n\r\nI would recommend anyone using this cmdlet use `Clear-RecycleBin -ErrorAction SilentlyContinue` to prevent irrelevant `ErrorRecord`.\r\n"},"comment":{"url":"https://api.github.com/repos/PowerShell/PowerShell/issues/comments/385202020","html_url":"https://github.com/PowerShell/PowerShell/issues/6743#issuecomment-385202020","issue_url":"https://api.github.com/repos/PowerShell/PowerShell/issues/6743","id":385202020,"user":{"login":"jeffbi","id":6708868,"avatar_url":"https://avatars3.githubusercontent.com/u/6708868?v=4","gravatar_id":"","url":"https://api.github.com/users/jeffbi","html_url":"https://github.com/jeffbi","followers_url":"https://api.github.com/users/jeffbi/followers","following_url":"https://api.github.com/users/jeffbi/following{/other_user}","gists_url":"https://api.github.com/users/jeffbi/gists{/gist_id}","starred_url":"https://api.github.com/users/jeffbi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jeffbi/subscriptions","organizations_url":"https://api.github.com/users/jeffbi/orgs","repos_url":"https://api.github.com/users/jeffbi/repos","events_url":"https://api.github.com/users/jeffbi/events{/privacy}","received_events_url":"https://api.github.com/users/jeffbi/received_events","type":"User","site_admin":false},"created_at":"2018-04-28T20:02:42Z","updated_at":"2018-04-28T20:02:42Z","author_association":"CONTRIBUTOR","body":"After poking around a little, it looks to me as if VSCode (via Electron) implements move-to-trash via one of several command-line programs. [https://github.com/electron/electron/pull/7178](https://github.com/electron/electron/pull/7178)"}}
{"actor":{"display_login":"jeffbi"}}