gh-111841: Fix os.putenv() and os.unsetenv() with embedded NUL on Windows by serhiy-storchaka · Pull Request #111842 · python/cpython

serhiy-storchaka · 2023-11-08T09:53:14Z

Issue: os.putenv() on Windows truncates value on an embedded NUL #111841

…on Windows

serhiy-storchaka · 2023-11-08T13:41:31Z

It also makes test_posix running on Windows. Interesting that only two test_posix tests failed on Windows: the one for testing this bug (which was fixed on other platforms), and the other that tests Posix-specific constants.

vstinner

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.

vstinner · 2023-11-09T09:49:36Z

Lib/test/test_posix.py

+    import posix
+except ImportError:
+    try:
+        import nt as posix


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?

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.

Modules/posixmodule.c

vstinner · 2023-11-09T16:50:41Z

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.

I looked at the code and played with Python on Windows:

PyUnicode_AsWideCharString(str, NULL) rejects embedded null characters.
PyUnicode_AsWideCharString(str, &size) does not reject embedded null characters.
win32_putenv() doesn't use PyUnicode_AsWideCharString(str, NULL) but PyUnicode_AsWideCharString(str, &size).
test_os checks if os.putenv() raises ValueError or OSError when the env var name contains an embedded null character.

Oh ok, now I get it.

Well, currently, OSError is raised at least.

Lib/test/test_posix.py

vstinner · 2023-11-09T18:08:08Z

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.

serhiy-storchaka · 2023-11-09T19:15:03Z

If you want to run test_posix on Windows, can you please do it in a separated PR?

Done. See #111913.

vstinner

LGTM

miss-islington-app · 2023-11-10T16:42:18Z

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

bedevere-app · 2023-11-10T16:42:31Z

GH-111966 is a backport of this pull request to the 3.12 branch.

bedevere-app · 2023-11-10T16:42:35Z

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 <storchaka@gmail.com>

… on Windows (GH-111842) (GH-111966) (cherry picked from commit 0b06d24) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

bedevere-bot · 2023-11-11T08:16:05Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 RHEL8 LTO + PGO 3.12 has failed when building commit 3bd8b74.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/1168/builds/411) and take a look at the build logs.
Check if the failure is related to this commit (3bd8b74) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/1168/builds/411

Summary of the results of the build (if available):

Click to see traceback logs

remote: Enumerating objects: 13, done.        
remote: Counting objects:   8% (1/12)        
remote: Counting objects:  16% (2/12)        
remote: Counting objects:  25% (3/12)        
remote: Counting objects:  33% (4/12)        
remote: Counting objects:  41% (5/12)        
remote: Counting objects:  50% (6/12)        
remote: Counting objects:  58% (7/12)        
remote: Counting objects:  66% (8/12)        
remote: Counting objects:  75% (9/12)        
remote: Counting objects:  83% (10/12)        
remote: Counting objects:  91% (11/12)        
remote: Counting objects: 100% (12/12)        
remote: Counting objects: 100% (12/12), done.        
remote: Compressing objects:   8% (1/12)        
remote: Compressing objects:  16% (2/12)        
remote: Compressing objects:  25% (3/12)        
remote: Compressing objects:  33% (4/12)        
remote: Compressing objects:  41% (5/12)        
remote: Compressing objects:  50% (6/12)        
remote: Compressing objects:  58% (7/12)        
remote: Compressing objects:  66% (8/12)        
remote: Compressing objects:  75% (9/12)        
remote: Compressing objects:  83% (10/12)        
remote: Compressing objects:  91% (11/12)        
remote: Compressing objects: 100% (12/12)        
remote: Compressing objects: 100% (12/12), done.        
remote: Total 13 (delta 0), reused 7 (delta 0), pack-reused 1        
From https://github.com/python/cpython
 * branch                  3.12       -> FETCH_HEAD
Auto packing the repository in background for optimum performance.
See "git help gc" for manual housekeeping.
Note: switching to '3bd8b743305f0d788302df3ff08c2bb7eb0dd297'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 3bd8b74330 [3.12] gh-111841: Fix os.putenv() and os.unsetenv() with embedded NUL on Windows (GH-111842) (GH-111966)
Switched to and reset branch '3.12'

