mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] make PathUri::from_abs_path infallible (#27976)
## Why `PathUri::from_abs_path` can fail for absolute paths that do not have a normal `file:` URI representation, forcing filesystem call sites to handle a conversion error even though the original path can be preserved losslessly. ## What Make `from_abs_path` infallible and migrate its callers. Unrepresentable paths use `file:///%00/bad/path/<base64>`, encoding Unix bytes or Windows UTF-16LE; `to_abs_path` validates and decodes that fallback. The leading encoded null reserves a namespace that cannot collide with a real Unix or Windows path, and fallback URIs remain opaque to lexical path operations. ## Validation Added path-URI coverage for Unix null and non-UTF-8 paths, Windows device/verbatim and non-Unicode paths, serialization, malformed fallbacks, opaque lexical operations, invalid native payloads, and literal `/bad/path` collision resistance.
This commit is contained in:
@@ -186,17 +186,7 @@ pub async fn verify_apply_patch_args(
|
||||
);
|
||||
}
|
||||
Hunk::DeleteFile { .. } => {
|
||||
let path_uri = match PathUri::from_abs_path(&path) {
|
||||
Ok(path_uri) => path_uri,
|
||||
Err(e) => {
|
||||
return MaybeApplyPatchVerified::CorrectnessError(
|
||||
ApplyPatchError::IoError(IoError {
|
||||
context: format!("Failed to read {}", path.display()),
|
||||
source: e,
|
||||
}),
|
||||
);
|
||||
}
|
||||
};
|
||||
let path_uri = PathUri::from_abs_path(&path);
|
||||
let content = match fs.read_file_text(&path_uri, sandbox).await {
|
||||
Ok(content) => content,
|
||||
Err(e) => {
|
||||
|
||||
@@ -391,7 +391,7 @@ async fn apply_hunks_to_files(
|
||||
for hunk in hunks {
|
||||
let affected_path = hunk.path().to_path_buf();
|
||||
let path_abs = hunk.resolve_path(cwd);
|
||||
let path_uri = PathUri::from_abs_path(&path_abs)?;
|
||||
let path_uri = PathUri::from_abs_path(&path_abs);
|
||||
match hunk {
|
||||
Hunk::AddFile { contents, .. } => {
|
||||
let overwritten_content =
|
||||
@@ -556,7 +556,7 @@ async fn ensure_not_directory(
|
||||
fs: &dyn ExecutorFileSystem,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> io::Result<()> {
|
||||
let path_uri = PathUri::from_abs_path(path)?;
|
||||
let path_uri = PathUri::from_abs_path(path);
|
||||
let metadata = fs.get_metadata(&path_uri, sandbox).await?;
|
||||
if metadata.is_directory {
|
||||
return Err(io::Error::new(
|
||||
@@ -573,9 +573,7 @@ async fn remove_failure_was_side_effect_free(
|
||||
fs: &dyn ExecutorFileSystem,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> bool {
|
||||
let Ok(path_uri) = PathUri::from_abs_path(path) else {
|
||||
return false;
|
||||
};
|
||||
let path_uri = PathUri::from_abs_path(path);
|
||||
match expected_content {
|
||||
Some(expected_content) => fs
|
||||
.read_file_text(&path_uri, sandbox)
|
||||
@@ -592,13 +590,7 @@ async fn read_optional_file_text_for_delta(
|
||||
exact: &mut bool,
|
||||
) -> Option<String> {
|
||||
note_existing_path_delta_support(path, fs, sandbox, exact).await;
|
||||
let path_uri = match PathUri::from_abs_path(path) {
|
||||
Ok(path_uri) => path_uri,
|
||||
Err(_) => {
|
||||
*exact = false;
|
||||
return None;
|
||||
}
|
||||
};
|
||||
let path_uri = PathUri::from_abs_path(path);
|
||||
match fs.read_file_text(&path_uri, sandbox).await {
|
||||
Ok(content) => Some(content),
|
||||
Err(source) if source.kind() == io::ErrorKind::NotFound => None,
|
||||
@@ -615,10 +607,7 @@ async fn note_existing_path_delta_support(
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
exact: &mut bool,
|
||||
) {
|
||||
let Ok(path_uri) = PathUri::from_abs_path(path) else {
|
||||
*exact = false;
|
||||
return;
|
||||
};
|
||||
let path_uri = PathUri::from_abs_path(path);
|
||||
match fs.get_metadata(&path_uri, sandbox).await {
|
||||
Ok(metadata) if metadata.is_file && !metadata.is_symlink => {}
|
||||
Ok(_) => *exact = false,
|
||||
@@ -633,12 +622,12 @@ async fn write_file_with_missing_parent_retry(
|
||||
contents: Vec<u8>,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> anyhow::Result<()> {
|
||||
let path_uri = PathUri::from_abs_path(path_abs)?;
|
||||
let path_uri = PathUri::from_abs_path(path_abs);
|
||||
match fs.write_file(&path_uri, contents.clone(), sandbox).await {
|
||||
Ok(()) => Ok(()),
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => {
|
||||
if let Some(parent_abs) = path_abs.parent() {
|
||||
let parent_uri = PathUri::from_abs_path(&parent_abs)?;
|
||||
let parent_uri = PathUri::from_abs_path(&parent_abs);
|
||||
fs.create_directory(
|
||||
&parent_uri,
|
||||
CreateDirectoryOptions { recursive: true },
|
||||
@@ -676,7 +665,7 @@ async fn derive_new_contents_from_chunks(
|
||||
fs: &dyn ExecutorFileSystem,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> std::result::Result<AppliedPatch, ApplyPatchError> {
|
||||
let path_uri = PathUri::from_abs_path(path_abs)?;
|
||||
let path_uri = PathUri::from_abs_path(path_abs);
|
||||
let original_contents = fs.read_file_text(&path_uri, sandbox).await.map_err(|err| {
|
||||
ApplyPatchError::IoError(IoError {
|
||||
context: format!("Failed to read file to update {}", path_abs.display()),
|
||||
|
||||
Reference in New Issue
Block a user