-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Recognize CONOUT$ and CONIN$ as reserved device names #13508
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
Recognize CONOUT$ and CONIN$ as reserved device names #13508
Conversation
Max size set to 7 in order to also detect the reserved name "CONOUT$"
|
@DHowett Have you any thoughts about blocking CONIN$ and CONOUT$ in PowerShell file operations? |
|
Yeah, we're already blocking the majority of them, so probably it would make sense to block all of them. Maybe a subset of them could be useful for specific provider cmdlets, but I don't think any of them would be universally useful for all the provider cmdlets to support. |
|
So, here's the thing that worries me about the windows reserved names. I don't know how we handle them in a world where pwsh also runs on Linux (and can access Do we have a holistic story that covers how powershell handles devicelike file handles in general? If not, is this just putting a band-aid over a problem? |
|
But, here: We should block these names. There are better ways to write output to the console or read input from the console. It is generally a bad thing that we would allow PowerShell users -- users who have opted into a higher-level abstraction -- direct unmanaged access to the raw I/O handles. CMD should be able to do it. Perhaps PowerShell should bend users towards using the object model instead. |
|
/cc @SteveL-MSFT not sure who best to answer @DHowett's question regarding similarly reserved names on MacOS/Linux, but maybe worth opening an issue for? 🤔 Otherwise... yep, agreed. 🙂 |
We follow the naming convention https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions but CONIN$/CONOUT$ is not documented there.
In Unix-s they are files. Users could do
|
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
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.
Perhaps IsReservedDeviceName() is on hot path and we could measure and optimize its performance in follow PR.
| if (((compareName.Length < 3) || (compareName.Length > 6)) && | ||
| ((noExtensionCompareName.Length < 3) || (noExtensionCompareName.Length > 6))) | ||
| if (((compareName.Length < 3) || (compareName.Length > 7)) && | ||
| ((noExtensionCompareName.Length < 3) || (noExtensionCompareName.Length > 7))) |
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.
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.
I open new tracking issue #13618.
|
@anmenaga The PR is ready to merge. |
|
@davidreis97 Thanks for your contribution! |
PR Summary
Fixes #13046.
Adds
CONOUT$andCONIN$to the list of reserved device names. The existing range of filename lengths to be checked for reserved names excludedCONOUT$since it only checked names of up to 6 characters. To includeCONOUT$, this range has been bumped to 7 characters.With these changes, attempting to execute filesystem commands with these names as targets will now raise an exception, as defined in the expected behaviour section of the linked issue.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.