Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions Lib/test/test_os.py
Original file line number Diff line number Diff line change
Expand Up @@ -2204,7 +2204,6 @@ def test_execv_with_bad_arglist(self):
self.assertRaises(ValueError, os.execv, 'notepad', ('',))
self.assertRaises(ValueError, os.execv, 'notepad', [''])

@unittest.expectedFailureIfWindows('TODO: RUSTPYTHON; os.execve not implemented yet for all platforms')
def test_execvpe_with_bad_arglist(self):
self.assertRaises(ValueError, os.execvpe, 'notepad', [], None)
self.assertRaises(ValueError, os.execvpe, 'notepad', [], {})
Expand Down Expand Up @@ -2264,7 +2263,6 @@ def test_internal_execvpe_str(self):
if os.name != "nt":
self._test_internal_execvpe(bytes)

@unittest.expectedFailureIfWindows('TODO: RUSTPYTHON; os.execve not implemented yet for all platforms')
def test_execve_invalid_env(self):
args = [sys.executable, '-c', 'pass']

Expand All @@ -2286,7 +2284,6 @@ def test_execve_invalid_env(self):
with self.assertRaises(ValueError):
os.execve(args[0], args, newenv)

@unittest.expectedFailureIfWindows('TODO: RUSTPYTHON; os.execve not implemented yet for all platforms')
@unittest.skipUnless(sys.platform == "win32", "Win32-specific test")
def test_execve_with_empty_path(self):
# bpo-32890: Check GetLastError() misuse
Expand Down
1 change: 1 addition & 0 deletions crates/vm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ features = [
"Win32_Globalization",
"Win32_Networking_WinSock",
"Win32_Security",
"Win32_Security_Authorization",
"Win32_Storage_FileSystem",
"Win32_System_Console",
"Win32_System_Diagnostics_Debug",
Expand Down
305 changes: 287 additions & 18 deletions crates/vm/src/stdlib/nt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,31 @@ pub(crate) mod module {
};

#[pyattr]
use libc::{O_BINARY, O_TEMPORARY};
use libc::{O_BINARY, O_NOINHERIT, O_RANDOM, O_SEQUENTIAL, O_TEMPORARY, O_TEXT};

// Windows spawn mode constants
#[pyattr]
const P_WAIT: i32 = 0;
#[pyattr]
const P_NOWAIT: i32 = 1;
#[pyattr]
const P_OVERLAY: i32 = 2;
#[pyattr]
const P_NOWAITO: i32 = 3;
#[pyattr]
const P_DETACH: i32 = 4;

// _O_SHORT_LIVED is not in libc, define manually
#[pyattr]
const O_SHORT_LIVED: i32 = 0x1000;

// Exit code constant
#[pyattr]
const EX_OK: i32 = 0;

// Maximum number of temporary files
#[pyattr]
const TMP_MAX: i32 = i32::MAX;

#[pyattr]
use windows_sys::Win32::System::LibraryLoader::{
Expand Down Expand Up @@ -138,19 +162,22 @@ pub(crate) mod module {

#[cfg(target_env = "msvc")]
#[pyfunction]
fn waitpid(pid: intptr_t, opt: i32, vm: &VirtualMachine) -> PyResult<(intptr_t, i32)> {
let mut status = 0;
fn waitpid(pid: intptr_t, opt: i32, vm: &VirtualMachine) -> PyResult<(intptr_t, u64)> {
let mut status: i32 = 0;
let pid = unsafe { suppress_iph!(_cwait(&mut status, pid, opt)) };
if pid == -1 {
Err(errno_err(vm))
} else {
Ok((pid, status << 8))
// Cast to unsigned to handle large exit codes (like 0xC000013A)
// then shift left by 8 to match POSIX waitpid format
let ustatus = (status as u32) as u64;
Ok((pid, ustatus << 8))
}
}

#[cfg(target_env = "msvc")]
#[pyfunction]
fn wait(vm: &VirtualMachine) -> PyResult<(intptr_t, i32)> {
fn wait(vm: &VirtualMachine) -> PyResult<(intptr_t, u64)> {
waitpid(-1, 0, vm)
}

Expand Down Expand Up @@ -212,12 +239,143 @@ pub(crate) mod module {
#[cfg(target_env = "msvc")]
unsafe extern "C" {
fn _wexecv(cmdname: *const u16, argv: *const *const u16) -> intptr_t;
fn _wexecve(
cmdname: *const u16,
argv: *const *const u16,
envp: *const *const u16,
) -> intptr_t;
fn _wspawnv(mode: i32, cmdname: *const u16, argv: *const *const u16) -> intptr_t;
fn _wspawnve(
mode: i32,
cmdname: *const u16,
argv: *const *const u16,
envp: *const *const u16,
) -> intptr_t;
}

#[cfg(target_env = "msvc")]
#[pyfunction]
fn spawnv(
mode: i32,
path: OsPath,
argv: Either<PyListRef, PyTupleRef>,
vm: &VirtualMachine,
) -> PyResult<intptr_t> {
use std::iter::once;

let make_widestring =
|s: &str| widestring::WideCString::from_os_str(s).map_err(|err| err.to_pyexception(vm));

let path = path.to_wide_cstring(vm)?;

let argv = vm.extract_elements_with(argv.as_ref(), |obj| {
let arg = PyStrRef::try_from_object(vm, obj)?;
make_widestring(arg.as_str())
})?;

let first = argv
.first()
.ok_or_else(|| vm.new_value_error("spawnv() arg 3 must not be empty"))?;

if first.is_empty() {
return Err(vm.new_value_error("spawnv() arg 3 first element cannot be empty"));
}

let argv_spawn: Vec<*const u16> = argv
.iter()
.map(|v| v.as_ptr())
.chain(once(std::ptr::null()))
.collect();

let result = unsafe { suppress_iph!(_wspawnv(mode, path.as_ptr(), argv_spawn.as_ptr())) };
if result == -1 {
Err(errno_err(vm))
} else {
Ok(result)
}
}

#[cfg(target_env = "msvc")]
#[pyfunction]
fn spawnve(
mode: i32,
path: OsPath,
argv: Either<PyListRef, PyTupleRef>,
env: PyDictRef,
vm: &VirtualMachine,
) -> PyResult<intptr_t> {
use std::iter::once;

let make_widestring =
|s: &str| widestring::WideCString::from_os_str(s).map_err(|err| err.to_pyexception(vm));

let path = path.to_wide_cstring(vm)?;

let argv = vm.extract_elements_with(argv.as_ref(), |obj| {
let arg = PyStrRef::try_from_object(vm, obj)?;
make_widestring(arg.as_str())
})?;

let first = argv
.first()
.ok_or_else(|| vm.new_value_error("spawnve() arg 2 cannot be empty"))?;

if first.is_empty() {
return Err(vm.new_value_error("spawnve() arg 2 first element cannot be empty"));
}
Comment on lines +319 to +325
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Incorrect argument index in error messages.

For spawnve(mode, path, argv, env), the argv parameter is argument 3, not argument 2. This is inconsistent with spawnv which correctly uses "arg 3".

         let first = argv
             .first()
-            .ok_or_else(|| vm.new_value_error("spawnve() arg 2 cannot be empty"))?;
+            .ok_or_else(|| vm.new_value_error("spawnve() arg 3 cannot be empty"))?;
 
         if first.is_empty() {
-            return Err(vm.new_value_error("spawnve() arg 2 first element cannot be empty"));
+            return Err(vm.new_value_error("spawnve() arg 3 first element cannot be empty"));
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let first = argv
.first()
.ok_or_else(|| vm.new_value_error("spawnve() arg 2 cannot be empty"))?;
if first.is_empty() {
return Err(vm.new_value_error("spawnve() arg 2 first element cannot be empty"));
}
let first = argv
.first()
.ok_or_else(|| vm.new_value_error("spawnve() arg 3 cannot be empty"))?;
if first.is_empty() {
return Err(vm.new_value_error("spawnve() arg 3 first element cannot be empty"));
}
🤖 Prompt for AI Agents
In crates/vm/src/stdlib/nt.rs around lines 319-325, the two error messages
mistakenly refer to `argv` as "arg 2" for spawnve(); update both messages to
reference "arg 3" instead so they match the spawnv wording and correctly reflect
the parameter order: change "spawnve() arg 2 cannot be empty" to "spawnve() arg
3 cannot be empty" and "spawnve() arg 2 first element cannot be empty" to
"spawnve() arg 3 first element cannot be empty".


let argv_spawn: Vec<*const u16> = argv
.iter()
.map(|v| v.as_ptr())
.chain(once(std::ptr::null()))
.collect();

// Build environment strings as "KEY=VALUE\0" wide strings
let mut env_strings: Vec<widestring::WideCString> = Vec::new();
for (key, value) in env.into_iter() {
let key = PyStrRef::try_from_object(vm, key)?;
let value = PyStrRef::try_from_object(vm, value)?;
let key_str = key.as_str();
let value_str = value.as_str();

// Validate: no null characters in key or value
if key_str.contains('\0') || value_str.contains('\0') {
return Err(vm.new_value_error("embedded null character"));
}
// Validate: no '=' in key
if key_str.contains('=') {
return Err(vm.new_value_error("illegal environment variable name"));
}

let env_str = format!("{}={}", key_str, value_str);
env_strings.push(make_widestring(&env_str)?);
}

let envp: Vec<*const u16> = env_strings
.iter()
.map(|s| s.as_ptr())
.chain(once(std::ptr::null()))
.collect();

let result = unsafe {
suppress_iph!(_wspawnve(
mode,
path.as_ptr(),
argv_spawn.as_ptr(),
envp.as_ptr()
))
};
if result == -1 {
Err(errno_err(vm))
} else {
Ok(result)
}
}

#[cfg(target_env = "msvc")]
#[pyfunction]
fn execv(
path: PyStrRef,
path: OsPath,
argv: Either<PyListRef, PyTupleRef>,
vm: &VirtualMachine,
) -> PyResult<()> {
Expand All @@ -226,7 +384,7 @@ pub(crate) mod module {
let make_widestring =
|s: &str| widestring::WideCString::from_os_str(s).map_err(|err| err.to_pyexception(vm));

let path = make_widestring(path.as_str())?;
let path = path.to_wide_cstring(vm)?;

let argv = vm.extract_elements_with(argv.as_ref(), |obj| {
let arg = PyStrRef::try_from_object(vm, obj)?;
Expand Down Expand Up @@ -254,6 +412,76 @@ pub(crate) mod module {
}
}

#[cfg(target_env = "msvc")]
#[pyfunction]
fn execve(
path: OsPath,
argv: Either<PyListRef, PyTupleRef>,
env: PyDictRef,
vm: &VirtualMachine,
) -> PyResult<()> {
use std::iter::once;

let make_widestring =
|s: &str| widestring::WideCString::from_os_str(s).map_err(|err| err.to_pyexception(vm));

let path = path.to_wide_cstring(vm)?;

let argv = vm.extract_elements_with(argv.as_ref(), |obj| {
let arg = PyStrRef::try_from_object(vm, obj)?;
make_widestring(arg.as_str())
})?;

let first = argv
.first()
.ok_or_else(|| vm.new_value_error("execve: argv must not be empty"))?;

if first.is_empty() {
return Err(vm.new_value_error("execve: argv first element cannot be empty"));
}

let argv_execve: Vec<*const u16> = argv
.iter()
.map(|v| v.as_ptr())
.chain(once(std::ptr::null()))
.collect();

// Build environment strings as "KEY=VALUE\0" wide strings
let mut env_strings: Vec<widestring::WideCString> = Vec::new();
for (key, value) in env.into_iter() {
let key = PyStrRef::try_from_object(vm, key)?;
let value = PyStrRef::try_from_object(vm, value)?;
let key_str = key.as_str();
let value_str = value.as_str();

// Validate: no null characters in key or value
if key_str.contains('\0') || value_str.contains('\0') {
return Err(vm.new_value_error("embedded null character"));
}
// Validate: no '=' in key
if key_str.contains('=') {
return Err(vm.new_value_error("illegal environment variable name"));
}

let env_str = format!("{}={}", key_str, value_str);
env_strings.push(make_widestring(&env_str)?);
}

let envp: Vec<*const u16> = env_strings
.iter()
.map(|s| s.as_ptr())
.chain(once(std::ptr::null()))
.collect();

if (unsafe { suppress_iph!(_wexecve(path.as_ptr(), argv_execve.as_ptr(), envp.as_ptr())) }
== -1)
{
Err(errno_err(vm))
} else {
Ok(())
}
}

#[pyfunction]
fn _getfinalpathname(path: OsPath, vm: &VirtualMachine) -> PyResult {
let real = path
Expand Down Expand Up @@ -464,18 +692,59 @@ pub(crate) mod module {
raw_set_handle_inheritable(handle, inheritable).map_err(|e| e.to_pyexception(vm))
}

#[pyfunction]
fn mkdir(
#[derive(FromArgs)]
struct MkdirArgs<'a> {
#[pyarg(any)]
path: OsPath,
mode: OptionalArg<i32>,
dir_fd: DirFd<'_, { _os::MKDIR_DIR_FD as usize }>,
vm: &VirtualMachine,
) -> PyResult<()> {
let mode = mode.unwrap_or(0o777);
let [] = dir_fd.0;
let _ = mode;
let wide = path.to_wide_cstring(vm)?;
let res = unsafe { FileSystem::CreateDirectoryW(wide.as_ptr(), std::ptr::null_mut()) };
#[pyarg(any, default = 0o777)]
mode: i32,
#[pyarg(flatten)]
dir_fd: DirFd<'a, { _os::MKDIR_DIR_FD as usize }>,
}

#[pyfunction]
fn mkdir(args: MkdirArgs<'_>, vm: &VirtualMachine) -> PyResult<()> {
use windows_sys::Win32::Foundation::LocalFree;
use windows_sys::Win32::Security::Authorization::{
ConvertStringSecurityDescriptorToSecurityDescriptorW, SDDL_REVISION_1,
};
use windows_sys::Win32::Security::SECURITY_ATTRIBUTES;

let [] = args.dir_fd.0;
let wide = args.path.to_wide_cstring(vm)?;

// CPython special case: mode 0o700 sets a protected ACL
let res = if args.mode == 0o700 {
let mut sec_attr = SECURITY_ATTRIBUTES {
nLength: std::mem::size_of::<SECURITY_ATTRIBUTES>() as u32,
lpSecurityDescriptor: std::ptr::null_mut(),
bInheritHandle: 0,
};
// Set a discretionary ACL (D) that is protected (P) and includes
// inheritable (OICI) entries that allow (A) full control (FA) to
// SYSTEM (SY), Administrators (BA), and the owner (OW).
let sddl: Vec<u16> = "D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)\0"
.encode_utf16()
.collect();
let convert_result = unsafe {
ConvertStringSecurityDescriptorToSecurityDescriptorW(
sddl.as_ptr(),
SDDL_REVISION_1,
&mut sec_attr.lpSecurityDescriptor,
std::ptr::null_mut(),
)
};
if convert_result == 0 {
return Err(errno_err(vm));
}
let res =
unsafe { FileSystem::CreateDirectoryW(wide.as_ptr(), &sec_attr as *const _ as _) };
unsafe { LocalFree(sec_attr.lpSecurityDescriptor) };
res
} else {
unsafe { FileSystem::CreateDirectoryW(wide.as_ptr(), std::ptr::null_mut()) }
};

if res == 0 {
return Err(errno_err(vm));
}
Expand Down
Loading