-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
bpo-30584: Fix test_os fails on non-English Windows #1980
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
Lib/test/test_os.py
Outdated
@@ -472,7 +472,7 @@ def test_access_denied(self): | |||
# force CreateFile to fail with ERROR_ACCESS_DENIED. | |||
DETACHED_PROCESS = 8 | |||
subprocess.check_call( | |||
['icacls.exe', fname, '/deny', 'Users:(S)'], | |||
['icacls.exe', fname, '/deny', '*S-1-5-32-545:(S)'], |
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.
Yes, using the numeric SID to avoid localized names is correct. Thanks for catching this.
Lib/test/test_os.py
Outdated
@@ -472,7 +472,7 @@ def test_access_denied(self): | |||
# force CreateFile to fail with ERROR_ACCESS_DENIED. | |||
DETACHED_PROCESS = 8 | |||
subprocess.check_call( | |||
['icacls.exe', fname, '/deny', 'Users:(S)'], | |||
['icacls.exe', fname, '/deny', '*S-1-5-32-545:(S)'], |
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.
Please add a comment mentionning the bpo and explaining what is this identifier, like:
# bpo-30584: Use security identifier *S-1-5-32-545 instead of localized "Users"
# to not depend on the locale
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.
Done.
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.
Oh, it's your first contribution. In that case, you should add yourself to Misc/ACKS please!
On Microsoft Support page Well-known security identifiers in Windows operating systems said:
So I suggest to use SID (*S-1-5-32-545) instead of name (Users).