mirror of
https://github.com/openai/codex.git
synced 2026-05-17 17:53:06 +00:00
[codex] Fix TUI wrapping for external borrowed slices (#21235)
Fixes #20587, reported by @noeljackson. This prevents the TUI wrapping code from panicking when `textwrap` returns a borrowed slice that does not point into the original source text. The fix follows the direction proposed by @misrtjakub in the issue comment: validate the borrowed slice pointer range first, and fall back to the existing owned-line mapper when the slice is external. - Guards borrowed wrapped slices before converting pointer offsets into byte ranges. - Reuses the existing owned-line range recovery path for external borrowed slices. - Adds coverage for rejecting borrowed slices outside the source text. End-user testing steps: - Start Codex in TUI mode under a PTY wrapper that can inject stdin after startup. - Inject `\x1b[200~test message\x1b[201~\r` after the TUI is ready. - Confirm Codex does not panic and the pasted text is handled normally. Local validation: - `cargo test -p codex-tui wrapping::tests::` - `cargo test -p codex-tui -- --skip status::tests::status_permissions_full_disk_managed_with_network_is_danger_full_access --skip status::tests::status_permissions_full_disk_managed_without_network_is_external_sandbox`
This commit is contained in:
@@ -49,8 +49,16 @@ where
|
||||
for (line_index, line) in textwrap::wrap(text, &opts).iter().enumerate() {
|
||||
match line {
|
||||
std::borrow::Cow::Borrowed(slice) => {
|
||||
let start = unsafe { slice.as_ptr().offset_from(text.as_ptr()) as usize };
|
||||
let end = start + slice.len();
|
||||
let range = borrowed_slice_range(text, slice).unwrap_or_else(|| {
|
||||
let synthetic_prefix = if line_index == 0 {
|
||||
opts.initial_indent
|
||||
} else {
|
||||
opts.subsequent_indent
|
||||
};
|
||||
map_owned_wrapped_line_to_range(text, cursor, slice, synthetic_prefix)
|
||||
});
|
||||
let start = range.start;
|
||||
let end = range.end;
|
||||
let trailing_spaces = text[end..].chars().take_while(|c| *c == ' ').count();
|
||||
lines.push(start..end + trailing_spaces + 1);
|
||||
cursor = end + trailing_spaces;
|
||||
@@ -84,10 +92,16 @@ where
|
||||
for (line_index, line) in textwrap::wrap(text, &opts).iter().enumerate() {
|
||||
match line {
|
||||
std::borrow::Cow::Borrowed(slice) => {
|
||||
let start = unsafe { slice.as_ptr().offset_from(text.as_ptr()) as usize };
|
||||
let end = start + slice.len();
|
||||
lines.push(start..end);
|
||||
cursor = end;
|
||||
let range = borrowed_slice_range(text, slice).unwrap_or_else(|| {
|
||||
let synthetic_prefix = if line_index == 0 {
|
||||
opts.initial_indent
|
||||
} else {
|
||||
opts.subsequent_indent
|
||||
};
|
||||
map_owned_wrapped_line_to_range(text, cursor, slice, synthetic_prefix)
|
||||
});
|
||||
cursor = range.end;
|
||||
lines.push(range);
|
||||
}
|
||||
std::borrow::Cow::Owned(slice) => {
|
||||
let synthetic_prefix = if line_index == 0 {
|
||||
@@ -104,6 +118,19 @@ where
|
||||
lines
|
||||
}
|
||||
|
||||
fn borrowed_slice_range(text: &str, slice: &str) -> Option<Range<usize>> {
|
||||
let text_start = text.as_ptr() as usize;
|
||||
let text_end = text_start.checked_add(text.len())?;
|
||||
let slice_start = slice.as_ptr() as usize;
|
||||
let slice_end = slice_start.checked_add(slice.len())?;
|
||||
|
||||
if slice_start < text_start || slice_end > text_end {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some((slice_start - text_start)..(slice_end - text_start))
|
||||
}
|
||||
|
||||
/// Maps an owned (materialized) wrapped line back to a byte range in `text`.
|
||||
///
|
||||
/// `textwrap` returns `Cow::Owned` when it inserts a hyphenation penalty
|
||||
@@ -1485,6 +1512,17 @@ them."#
|
||||
assert_eq!(range, 0..5);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn borrowed_slice_range_rejects_slices_outside_source_text() {
|
||||
let text = "test message";
|
||||
let external = String::from("test");
|
||||
|
||||
assert_eq!(borrowed_slice_range(text, &external), None);
|
||||
|
||||
let fallback = map_owned_wrapped_line_to_range(text, /*cursor*/ 0, &external, "");
|
||||
assert_eq!(fallback, 0..4);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn map_owned_wrapped_line_to_range_indent_coincides_with_source() {
|
||||
// When the synthetic indent prefix starts with a character that also
|
||||
|
||||
Reference in New Issue
Block a user