mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
path-uri: clarify invalid host path errors (#28473)
## Why Ensure a consistent string format when exposing path conversion errors to the model. ## What - Render `PathUriParseError::InvalidFileUriPath` as `'$PATH' is invalid on '$OS'`.
This commit is contained in:
committed by
GitHub
Unverified
parent
40e7dda4d2
commit
7162030b37
@@ -154,11 +154,8 @@ fn sandbox_cwd(sandbox: &FileSystemSandboxContext) -> Result<SandboxCwd, JSONRPC
|
||||
}
|
||||
|
||||
fn native_sandbox_cwd(cwd: &PathUri) -> Result<AbsolutePathBuf, JSONRPCErrorError> {
|
||||
cwd.to_abs_path().map_err(|err| {
|
||||
invalid_request(format!(
|
||||
"file system sandbox cwd is not native to this exec-server host: {err}"
|
||||
))
|
||||
})
|
||||
cwd.to_abs_path()
|
||||
.map_err(|err| invalid_request(err.to_string()))
|
||||
}
|
||||
|
||||
fn helper_read_roots(runtime_paths: &ExecServerRuntimePaths) -> Vec<AbsolutePathBuf> {
|
||||
@@ -561,16 +558,16 @@ mod tests {
|
||||
FileSystemSpecialPath::project_roots(/*subpath*/ None),
|
||||
FileSystemAccessMode::Write,
|
||||
)]);
|
||||
let sandbox_context = sandbox_context_with_cwd(&policy, cwd);
|
||||
let sandbox_context = sandbox_context_with_cwd(&policy, cwd.clone());
|
||||
|
||||
let err = sandbox_cwd(&sandbox_context).expect_err("non-native cwd should be rejected");
|
||||
|
||||
assert_eq!(
|
||||
err,
|
||||
crate::rpc::invalid_request(
|
||||
"file system sandbox cwd is not native to this exec-server host: file URI contains an invalid absolute path"
|
||||
.to_string()
|
||||
)
|
||||
crate::rpc::invalid_request(format!(
|
||||
"'{cwd}' is invalid on '{}'",
|
||||
std::env::consts::OS
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -211,13 +211,17 @@ impl PathUri {
|
||||
return Err(PathUriParseError::JoinPathMustBeRelative(path.to_string()));
|
||||
}
|
||||
if path.contains('\0') {
|
||||
return Err(PathUriParseError::InvalidFileUriPath);
|
||||
return Err(PathUriParseError::InvalidFileUriPath {
|
||||
path: path.to_string(),
|
||||
});
|
||||
}
|
||||
if path.is_empty() {
|
||||
return Ok(self.clone());
|
||||
}
|
||||
if decode_bad_path_uri(&self.0).is_some() {
|
||||
return Err(PathUriParseError::InvalidFileUriPath);
|
||||
return Err(PathUriParseError::InvalidFileUriPath {
|
||||
path: self.to_string(),
|
||||
});
|
||||
}
|
||||
|
||||
let mut url = self.0.clone();
|
||||
@@ -280,18 +284,28 @@ impl PathUri {
|
||||
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
PathUriParseError::InvalidFileUriPath,
|
||||
PathUriParseError::InvalidFileUriPath {
|
||||
path: self.to_string(),
|
||||
},
|
||||
));
|
||||
}
|
||||
|
||||
let path = self.0.to_file_path().map_err(|()| {
|
||||
io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
PathUriParseError::InvalidFileUriPath,
|
||||
PathUriParseError::InvalidFileUriPath {
|
||||
path: self.to_string(),
|
||||
},
|
||||
)
|
||||
})?;
|
||||
AbsolutePathBuf::from_absolute_path_checked(path)
|
||||
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))
|
||||
AbsolutePathBuf::from_absolute_path_checked(path).map_err(|_| {
|
||||
io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
PathUriParseError::InvalidFileUriPath {
|
||||
path: self.to_string(),
|
||||
},
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
/// Returns a clone of the canonical URL.
|
||||
@@ -472,7 +486,9 @@ fn validate_file_url(url: &Url) -> Result<(), PathUriParseError> {
|
||||
if urlencoding::decode_binary(url.path().as_bytes()).contains(&0)
|
||||
&& decode_bad_path_uri(url).is_none()
|
||||
{
|
||||
return Err(PathUriParseError::InvalidFileUriPath);
|
||||
return Err(PathUriParseError::InvalidFileUriPath {
|
||||
path: url.to_string(),
|
||||
});
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -483,8 +499,8 @@ pub enum PathUriParseError {
|
||||
InvalidUri(#[from] url::ParseError),
|
||||
#[error("unsupported path URI scheme `{0}`")]
|
||||
UnsupportedScheme(String),
|
||||
#[error("file URI contains an invalid absolute path")]
|
||||
InvalidFileUriPath,
|
||||
#[error("'{path}' is invalid on '{os}'", os = std::env::consts::OS)]
|
||||
InvalidFileUriPath { path: String },
|
||||
#[error("credentials are not allowed in path URIs")]
|
||||
CredentialsNotAllowed,
|
||||
#[error("ports are not allowed in path URIs")]
|
||||
|
||||
@@ -42,7 +42,13 @@ fn non_native_uri_io_conversion_is_invalid_input() {
|
||||
.to_abs_path()
|
||||
.expect_err("URI should not be host-native");
|
||||
|
||||
assert_eq!(error.kind(), io::ErrorKind::InvalidInput);
|
||||
assert_eq!(
|
||||
(error.kind(), error.to_string()),
|
||||
(
|
||||
io::ErrorKind::InvalidInput,
|
||||
format!("'{uri}' is invalid on '{}'", std::env::consts::OS),
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -193,7 +199,9 @@ fn malformed_bad_path_uris_are_rejected() {
|
||||
] {
|
||||
assert_eq!(
|
||||
PathUri::parse(uri),
|
||||
Err(PathUriParseError::InvalidFileUriPath),
|
||||
Err(PathUriParseError::InvalidFileUriPath {
|
||||
path: uri.to_string(),
|
||||
}),
|
||||
"parsing {uri}"
|
||||
);
|
||||
}
|
||||
@@ -222,7 +230,9 @@ fn bad_path_uris_are_opaque_to_lexical_operations() {
|
||||
assert_eq!(uri.join(""), Ok(uri.clone()));
|
||||
assert_eq!(
|
||||
uri.join("child"),
|
||||
Err(PathUriParseError::InvalidFileUriPath)
|
||||
Err(PathUriParseError::InvalidFileUriPath {
|
||||
path: uri.to_string(),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
@@ -497,10 +507,12 @@ fn join_rejects_absolute_and_null_paths() {
|
||||
base.join("/src"),
|
||||
Err(PathUriParseError::JoinPathMustBeRelative(path)) if path == "/src"
|
||||
));
|
||||
assert!(matches!(
|
||||
assert_eq!(
|
||||
base.join("src\0file"),
|
||||
Err(PathUriParseError::InvalidFileUriPath)
|
||||
));
|
||||
Err(PathUriParseError::InvalidFileUriPath {
|
||||
path: "src\0file".to_string(),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user