Validate UI trees at decode time and simplify display elements (#59518)

## Why

UI tree limits previously applied during rendering, silently dropping content
beyond the budgets. Validate the whole JSON tree before rendering so oversized
inputs are rejected, including content outside the visible line.

## What changed

- Add a `UiTree` string parser that limits JSON to 64 KiB, nesting depth to 16,
  non-null nodes to 256, and total string content to 16 KiB of UTF-8.
- Represent text as string leaves or `text` elements with children. Remove node
  keys, `spacer`, and `divider` elements.
- Require an explicit `row` gap as a non-negative JavaScript-safe integer measured
  in terminal columns. Bound rendered spacing by the viewport and skip empty
  children when inserting gaps.
- Update footer content and built-in status lines to use the revised tree format.

## Testing

Add coverage for malformed elements and gaps, whole-tree size boundaries, URL
length limits, large-gap rendering, and bounded UTF-8 prefixes for directly
constructed text. Update hyperlink and footer clipping tests for the new format.

GitOrigin-RevId: 3c00881d3e7f2d79d11aa188f64f0725cd47ee68
This commit is contained in:
Joe Savona
2026-11-04 23:31:55 +00:00
committed by copyberry
parent 08511817f3
commit 47172f064f
7 changed files with 211 additions and 173 deletions
@@ -25,17 +25,18 @@ fn composer() -> ChatComposer {
#[test]
fn extension_status_line_clips_links_to_footer_layout() {
let mut composer = composer();
let tree = serde_json::from_value(serde_json::json!({
"type": "row", "props": {}, "children": [
{ "type": "text", "props": { "text": "plain" } },
{ "type": "row", "props": {}, "children": [
{ "type": "text", "props": { "text": " · " } },
let tree = r#"{
"type": "row", "props": {"gap": 0}, "children": [
"plain",
{ "type": "row", "props": {"gap": 0}, "children": [
" · ",
{ "type": "link", "props": { "href": "https://example.com/pr/42" }, "children": [
{ "type": "text", "props": { "text": "PR #42" } }
"PR #42"
] }
] }
]
}))
}"#
.parse()
.unwrap();
composer.set_status_line(Some(FooterContent::Extension { content: tree }));
@@ -8,14 +8,14 @@ use ratatui::widgets::Widget;
use crate::line_truncation::truncate_line_with_ellipsis_if_overflow;
use crate::terminal_hyperlinks::HyperlinkLine;
use crate::ui_tree::UiNode;
use crate::ui_tree::UiTree;
/// Owned content for the footer's negotiated left-hand area.
#[derive(Clone, Debug, PartialEq)]
pub(crate) enum FooterContent {
Line(Line<'static>),
HyperlinkLine(HyperlinkLine),
Extension { content: UiNode },
Extension { content: UiTree },
}
impl FooterContent {
@@ -4,11 +4,11 @@ use super::STATUS_LINE_SEPARATOR;
use super::footer_content::FooterContent;
use super::status_line_from_segments;
use super::status_line_setup::StatusLineItem;
use crate::ui_tree::Key;
use crate::ui_tree::LinkHref;
use crate::ui_tree::LinkProps;
use crate::ui_tree::StackProps;
use crate::ui_tree::RowProps;
use crate::ui_tree::UiNode;
use crate::ui_tree::UiTree;
use codex_protocol::ThreadId;
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -38,27 +38,27 @@ impl StatusLineRenderer {
let mut children = Vec::new();
for (item, text) in segments {
if !children.is_empty() {
children.push(Some(UiNode::text(/*key*/ None, STATUS_LINE_SEPARATOR)));
children.push(Some(UiNode::String(STATUS_LINE_SEPARATOR.to_owned())));
}
let key = Some(Key::String(item.to_string()));
let text = UiNode::String(text);
let node = if item == StatusLineItem::PullRequestNumber
&& let Some(href) = pull_request_url.and_then(|url| LinkHref::try_from(url).ok())
{
UiNode::Link {
key,
props: LinkProps { href, title: None },
children: vec![Some(UiNode::text(/*key*/ None, text))],
children: vec![Some(text)],
}
} else {
UiNode::text(key, text)
text
};
children.push(Some(node));
}
let tree = UiNode::Row {
key: Some(Key::String("status-line".to_string())),
props: StackProps::default(),
props: RowProps { gap: 0 },
children,
};
Some(FooterContent::Extension { content: tree })
Some(FooterContent::Extension {
content: UiTree(Some(tree)),
})
}
}
+24 -52
View File
@@ -1,10 +1,6 @@
//! Bounded, display-only subset of the `codex-tui/1` tree contract.
//!
//! The wire tree is independent of its producer. Producers normalize text into leaf nodes and
//! arrays into composite nodes; unsupported primitives and properties fail deserialization. This
//! first slice renders one horizontal line of plain text, links, rows, spacers, dividers, and
//! numeric gaps. Links underline their text. Keys are preserved for future reconciliation.
//! Display elements for the terminal UI and their JSON representation.
mod decode;
mod render;
#[cfg(test)]
@@ -14,47 +10,36 @@ mod tests;
use serde::Deserialize;
use serde::Serialize;
/// A single display node, or no content.
pub(crate) type UiTree = Option<UiNode>;
const MAX_GAP: u64 = (1 << 53) - 1;
const MAX_TEXT_BYTES: usize = 16 * 1024;
/// A nullable root with bounded depth, node count, and total UTF-8 leaf bytes.
#[derive(Clone, Debug, PartialEq, Serialize)]
pub(crate) struct UiTree(pub(crate) Option<UiNode>);
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(tag = "type", rename_all = "kebab-case", deny_unknown_fields)]
pub(crate) enum UiNode {
Text {
#[serde(skip_serializing_if = "Option::is_none")]
key: Option<Key>,
#[serde(default, skip_serializing)]
props: TextProps,
children: Vec<Option<UiNode>>,
},
Link {
#[serde(skip_serializing_if = "Option::is_none")]
key: Option<Key>,
props: LinkProps,
children: Vec<UiTree>,
children: Vec<Option<UiNode>>,
},
Row {
#[serde(skip_serializing_if = "Option::is_none")]
key: Option<Key>,
props: StackProps,
children: Vec<UiTree>,
},
Spacer {
#[serde(skip_serializing_if = "Option::is_none")]
key: Option<Key>,
},
Divider {
#[serde(skip_serializing_if = "Option::is_none")]
key: Option<Key>,
props: RowProps,
children: Vec<Option<UiNode>>,
},
#[serde(untagged)]
String(String),
}
impl UiNode {
pub(crate) fn text(key: Option<Key>, text: impl Into<String>) -> Self {
Self::Text {
key,
props: TextProps { text: text.into() },
}
}
}
#[derive(Clone, Debug, Default, PartialEq, Deserialize)]
#[serde(deny_unknown_fields)]
pub(crate) struct TextProps {}
#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
@@ -64,7 +49,7 @@ pub(crate) struct LinkProps {
pub(crate) title: Option<String>,
}
/// An absolute HTTP(S) destination safe to embed in a terminal hyperlink.
/// An absolute HTTP(S) URL without control characters, at most 8 KiB in UTF-8.
#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
#[serde(try_from = "String")]
pub(crate) struct LinkHref(String);
@@ -84,27 +69,14 @@ impl TryFrom<&str> for LinkHref {
crate::terminal_hyperlinks::web_destination(value)
.filter(|destination| destination == value)
.map(Self)
.ok_or("link href must be an absolute HTTP(S) URL without control characters")
.ok_or("link href must be an absolute HTTP(S) URL of at most 8 KiB without control characters")
}
}
#[derive(Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
pub(crate) enum Key {
String(String),
Number(serde_json::Number),
}
#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub(crate) struct TextProps {
pub(crate) text: String,
}
#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub(crate) struct StackProps {
/// Numeric spacing uses the profile's four-point scale.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub(crate) gap: Option<f64>,
pub(crate) struct RowProps {
/// Horizontal spacing in terminal columns, expressed as a non-negative JavaScript-safe integer.
#[serde(deserialize_with = "decode::gap")]
pub(crate) gap: u64,
}
+54
View File
@@ -0,0 +1,54 @@
//! Enforce the input byte limit before parsing and the UI tree limits after deserialization.
use super::MAX_GAP;
use super::MAX_TEXT_BYTES;
use super::UiNode;
use super::UiTree;
use serde::Deserialize;
use serde::Deserializer;
use serde::de::Error;
impl std::str::FromStr for UiTree {
type Err = serde_json::Error;
fn from_str(text: &str) -> Result<Self, Self::Err> {
if text.len() > 64 * 1024 {
return Err(Self::Err::custom("UI tree JSON exceeds 64 KiB"));
}
let root: Option<UiNode> = serde_json::from_str(text)?;
let mut pending: Vec<_> = root.iter().map(|node| (node, 0)).collect();
let mut nodes = 0;
let mut text_bytes = 0;
while let Some((node, depth)) = pending.pop() {
if depth > 16 {
return Err(Self::Err::custom("UI tree exceeds maximum depth 16"));
}
nodes += 1;
if nodes > 256 {
return Err(Self::Err::custom("UI tree exceeds 256 non-null nodes"));
}
match node {
UiNode::String(text) => {
text_bytes += text.len();
if text_bytes > MAX_TEXT_BYTES {
return Err(Self::Err::custom("UI tree exceeds 16 KiB of text"));
}
}
UiNode::Text { children, .. }
| UiNode::Link { children, .. }
| UiNode::Row { children, .. } => {
pending.extend(children.iter().flatten().map(|child| (child, depth + 1)));
}
}
}
Ok(Self(root))
}
}
pub(super) fn gap<'de, D: Deserializer<'de>>(de: D) -> Result<u64, D::Error> {
let gap = u64::deserialize(de)?;
if gap > MAX_GAP {
return Err(D::Error::custom("gap exceeds the maximum safe integer"));
}
Ok(gap)
}
+38 -70
View File
@@ -1,5 +1,6 @@
//! Render the visible prefix of a horizontal display tree into one terminal line.
use super::MAX_TEXT_BYTES;
use super::UiNode;
use super::UiTree;
use crate::history_cell::sanitize_user_text;
@@ -8,27 +9,14 @@ use crate::terminal_hyperlinks::HyperlinkLine;
use crate::terminal_hyperlinks::TerminalHyperlink;
use ratatui::style::Modifier;
const MAX_DEPTH: usize = 16;
const MAX_NODES: usize = 256;
const MAX_TEXT_BYTES: usize = 16 * 1024;
struct Budget {
nodes: usize,
text_bytes: usize,
}
impl UiNode {
impl UiTree {
/// Render one row. On overflow, fit an unlinked ellipsis inside the supplied width.
pub(crate) fn render_line(&self, width: u16) -> HyperlinkLine {
let width = usize::from(width);
if width == 0 {
let Some(root) = self.0.as_ref().filter(|_| width > 0) else {
return HyperlinkLine::default();
}
let mut budget = Budget {
nodes: MAX_NODES,
text_bytes: MAX_TEXT_BYTES,
};
let mut line = node_line(Some(self), /*depth*/ 0, &mut budget).unwrap_or_default();
let mut line = node_line(root, width);
if line.width() > width {
line.line = truncate_line_to_width(line.line, width - 1);
line.clip_hyperlinks_to_width(line.width());
@@ -38,30 +26,18 @@ impl UiNode {
}
}
fn node_line(node: Option<&UiNode>, depth: usize, budget: &mut Budget) -> Option<HyperlinkLine> {
if budget.nodes == 0 {
return None;
}
budget.nodes -= 1;
if depth > MAX_DEPTH {
return None;
}
match node? {
UiNode::Text { props, .. } => {
let raw = &props.text;
if budget.text_bytes == 0 && !raw.is_empty() {
return None;
}
let end = raw.floor_char_boundary(raw.len().min(budget.text_bytes));
let first = raw[..end].split('\n').next().unwrap_or_default();
budget.text_bytes -= first.len();
let visible = sanitize_user_text(first.into()).replace('\t', " ");
Some(visible.into())
fn node_line(node: &UiNode, width: usize) -> HyperlinkLine {
match node {
UiNode::String(raw) => {
// Built-in values are constructed directly, bypassing the wire decoder.
let raw = &raw[..raw.floor_char_boundary(MAX_TEXT_BYTES)];
let first = raw.split('\n').next().unwrap_or_default();
sanitize_user_text(first.into())
.replace('\t', " ")
.into()
}
UiNode::Link {
props, children, ..
} => {
let mut line = row_line(children, /*gap*/ 0, depth, budget)?;
UiNode::Link { props, children } => {
let mut line = row_line(children, /*gap*/ 0, width);
for span in &mut line.line.spans {
span.style = span.style.add_modifier(Modifier::UNDERLINED);
}
@@ -69,48 +45,40 @@ fn node_line(node: Option<&UiNode>, depth: usize, budget: &mut Budget) -> Option
if width > 0 {
line.hyperlinks = vec![TerminalHyperlink::web(0..width, props.href.0.clone())];
}
Some(line)
line
}
UiNode::Row {
props, children, ..
} => {
// A spacing unit is four points; a terminal cell is eight points across.
let gap = (props.gap.unwrap_or(0.0).clamp(0.0, 32.0) / 2.0).ceil() as usize;
row_line(children, gap, depth, budget)
}
UiNode::Spacer { .. } => Some(" ".into()),
UiNode::Divider { .. } => Some("│".into()),
UiNode::Row { props, children } => row_line(children, props.gap, width),
UiNode::Text { children, .. } => row_line(children, /*gap*/ 0, width),
}
}
fn row_line(
children: &[UiTree],
gap: usize,
depth: usize,
budget: &mut Budget,
) -> Option<HyperlinkLine> {
let mut result: Option<HyperlinkLine> = None;
fn row_line(children: &[Option<UiNode>], gap: u64, width: usize) -> HyperlinkLine {
let mut result = HyperlinkLine::default();
let mut used = 0;
for child in children {
if budget.nodes == 0 {
break;
}
let Some(child) = node_line(child.as_ref(), depth + 1, budget) else {
for child in children.iter().flatten() {
let child = node_line(child, width);
let child_width = child.width();
if child_width == 0 {
continue;
};
let spacing = if result.is_some() { gap } else { 0 };
let line = result.get_or_insert_default();
if spacing > 0 {
line.line.push_span(" ".repeat(spacing));
}
let offset = used + spacing;
used = offset + child.width();
line.line.spans.extend(child.line.spans);
line.hyperlinks
if used > 0 && gap > 0 {
// One column past the viewport is sufficient to detect overflow.
let spacing = gap.min((width + 1 - used) as u64) as usize;
result.line.push_span(" ".repeat(spacing));
used += spacing;
}
let offset = used;
result.line.spans.extend(child.line.spans);
result
.hyperlinks
.extend(child.hyperlinks.into_iter().map(|mut link| {
link.columns = link.columns.start + offset..link.columns.end + offset;
link
}));
used += child_width;
if used > width {
break;
}
}
result
}
+75 -32
View File
@@ -4,45 +4,83 @@ use super::*;
use pretty_assertions::assert_eq;
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use serde_json::Value;
use serde_json::json;
fn tree(value: serde_json::Value) -> UiNode {
serde_json::from_value::<UiTree>(value).unwrap().unwrap()
fn parse(value: Value) -> serde_json::Result<UiTree> {
value.to_string().parse()
}
fn text(key: &str, value: &str) -> serde_json::Value {
json!({"type":"text","key":key,"props":{"text":value}})
fn tree(value: Value) -> UiTree {
parse(value).unwrap()
}
#[test]
fn normalized_tree_round_trips_and_enforces_leaf_shape() {
let value = json!({"type":"row","key":7,"props":{"gap":2.0},"children":[{"type":"link","props":{"href":"https://example.com/pr/42"},"children":[text("usage", "12%")]}, null, {"type":"spacer"}]});
fn elements_round_trip_and_require_explicit_row_spacing() {
let value = json!({"type":"row","props":{"gap":1},"children":[{"type":"link","props":{"href":"https://example.com/pr/42","title":"PR details"},"children":["12%"]}, null, " "]});
assert_eq!(serde_json::to_value(tree(value.clone())).unwrap(), value);
assert!(
serde_json::from_value::<UiTree>(
json!({"type":"text","props":{"text":"hello"},"children":[]})
)
.is_err()
);
assert_eq!(serde_json::from_value::<UiTree>(json!(null)).unwrap(), None);
for href in [
"relative/path",
"file:///tmp/example",
"https://example.com/\u{001b}",
] {
assert!(
serde_json::from_value::<UiTree>(
json!({"type":"link","props":{"href":href},"children":[]})
)
.is_err()
);
assert!(parse(json!({"type":"link","props":{"href":href},"children":[]})).is_err());
}
for (gap, expected) in [(0, "AB"), (1, "A B"), (2, "A B"), (MAX_GAP, "A …")] {
let row = tree(json!({"type":"row","props":{"gap":gap},"children":["A","B"]}));
assert_eq!(row.render_line(/*width*/ 4).line.to_string(), expected);
}
}
#[test]
fn rejects_invalid_gaps_and_elements() {
for gap in ["-1", "1.5", "9007199254740992", r#""1""#, "[1]"] {
let row = format!(r#"{{"children":[],"props":{{"gap":{gap}}},"type":"row"}}"#);
assert!(row.parse::<UiTree>().is_err(), "{row}");
}
for input in [
"[]",
"null true",
r#"{"type":{"text":null},"children":[]}"#,
r#"{"type":"text","children":[["Ready"]]}"#,
r#"{"type":"text","props":null,"children":[]}"#,
r#"{"type":"row","props":{},"children":[]}"#,
r#"{"type":"row","props":{"gap":0,"extra":true},"children":[]}"#,
r#"{"props":{"gap":0,"gap":1},"children":[],"type":"row"}"#,
] {
assert!(input.parse::<UiTree>().is_err(), "{input}");
}
}
#[test]
fn deserialization_checks_limits_across_the_whole_tree() {
let parent = |children: Vec<Value>| json!({"type":"text","children":children});
assert!(parse(parent(vec![Value::Null; 257])).is_ok());
let nested = (0..16).fold(json!("X"), |child, _| parent(vec![child]));
assert!(parse(nested.clone()).is_ok());
assert!(parse(parent(vec![nested])).is_err());
let mut leaves = vec![json!(""); 255];
assert!(parse(parent(leaves.clone())).is_ok());
leaves.push(json!(""));
assert!(parse(parent(leaves)).is_err());
let text = "界".repeat(5461); // 16,383 UTF-8 bytes.
for (suffix, ok) in [("x", true), ("xx", false)] {
assert_eq!(parse(parent(vec![json!(text), json!(suffix)])).is_ok(), ok);
}
let at_limit = format!("null{}", " ".repeat(64 * 1024 - 4));
assert_eq!(at_limit.parse::<UiTree>().unwrap(), UiTree(None));
assert!(format!("{at_limit} ").parse::<UiTree>().is_err());
let prefix = "https://example.com/";
let href = format!("{prefix}{}", "x".repeat(8192 - prefix.len()));
let link = |url: &str| json!({"type":"link","props":{"href":url},"children":["ok"]});
assert!(parse(link(&href)).is_ok());
assert!(parse(link(&format!("{href}x"))).is_err());
}
#[test]
fn links_only_annotate_their_own_cells() {
let tree = tree(
json!({"type":"row","props":{"gap":2},"children":[text("prefix", "界"),{"type":"link","props":{"href":"https://example.com/pr/42"},"children":[{"type":"row","props":{},"children":[text("pr", "PR #42")]}]}]}),
json!({"type":"row","props":{"gap":1},"children":["界",{"type":"link","props":{"href":"https://example.com/pr/42"},"children":[{"type":"text","children":["PR #42"]}]}]}),
);
let area = Rect::new(
/*x*/ 0, /*y*/ 0, /*width*/ 16, /*height*/ 1,
@@ -75,32 +113,30 @@ fn links_only_annotate_their_own_cells() {
}
#[test]
fn extension_status_line_keeps_exactly_fitting_halfwidth_voiced_katakana() {
let status = tree(text("name", "ガ"));
fn render_line_keeps_exactly_fitting_halfwidth_voiced_katakana() {
let text = tree(json!("ガ"));
let area = Rect::new(
/*x*/ 0, /*y*/ 0, /*width*/ 2, /*height*/ 1,
);
let mut buffer = Buffer::empty(area);
status
.render_line(area.width)
text.render_line(area.width)
.render_fitted(area, &mut buffer);
assert_eq!(buffer[(0, 0)].symbol(), "ガ");
}
#[test]
fn ellipsis_counts_toward_width_only_when_there_is_overflow() {
let status =
tree(json!({"type":"row","props":{"gap":2},"children":[text("label","abcde"), null]}));
let row = tree(json!({"type":"row","props":{"gap":1},"children":["abcde", null]}));
for (width, expected) in [(0, ""), (1, "…"), (4, "abc…"), (5, "abcde")] {
let line = status.render_line(width);
let line = row.render_line(width);
assert_eq!(line.line.to_string(), expected);
assert!(line.width() <= usize::from(width));
}
let wide = tree(text("label", "a界界"));
let wide = tree(json!("a界界"));
assert_eq!(wide.render_line(/*width*/ 4).line.to_string(), "a界…");
let long = "x".repeat(513);
assert_eq!(
tree(text("label", &long))
tree(json!(&long))
.render_line(/*width*/ 513)
.line
.to_string(),
@@ -110,9 +146,16 @@ fn ellipsis_counts_toward_width_only_when_there_is_overflow() {
#[test]
fn first_line_skips_hidden_text_before_rendering_its_sibling() {
let multiline = format!("\u{1b}[2Jok\n{}", "x".repeat(16 * 1024));
let tree = tree(json!({"type":"row","props":{},"children":[
text("first", &multiline), text("sibling", "!")
let multiline = format!("\u{1b}[2Jok\n{}", "x".repeat(1024));
let tree = tree(json!({"type":"row","props":{"gap":0},"children":[
&multiline, "!"
]}));
assert_eq!(tree.render_line(/*width*/ 4).line.to_string(), "ok!");
}
#[test]
fn directly_constructed_text_keeps_a_bounded_utf8_prefix() {
let prefix = "x".repeat(MAX_TEXT_BYTES - 1);
let text = UiTree(Some(UiNode::String(format!("{prefix}界tail"))));
assert_eq!(text.render_line(u16::MAX).line.to_string(), prefix);
}