Skip to content

Commit 4828fb3

Browse files
youknowoneyouknowone
authored
test_os, test_io on windows (#6379)
* Allow hidden env vars on nt * Enable test_os on windows * enable test_io on windows
1 parent 20d9099 commit 4828fb3

File tree

5 files changed

+94
-51
lines changed

5 files changed

+94
-51
lines changed

‎.github/workflows/ci.yaml‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,11 @@ env:
2020
CARGO_ARGS_NO_SSL: --no-default-features --features stdlib,importlib,stdio,encodings,sqlite
2121
# Skip additional tests on Windows. They are checked on Linux and MacOS.
2222
# test_glob: many failing tests
23-
# test_io: many failing tests
24-
# test_os: many failing tests
2523
# test_pathlib: panic by surrogate chars
2624
# test_posixpath: OSError: (22, 'The filename, directory name, or volume label syntax is incorrect. (os error 123)')
2725
# test_venv: couple of failing tests
2826
WINDOWS_SKIPS: >-
2927
test_glob
30-
test_io
31-
test_os
3228
test_rlcompleter
3329
test_pathlib
3430
test_posixpath

‎Lib/test/test_ntpath.py‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -990,8 +990,6 @@ def test_realpath_permission(self):
990990

991991
self.assertPathEqual(test_file, ntpath.realpath(test_file_short))
992992

993-
# TODO: RUSTPYTHON
994-
@unittest.expectedFailureIfWindows("TODO: RUSTPYTHON; ValueError: illegal environment variable name")
995993
def test_expandvars(self):
996994
with os_helper.EnvironmentVarGuard() as env:
997995
env.clear()
@@ -1018,8 +1016,6 @@ def test_expandvars(self):
10181016
tester('ntpath.expandvars("\'%foo%\'%bar")', "\'%foo%\'%bar")
10191017
tester('ntpath.expandvars("bar\'%foo%")', "bar\'%foo%")
10201018

1021-
# TODO: RUSTPYTHON
1022-
@unittest.expectedFailureIfWindows("TODO: RUSTPYTHON; ValueError: illegal environment variable name")
10231019
@unittest.skipUnless(os_helper.FS_NONASCII, 'need os_helper.FS_NONASCII')
10241020
def test_expandvars_nonascii(self):
10251021
def check(value, expected):
@@ -1040,8 +1036,6 @@ def check(value, expected):
10401036
check('%spam%bar', '%sbar' % nonascii)
10411037
check('%{}%bar'.format(nonascii), 'ham%sbar' % nonascii)
10421038

1043-
# TODO: RUSTPYTHON
1044-
@unittest.expectedFailureIfWindows("TODO: RUSTPYTHON")
10451039
def test_expanduser(self):
10461040
tester('ntpath.expanduser("test")', 'test')
10471041

@@ -1515,16 +1509,6 @@ class NtCommonTest(test_genericpath.CommonTest, unittest.TestCase):
15151509
pathmodule = ntpath
15161510
attributes = ['relpath']
15171511

1518-
# TODO: RUSTPYTHON
1519-
@unittest.expectedFailureIfWindows("TODO: RUSTPYTHON; ValueError: illegal environment variable name")
1520-
def test_expandvars(self):
1521-
return super().test_expandvars()
1522-
1523-
# TODO: RUSTPYTHON
1524-
@unittest.expectedFailureIfWindows("TODO: RUSTPYTHON; ValueError: illegal environment variable name")
1525-
def test_expandvars_nonascii(self):
1526-
return super().test_expandvars_nonascii()
1527-
15281512

15291513
class PathLikeTests(NtpathTestCase):
15301514

‎Lib/test/test_unittest/testmock/testpatch.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -658,8 +658,6 @@ def test():
658658
self.assertEqual(foo, {})
659659

660660

661-
# TODO: RUSTPYTHON
662-
@unittest.expectedFailureIfWindows("TODO: RUSTPYTHON")
663661
def test_patch_dict_with_string(self):
664662
@patch.dict('os.environ', {'konrad_delong': 'some value'})
665663
def test():

‎crates/vm/src/stdlib/nt.rs‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,13 @@ pub(crate) mod module {
125125
let environ = vm.ctx.new_dict();
126126

127127
for (key, value) in env::vars() {
128+
// Skip hidden Windows environment variables (e.g., =C:, =D:, =ExitCode)
129+
// These are internal cmd.exe bookkeeping variables that store per-drive
130+
// current directories. They cannot be modified via _wputenv() and should
131+
// not be exposed to Python code.
132+
if key.starts_with('=') {
133+
continue;
134+
}
128135
environ.set_item(&key, vm.new_pyobj(value), vm).unwrap();
129136
}
130137
environ
@@ -364,8 +371,9 @@ pub(crate) mod module {
364371
if key_str.contains('\0') || value_str.contains('\0') {
365372
return Err(vm.new_value_error("embedded null character"));
366373
}
367-
// Validate: no '=' in key
368-
if key_str.contains('=') {
374+
// Validate: no '=' in key (search from index 1 because on Windows
375+
// starting '=' is allowed for defining hidden environment variables)
376+
if key_str.get(1..).is_some_and(|s| s.contains('=')) {
369377
return Err(vm.new_value_error("illegal environment variable name"));
370378
}
371379

@@ -480,8 +488,9 @@ pub(crate) mod module {
480488
if key_str.contains('\0') || value_str.contains('\0') {
481489
return Err(vm.new_value_error("embedded null character"));
482490
}
483-
// Validate: no '=' in key
484-
if key_str.contains('=') {
491+
// Validate: no '=' in key (search from index 1 because on Windows
492+
// starting '=' is allowed for defining hidden environment variables)
493+
if key_str.get(1..).is_some_and(|s| s.contains('=')) {
485494
return Err(vm.new_value_error("illegal environment variable name"));
486495
}
487496

‎crates/vm/src/stdlib/os.rs‎

Lines changed: 81 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{
77
convert::{IntoPyException, ToPyException, ToPyObject},
88
function::{ArgumentError, FromArgs, FuncArgs},
99
};
10-
use std::{ffi, fs, io, path::Path};
10+
use std::{fs, io, path::Path};
1111

1212
pub(crate) fn fs_metadata<P: AsRef<Path>>(
1313
path: P,
@@ -112,7 +112,8 @@ pub(super) struct FollowSymlinks(
112112
#[pyarg(named, name = "follow_symlinks", default = true)] pub bool,
113113
);
114114

115-
fn bytes_as_os_str<'a>(b: &'a [u8], vm: &VirtualMachine) -> PyResult<&'a ffi::OsStr> {
115+
#[cfg(not(windows))]
116+
fn bytes_as_os_str<'a>(b: &'a [u8], vm: &VirtualMachine) -> PyResult<&'a std::ffi::OsStr> {
116117
rustpython_common::os::bytes_as_os_str(b)
117118
.map_err(|_| vm.new_unicode_decode_error("can't decode path for utf-8"))
118119
}
@@ -160,7 +161,7 @@ pub(super) mod _os {
160161
suppress_iph,
161162
},
162163
convert::{IntoPyException, ToPyObject},
163-
function::{ArgBytesLike, Either, FsPath, FuncArgs, OptionalArg},
164+
function::{ArgBytesLike, FsPath, FuncArgs, OptionalArg},
164165
ospath::{IOErrorBuilder, OsPath, OsPathOrFd, OutputMode},
165166
protocol::PyIterReturn,
166167
recursion::ReprGuard,
@@ -171,7 +172,7 @@ pub(super) mod _os {
171172
use crossbeam_utils::atomic::AtomicCell;
172173
use itertools::Itertools;
173174
use std::{
174-
env, ffi, fs,
175+
env, fs,
175176
fs::OpenOptions,
176177
io,
177178
path::PathBuf,
@@ -403,7 +404,7 @@ pub(super) mod _os {
403404
b"." | b".." => None,
404405
_ => Some(
405406
OutputMode::String
406-
.process_path(ffi::OsStr::from_bytes(fname), vm),
407+
.process_path(std::ffi::OsStr::from_bytes(fname), vm),
407408
),
408409
}
409410
})
@@ -415,31 +416,62 @@ pub(super) mod _os {
415416
Ok(list)
416417
}
417418

418-
fn env_bytes_as_bytes(obj: &Either<PyStrRef, PyBytesRef>) -> &[u8] {
419+
#[cfg(not(windows))]
420+
fn env_bytes_as_bytes(obj: &crate::function::Either<PyStrRef, PyBytesRef>) -> &[u8] {
419421
match obj {
420-
Either::A(s) => s.as_bytes(),
421-
Either::B(b) => b.as_bytes(),
422+
crate::function::Either::A(s) => s.as_bytes(),
423+
crate::function::Either::B(b) => b.as_bytes(),
422424
}
423425
}
424426

425-
/// Check if environment variable length exceeds Windows limit.
426-
/// size should be key.len() + value.len() + 2 (for '=' and null terminator)
427427
#[cfg(windows)]
428-
fn check_env_var_len(size: usize, vm: &VirtualMachine) -> PyResult<()> {
428+
unsafe extern "C" {
429+
fn _wputenv(envstring: *const u16) -> libc::c_int;
430+
}
431+
432+
/// Check if wide string length exceeds Windows environment variable limit.
433+
#[cfg(windows)]
434+
fn check_env_var_len(wide_len: usize, vm: &VirtualMachine) -> PyResult<()> {
429435
use crate::common::windows::_MAX_ENV;
430-
if size > _MAX_ENV {
436+
if wide_len > _MAX_ENV + 1 {
431437
return Err(vm.new_value_error(format!(
432-
"the environment variable is longer than {} characters",
433-
_MAX_ENV
438+
"the environment variable is longer than {_MAX_ENV} characters",
434439
)));
435440
}
436441
Ok(())
437442
}
438443

444+
#[cfg(windows)]
445+
#[pyfunction]
446+
fn putenv(key: PyStrRef, value: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
447+
let key_str = key.as_str();
448+
let value_str = value.as_str();
449+
// Search from index 1 because on Windows starting '=' is allowed for
450+
// defining hidden environment variables.
451+
if key_str.is_empty()
452+
|| key_str.get(1..).is_some_and(|s| s.contains('='))
453+
|| key_str.contains('\0')
454+
|| value_str.contains('\0')
455+
{
456+
return Err(vm.new_value_error("illegal environment variable name"));
457+
}
458+
let env_str = format!("{}={}", key_str, value_str);
459+
let wide = env_str.to_wide_with_nul();
460+
check_env_var_len(wide.len(), vm)?;
461+
462+
// Use _wputenv like CPython (not SetEnvironmentVariableW) to update CRT environ
463+
let result = unsafe { suppress_iph!(_wputenv(wide.as_ptr())) };
464+
if result != 0 {
465+
return Err(vm.new_last_errno_error());
466+
}
467+
Ok(())
468+
}
469+
470+
#[cfg(not(windows))]
439471
#[pyfunction]
440472
fn putenv(
441-
key: Either<PyStrRef, PyBytesRef>,
442-
value: Either<PyStrRef, PyBytesRef>,
473+
key: crate::function::Either<PyStrRef, PyBytesRef>,
474+
value: crate::function::Either<PyStrRef, PyBytesRef>,
443475
vm: &VirtualMachine,
444476
) -> PyResult<()> {
445477
let key = env_bytes_as_bytes(&key);
@@ -450,17 +482,44 @@ pub(super) mod _os {
450482
if key.is_empty() || key.contains(&b'=') {
451483
return Err(vm.new_value_error("illegal environment variable name"));
452484
}
453-
#[cfg(windows)]
454-
check_env_var_len(key.len() + value.len() + 2, vm)?;
455485
let key = super::bytes_as_os_str(key, vm)?;
456486
let value = super::bytes_as_os_str(value, vm)?;
457487
// SAFETY: requirements forwarded from the caller
458488
unsafe { env::set_var(key, value) };
459489
Ok(())
460490
}
461491

492+
#[cfg(windows)]
493+
#[pyfunction]
494+
fn unsetenv(key: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
495+
let key_str = key.as_str();
496+
// Search from index 1 because on Windows starting '=' is allowed for
497+
// defining hidden environment variables.
498+
if key_str.is_empty()
499+
|| key_str.get(1..).is_some_and(|s| s.contains('='))
500+
|| key_str.contains('\0')
501+
{
502+
return Err(vm.new_value_error("illegal environment variable name"));
503+
}
504+
// "key=" to unset (empty value removes the variable)
505+
let env_str = format!("{}=", key_str);
506+
let wide = env_str.to_wide_with_nul();
507+
check_env_var_len(wide.len(), vm)?;
508+
509+
// Use _wputenv like CPython (not SetEnvironmentVariableW) to update CRT environ
510+
let result = unsafe { suppress_iph!(_wputenv(wide.as_ptr())) };
511+
if result != 0 {
512+
return Err(vm.new_last_errno_error());
513+
}
514+
Ok(())
515+
}
516+
517+
#[cfg(not(windows))]
462518
#[pyfunction]
463-
fn unsetenv(key: Either<PyStrRef, PyBytesRef>, vm: &VirtualMachine) -> PyResult<()> {
519+
fn unsetenv(
520+
key: crate::function::Either<PyStrRef, PyBytesRef>,
521+
vm: &VirtualMachine,
522+
) -> PyResult<()> {
464523
let key = env_bytes_as_bytes(&key);
465524
if key.contains(&b'\0') {
466525
return Err(vm.new_value_error("embedded null byte"));
@@ -474,9 +533,6 @@ pub(super) mod _os {
474533
),
475534
));
476535
}
477-
// For unsetenv, size is key + '=' (no value, just clearing)
478-
#[cfg(windows)]
479-
check_env_var_len(key.len() + 1, vm)?;
480536
let key = super::bytes_as_os_str(key, vm)?;
481537
// SAFETY: requirements forwarded from the caller
482538
unsafe { env::remove_var(key) };
@@ -984,7 +1040,7 @@ pub(super) mod _os {
9841040
OsPathOrFd::Path(path) => {
9851041
use rustpython_common::os::ffi::OsStrExt;
9861042
let path = path.as_ref().as_os_str().as_bytes();
987-
let path = match ffi::CString::new(path) {
1043+
let path = match std::ffi::CString::new(path) {
9881044
Ok(x) => x,
9891045
Err(_) => return Ok(None),
9901046
};
@@ -1483,7 +1539,7 @@ pub(super) mod _os {
14831539

14841540
#[pyfunction]
14851541
fn strerror(e: i32) -> String {
1486-
unsafe { ffi::CStr::from_ptr(libc::strerror(e)) }
1542+
unsafe { std::ffi::CStr::from_ptr(libc::strerror(e)) }
14871543
.to_string_lossy()
14881544
.into_owned()
14891545
}
@@ -1587,7 +1643,7 @@ pub(super) mod _os {
15871643
if encoding.is_null() || encoding.read() == '\0' as libc::c_char {
15881644
"UTF-8".to_owned()
15891645
} else {
1590-
ffi::CStr::from_ptr(encoding).to_string_lossy().into_owned()
1646+
std::ffi::CStr::from_ptr(encoding).to_string_lossy().into_owned()
15911647
}
15921648
};
15931649

0 commit comments

Comments
 (0)