find: ‘./.git/objects/1c’: No such file or directory
find: ‘./.git/objects/20’: No such file or directory
find: ‘./.git/objects/21’: No such file or directory
find: ‘./.git/objects/22’: No such file or directory
find: ‘./.git/objects/24’: No such file or directory
find: ‘./.git/objects/27’: No such file or directory
find: ‘./.git/objects/23’: No such file or directory
find: ‘./.git/objects/1b’: No such file or directory
find: ‘./.git/objects/1a’: No such file or directory
find: ‘./.git/objects/1d’: No such file or directory
find: ‘./.git/objects/28’: No such file or directory
make[2]: [Makefile:2783: pycremoval] Error 1 (ignored)
find: ‘./.git/objects/47’: No such file or directory
find: ‘./.git/objects/46’: No such file or directory
find: ‘./.git/objects/49’: No such file or directory
find: ‘./.git/objects/44’: No such file or directory
find: ‘./.git/objects/48’: No such file or directory
find: ‘./.git/objects/45’: No such file or directory
make[2]: [Makefile:2784: pycremoval] Error 1 (ignored)
find: ‘./.git/objects/68’: No such file or directory
find: ‘./.git/objects/6a’: No such file or directory
find: ‘./.git/objects/6f’: No such file or directory
find: ‘./.git/objects/6c’: No such file or directory
find: ‘./.git/objects/6e’: No such file or directory
find: ‘./.git/objects/70’: No such file or directory
find: ‘./.git/objects/69’: No such file or directory
make[2]: *** [Makefile:2802: clean-retain-profile] Error 1
make[1]: *** [Makefile:794: profile-clean-stamp] Error 2
make: *** [Makefile:813: profile-run-stamp] Error 2

find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
make: [Makefile:2820: clean-retain-profile] Error 1 (ignored)

… on Windows (GH-111842) (GH-111967) (cherry picked from commit 0b06d24) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

…on Windows (pythonGH-111842)

pythongh-111841: Fix os.putenv() and os.unsetenv() with embedded NUL …

9491455

…on Windows

serhiy-storchaka added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Nov 8, 2023

bedevere-app bot added the awaiting core review label Nov 8, 2023

bedevere-app bot mentioned this pull request Nov 8, 2023

os.putenv() on Windows truncates value on an embedded NUL #111841

Closed

serhiy-storchaka added 2 commits November 8, 2023 12:14

Update tests.

4f3e0ad

Update tests.

9aacf52

serhiy-storchaka requested a review from vstinner November 8, 2023 17:48

vstinner reviewed Nov 9, 2023

View reviewed changes

Lib/test/test_posix.py Show resolved Hide resolved

Merge branch 'main' into win-os-putenv-embedded-nul

da69742

vstinner approved these changes Nov 10, 2023

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting core review labels Nov 10, 2023

serhiy-storchaka merged commit 0b06d24 into python:main Nov 10, 2023

bedevere-app bot removed the awaiting merge label Nov 10, 2023

serhiy-storchaka deleted the win-os-putenv-embedded-nul branch November 10, 2023 16:42

bedevere-app bot removed the needs backport to 3.12 only security fixes label Nov 10, 2023

bedevere-app bot removed the needs backport to 3.11 only security fixes label Nov 10, 2023

serhiy-storchaka added a commit that referenced this pull request Nov 11, 2023

[3.12] gh-111841: Fix os.putenv() and os.unsetenv() with embedded NUL…

3bd8b74

… on Windows (GH-111842) (GH-111966) (cherry picked from commit 0b06d24) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

serhiy-storchaka added a commit that referenced this pull request Nov 11, 2023

[3.11] gh-111841: Fix os.putenv() and os.unsetenv() with embedded NUL…

cd3e2d3

… on Windows (GH-111842) (GH-111967) (cherry picked from commit 0b06d24) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024

pythongh-111841: Fix os.putenv() and os.unsetenv() with embedded NUL …

bb03a8f

…on Windows (pythonGH-111842)

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024

pythongh-111841: Fix os.putenv() and os.unsetenv() with embedded NUL …

0224e57

…on Windows (pythonGH-111842)

Uh oh!

Conversation

serhiy-storchaka commented Nov 8, 2023 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka commented Nov 8, 2023

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vstinner commented Nov 9, 2023

Uh oh!

Uh oh!

vstinner commented Nov 9, 2023

Uh oh!

serhiy-storchaka commented Nov 9, 2023

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington-app bot commented Nov 10, 2023

Uh oh!

bedevere-app bot commented Nov 10, 2023

Uh oh!

bedevere-app bot commented Nov 10, 2023

Uh oh!

bedevere-bot commented Nov 11, 2023

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

serhiy-storchaka commented Nov 8, 2023 •

edited by bedevere-app bot

Loading