|
Expand Up
|
@@ -7,7 +7,7 @@ use crate::{ |
|
|
convert::{IntoPyException, ToPyException, ToPyObject}, |
|
|
function::{ArgumentError, FromArgs, FuncArgs}, |
|
|
}; |
|
|
use std::{ffi, fs, io, path::Path}; |
|
|
use std::{fs, io, path::Path}; |
|
|
|
|
|
pub(crate) fn fs_metadata<P: AsRef<Path>>( |
|
|
path: P, |
|
Expand Down
Expand Up
|
@@ -112,7 +112,8 @@ pub(super) struct FollowSymlinks( |
|
|
#[pyarg(named, name = "follow_symlinks", default = true)] pub bool, |
|
|
); |
|
|
|
|
|
fn bytes_as_os_str<'a>(b: &'a [u8], vm: &VirtualMachine) -> PyResult<&'a ffi::OsStr> { |
|
|
#[cfg(not(windows))] |
|
|
fn bytes_as_os_str<'a>(b: &'a [u8], vm: &VirtualMachine) -> PyResult<&'a std::ffi::OsStr> { |
|
|
rustpython_common::os::bytes_as_os_str(b) |
|
|
.map_err(|_| vm.new_unicode_decode_error("can't decode path for utf-8")) |
|
|
} |
|
Expand Down
Expand Up
|
@@ -160,7 +161,7 @@ pub(super) mod _os { |
|
|
suppress_iph, |
|
|
}, |
|
|
convert::{IntoPyException, ToPyObject}, |
|
|
function::{ArgBytesLike, Either, FsPath, FuncArgs, OptionalArg}, |
|
|
function::{ArgBytesLike, FsPath, FuncArgs, OptionalArg}, |
|
|
ospath::{IOErrorBuilder, OsPath, OsPathOrFd, OutputMode}, |
|
|
protocol::PyIterReturn, |
|
|
recursion::ReprGuard, |
|
Expand All
|
@@ -171,7 +172,7 @@ pub(super) mod _os { |
|
|
use crossbeam_utils::atomic::AtomicCell; |
|
|
use itertools::Itertools; |
|
|
use std::{ |
|
|
env, ffi, fs, |
|
|
env, fs, |
|
|
fs::OpenOptions, |
|
|
io, |
|
|
path::PathBuf, |
|
Expand Down
Expand Up
|
@@ -403,7 +404,7 @@ pub(super) mod _os { |
|
|
b"." | b".." => None, |
|
|
_ => Some( |
|
|
OutputMode::String |
|
|
.process_path(ffi::OsStr::from_bytes(fname), vm), |
|
|
.process_path(std::ffi::OsStr::from_bytes(fname), vm), |
|
|
), |
|
|
} |
|
|
}) |
|
Expand All
|
@@ -415,31 +416,62 @@ pub(super) mod _os { |
|
|
Ok(list) |
|
|
} |
|
|
|
|
|
fn env_bytes_as_bytes(obj: &Either<PyStrRef, PyBytesRef>) -> &[u8] { |
|
|
#[cfg(not(windows))] |
|
|
fn env_bytes_as_bytes(obj: &crate::function::Either<PyStrRef, PyBytesRef>) -> &[u8] { |
|
|
match obj { |
|
|
Either::A(s) => s.as_bytes(), |
|
|
Either::B(b) => b.as_bytes(), |
|
|
crate::function::Either::A(s) => s.as_bytes(), |
|
|
crate::function::Either::B(b) => b.as_bytes(), |
|
|
} |
|
|
} |
|
|
|
|
|
/// Check if environment variable length exceeds Windows limit. |
|
|
/// size should be key.len() + value.len() + 2 (for '=' and null terminator) |
|
|
#[cfg(windows)] |
|
|
fn check_env_var_len(size: usize, vm: &VirtualMachine) -> PyResult<()> { |
|
|
unsafe extern "C" { |
|
|
fn _wputenv(envstring: *const u16) -> libc::c_int; |
|
|
} |
|
|
|
|
|
/// Check if wide string length exceeds Windows environment variable limit. |
|
|
#[cfg(windows)] |
|
|
fn check_env_var_len(wide_len: usize, vm: &VirtualMachine) -> PyResult<()> { |
|
|
use crate::common::windows::_MAX_ENV; |
|
|
if size > _MAX_ENV { |
|
|
if wide_len > _MAX_ENV + 1 { |
|
|
return Err(vm.new_value_error(format!( |
|
|
"the environment variable is longer than {} characters", |
|
|
_MAX_ENV |
|
|
"the environment variable is longer than {_MAX_ENV} characters", |
|
|
))); |
|
|
} |
|
|
Ok(()) |
|
|
} |
|
|
|
|
|
#[cfg(windows)] |
|
|
#[pyfunction] |
|
|
fn putenv(key: PyStrRef, value: PyStrRef, vm: &VirtualMachine) -> PyResult<()> { |
|
|
let key_str = key.as_str(); |
|
|
let value_str = value.as_str(); |
|
|
// Search from index 1 because on Windows starting '=' is allowed for |
|
|
// defining hidden environment variables. |
|
|
if key_str.is_empty() |
|
|
|| key_str.get(1..).is_some_and(|s| s.contains('=')) |
|
|
|| key_str.contains('\0') |
|
|
|| value_str.contains('\0') |
|
|
{ |
|
|
return Err(vm.new_value_error("illegal environment variable name")); |
|
|
} |
|
|
let env_str = format!("{}={}", key_str, value_str); |
|
|
let wide = env_str.to_wide_with_nul(); |
|
|
check_env_var_len(wide.len(), vm)?; |
|
|
|
|
|
// Use _wputenv like CPython (not SetEnvironmentVariableW) to update CRT environ |
|
|
let result = unsafe { suppress_iph!(_wputenv(wide.as_ptr())) }; |
|
|
if result != 0 { |
|
|
return Err(vm.new_last_errno_error()); |
|
|
} |
|
|
Ok(()) |
|
|
} |
Comment thread
Comment on lines
+444
to
+468
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.
⚠️ Potential issue | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
# Search for test cases related to putenv
rg -n "putenv|os.putenv" --type=py crates/ Repository: RustPython/RustPython
Length of output: 186
🏁 Script executed:
# Check if there are any test files for the os module
fd -e py -p test | rg -i "test.*os|os.*test" | head -20 Repository: RustPython/RustPython
Length of output: 186
🏁 Script executed:
# Look at the git context to see if there are comments or commits related to this
rg -n "hidden.*environment\|environment.*variable.*name" --type=rs Repository: RustPython/RustPython
Length of output: 186
🌐 Web query:
CPython putenv Windows environment variable validation
💡 Result:
Short answer: CPython validates environment-variable names for os.putenv / os.environ on Windows — names may not contain '=' except possibly as the first character (Windows "hidden" variables like "=C:..."); CPython rejects names with '=' elsewhere and keeps behavior in sync with the C runtime/Windows APIs. See Python issue tracker discussions and the related CPython changes. [1][2][3]
Sources
- Reject '=' in env var names (issue discussion & PRs). [1]
- Windows parsing/leading '=' (hidden) behavior and tests. [2]
- Changes to os.putenv/os.unsetenv implementation and Windows notes (CRT vs SetEnvironmentVariable). [3]
🌐 Web query:
Windows environment variable naming rules "=" character
💡 Result:
Short answer: Windows environment variable names cannot contain the equals sign ("="). Environment strings are stored as "Name=Value\0" so the '=' is the separator and is not allowed in the name. [1][2]
References:
- Microsoft: Environment Variables (Win32) — "The name of an environment variable cannot include an equal sign (=)." [1]
- POSIX / Open Group: environment strings have the form name=value and "names shall not contain the character '='." (useful for general background). [2]
🌐 Web query:
CPython source code putenv Windows implementation validation
💡 Result:
Short answer: CPython validates Windows putenv names and values in Modules/posixmodule.c (win32_putenv). On Windows it:
- rejects empty names and any name containing '=' at or after index 1 (leading '=' is allowed for hidden vars).
- builds a "NAME=VALUE" wide string and uses PyUnicode_AsWideCharString (which rejects embedded NULs).
- checks the combined length against _MAX_ENV and raises ValueError if too long.
- calls the CRT _wputenv (so CRT updates _wenviron/_environ).
Sources: CPython win32_putenv implementation in Modules/posixmodule.c and the related tracker/commit discussion. [1][2]
References
- win32_putenv implementation (Modules/posixmodule.c). Shows the '=' check, NUL rejection via PyUnicode_AsWideCharString, length check against _MAX_ENV, and use of _wputenv. [1]
- bpo-39406 discussion / commits about using CRT putenv on Windows (context why CPython prefers _wputenv). [2]
[1] Modules/posixmodule.c — win32_putenv implementation.
[2] bpo-39406 / CPython commit discussion (os.putenv() behavior on Windows).
Fix edge case: key consisting of only "=" should be rejected
The validation logic allows a key consisting of just "=", which is invalid. The check key_str.get(1..).is_some_and(|s| s.contains('=')) returns false for this case because the substring after index 1 is empty.
Per Windows environment variable rules and CPython's implementation, "=" alone is not a valid environment variable name (it is the separator character in "NAME=VALUE" format). While the code correctly allows leading "=" for Windows hidden variables (e.g., "=C:"), it should reject a key that is only "=".
Add an additional check: || key_str == "=" to explicitly reject this edge case.
🤖 Prompt for AI Agents
In crates/vm/src/stdlib/os.rs around lines 446 to 476, the current validation
for Windows environment variable keys mistakenly allows a key equal to "=";
update the existing conditional that rejects empty, null-containing, or
'='-containing keys to also explicitly reject the single-character key "="
(e.g., add a check like key_str == "=" to the OR chain) so that a lone "=" is
treated as an invalid environment variable name.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
|
|
|
|
|
#[cfg(not(windows))] |
|
|
#[pyfunction] |
|
|
fn putenv( |
|
|
key: Either<PyStrRef, PyBytesRef>, |
|
|
value: Either<PyStrRef, PyBytesRef>, |
|
|
key: crate::function::Either<PyStrRef, PyBytesRef>, |
|
|
value: crate::function::Either<PyStrRef, PyBytesRef>, |
|
|
vm: &VirtualMachine, |
|
|
) -> PyResult<()> { |
|
|
let key = env_bytes_as_bytes(&key); |
|
Expand All
|
@@ -450,17 +482,44 @@ pub(super) mod _os { |
|
|
if key.is_empty() || key.contains(&b'=') { |
|
|
return Err(vm.new_value_error("illegal environment variable name")); |
|
|
} |
|
|
#[cfg(windows)] |
|
|
check_env_var_len(key.len() + value.len() + 2, vm)?; |
|
|
let key = super::bytes_as_os_str(key, vm)?; |
|
|
let value = super::bytes_as_os_str(value, vm)?; |
|
|
// SAFETY: requirements forwarded from the caller |
|
|
unsafe { env::set_var(key, value) }; |
|
|
Ok(()) |
|
|
} |
|
|
|
|
|
#[cfg(windows)] |
|
|
#[pyfunction] |
|
|
fn unsetenv(key: PyStrRef, vm: &VirtualMachine) -> PyResult<()> { |
|
|
let key_str = key.as_str(); |
|
|
// Search from index 1 because on Windows starting '=' is allowed for |
|
|
// defining hidden environment variables. |
|
|
if key_str.is_empty() |
|
|
|| key_str.get(1..).is_some_and(|s| s.contains('=')) |
|
|
|| key_str.contains('\0') |
|
|
{ |
|
|
return Err(vm.new_value_error("illegal environment variable name")); |
|
|
} |
|
|
// "key=" to unset (empty value removes the variable) |
|
|
let env_str = format!("{}=", key_str); |
|
|
let wide = env_str.to_wide_with_nul(); |
|
|
check_env_var_len(wide.len(), vm)?; |
|
|
|
|
|
// Use _wputenv like CPython (not SetEnvironmentVariableW) to update CRT environ |
|
|
let result = unsafe { suppress_iph!(_wputenv(wide.as_ptr())) }; |
|
|
if result != 0 { |
|
|
return Err(vm.new_last_errno_error()); |
|
|
} |
|
|
Ok(()) |
|
|
} |
Comment thread
Comment on lines
+492
to
+515
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.
⚠️ Potential issue | 🔴 Critical
Critical issue: key consisting of only "=" is incorrectly accepted.
The validation at lines 498-502 has the same flaw as putenv above. When key_str = "=", the check key_str.get(1..).is_some_and(|s| s.contains('=')) returns false because the substring after index 1 is empty.
A key of just "=" is not a valid environment variable name and should be rejected.
Apply this diff to fix the validation:
if key_str.is_empty()
|| key_str.get(1..).is_some_and(|s| s.contains('='))
+ || key_str == "="
|| key_str.contains('\0')
{
🤖 Prompt for AI Agents
In crates/vm/src/stdlib/os.rs around lines 492 to 515, the validation allows a
key consisting only of "=" because checking key_str.get(1..) misses the case
where the first character is '='; update the guard to also reject keys that
start with '=' (e.g., key_str.starts_with('=') or key_str.chars().next() ==
Some('=')) so that a lone "=" or any name beginning with '=' is treated as
invalid, while keeping the existing empty and NUL checks and the subsequent
logic unchanged.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
|
|
|
|
|
#[cfg(not(windows))] |
|
|
#[pyfunction] |
|
|
fn unsetenv(key: Either<PyStrRef, PyBytesRef>, vm: &VirtualMachine) -> PyResult<()> { |
|
|
fn unsetenv( |
|
|
key: crate::function::Either<PyStrRef, PyBytesRef>, |
|
|
vm: &VirtualMachine, |
|
|
) -> PyResult<()> { |
|
|
let key = env_bytes_as_bytes(&key); |
|
|
if key.contains(&b'\0') { |
|
|
return Err(vm.new_value_error("embedded null byte")); |
|
Expand All
|
@@ -474,9 +533,6 @@ pub(super) mod _os { |
|
|
), |
|
|
)); |
|
|
} |
|
|
// For unsetenv, size is key + '=' (no value, just clearing) |
|
|
#[cfg(windows)] |
|
|
check_env_var_len(key.len() + 1, vm)?; |
|
|
let key = super::bytes_as_os_str(key, vm)?; |
|
|
// SAFETY: requirements forwarded from the caller |
|
|
unsafe { env::remove_var(key) }; |
|
Expand Down
Expand Up
|
@@ -984,7 +1040,7 @@ pub(super) mod _os { |
|
|
OsPathOrFd::Path(path) => { |
|
|
use rustpython_common::os::ffi::OsStrExt; |
|
|
let path = path.as_ref().as_os_str().as_bytes(); |
|
|
let path = match ffi::CString::new(path) { |
|
|
let path = match std::ffi::CString::new(path) { |
|
|
Ok(x) => x, |
|
|
Err(_) => return Ok(None), |
|
|
}; |
|
Expand Down
Expand Up
|
@@ -1483,7 +1539,7 @@ pub(super) mod _os { |
|
|
|
|
|
#[pyfunction] |
|
|
fn strerror(e: i32) -> String { |
|
|
unsafe { ffi::CStr::from_ptr(libc::strerror(e)) } |
|
|
unsafe { std::ffi::CStr::from_ptr(libc::strerror(e)) } |
|
|
.to_string_lossy() |
|
|
.into_owned() |
|
|
} |
|
Expand Down
Expand Up
|
@@ -1587,7 +1643,7 @@ pub(super) mod _os { |
|
|
if encoding.is_null() || encoding.read() == '\0' as libc::c_char { |
|
|
"UTF-8".to_owned() |
|
|
} else { |
|
|
ffi::CStr::from_ptr(encoding).to_string_lossy().into_owned() |
|
|
std::ffi::CStr::from_ptr(encoding).to_string_lossy().into_owned() |
|
|
} |
|
|
}; |
|
|
|
|
Expand Down
|
|