Skip to content

nt is_dir,is_file,listmount,listvolume #6373

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.

Sign up for GitHub

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

Merged
youknowone merged 3 commits into from
Dec 9, 2025
Merged

nt is_dir,is_file,listmount,listvolume #6373

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d82577a
is_dir/is_file for windows
youknowone Dec 8, 2025
2e038cf
listmount/listvolume
youknowone Dec 8, 2025
2dab1f6
check_env_var_len
youknowone Dec 8, 2025
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
Prev Previous commit
check_env_var_len
  • Loading branch information
@youknowone
youknowone committed Dec 9, 2025
commit 2dab1f61a584ece35df9a1d14772cbf8c51ee515
1 change: 0 additions & 1 deletion Lib/test/test_os.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1152,7 +1152,6 @@ def test_putenv_unsetenv(self):
self.assertEqual(proc.stdout.rstrip(), repr(None))

# On OS X < 10.6, unsetenv() doesn't return a value (bpo-13415).
@unittest.expectedFailureIfWindows('TODO: RUSTPYTHON; (AssertionError: ValueError not raised by putenv)')
@support.requires_mac_ver(10, 6)
def test_putenv_unsetenv_error(self):
# Empty variable name is invalid.
Expand Down
3 changes: 3 additions & 0 deletions crates/common/src/windows.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ use std::{
os::windows::ffi::{OsStrExt, OsStringExt},
};

/// _MAX_ENV from Windows CRT stdlib.h - maximum environment variable size
pub const _MAX_ENV: usize = 32767;

pub trait ToWideString {
fn to_wide(&self) -> Vec<u16>;
fn to_wide_with_nul(&self) -> Vec<u16>;
Expand Down
19 changes: 19 additions & 0 deletions crates/vm/src/stdlib/os.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,20 @@ pub(super) mod _os {
}
}

/// 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<()> {
use crate::common::windows::_MAX_ENV;
if size > _MAX_ENV {
return Err(vm.new_value_error(format!(
"the environment variable is longer than {} characters",
_MAX_ENV
)));
}
Ok(())
}

#[pyfunction]
fn putenv(
key: Either<PyStrRef, PyBytesRef>,
Expand All @@ -442,6 +456,8 @@ 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
Expand All @@ -464,6 +480,9 @@ pub(super) mod _os {
),
));
}
// For unsetenv, size is key + '=' (no value, just clearing)
#[cfg(windows)]
check_env_var_len(key.len() + 1, vm)?;
Comment on lines +483 to +485
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Dec 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential off-by-one: missing null terminator in size calculation.

The comment states "size is key + '='" but Windows _MAX_ENV includes the null terminator. For unsetenv, Windows internally sets "KEY=\0", so the size should be key.len() + 2 (key + '=' + '\0'), not key.len() + 1.

         // For unsetenv, size is key + '=' (no value, just clearing)
         #[cfg(windows)]
-        check_env_var_len(key.len() + 1, vm)?;
+        check_env_var_len(key.len() + 2, vm)?;  // key + '=' + '\0'
📝 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
// For unsetenv, size is key + '=' (no value, just clearing)
#[cfg(windows)]
check_env_var_len(key.len() + 1, vm)?;
// For unsetenv, size is key + '=' (no value, just clearing)
#[cfg(windows)]
check_env_var_len(key.len() + 2, vm)?; // key + '=' + '\0'
🤖 Prompt for AI Agents 
In crates/vm/src/stdlib/os.rs around lines 483 to 485, the size computed for
unsetenv on Windows omits the null terminator; change the Windows branch to pass
key.len() + 2 (key + '=' + '\0') into check_env_var_len instead of key.len() + 1
so the length accounts for the terminating NUL when validating against _MAX_ENV.

let key = super::bytes_as_os_str(key, vm)?;
// SAFETY: requirements forwarded from the caller
unsafe { env::remove_var(key) };
Expand Down
Loading