-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
gh-111841: Fix os.putenv() and os.unsetenv() with embedded NUL on Windows #111842
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
gh-111841: Fix os.putenv() and os.unsetenv() with embedded NUL on Windows #111842
Conversation
It also makes |
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 don't understand this change. PyUnicode_AsWideCharString() should already implements the check and test_os has tests on embedded null characters in the env var name and in the env var value.
Lib/test/test_posix.py
Outdated
import posix | ||
except ImportError: | ||
try: | ||
import nt as posix |
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.
That's strange. For me, "test_posix" are tests for the POSIX platforms: all platforms but Windows.
If you want to write tests on alls platform including Windows, add tests to test_os, no?
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.
They are already written, and it turned out that they are compatible with Windows.
In the past there were several platform specific implementations: posix
, nt
, dos
, os2
, mac
, ce
. But posix
and nt
became so close that they are now build from the same source, and other implementations are gone.
I looked at the code and played with Python on Windows:
Oh ok, now I get it. Well, currently, OSError is raised at least. |
If you want to run test_posix on Windows, can you please do it in a separated PR? This PR is about embedded null characters and putenv(). Running test_posix on Windows is a significant change. |
Done. See #111913. |
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
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12. |
GH-111966 is a backport of this pull request to the 3.12 branch. |
GH-111967 is a backport of this pull request to the 3.11 branch. |
…on Windows (pythonGH-111842) (cherry picked from commit 0b06d24) Co-authored-by: Serhiy Storchaka <[email protected]>
…on Windows (pythonGH-111842) (cherry picked from commit 0b06d24) Co-authored-by: Serhiy Storchaka <[email protected]>
… on Windows (GH-111842) (GH-111966) (cherry picked from commit 0b06d24) Co-authored-by: Serhiy Storchaka <[email protected]>
|
… on Windows (GH-111842) (GH-111967) (cherry picked from commit 0b06d24) Co-authored-by: Serhiy Storchaka <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.