Skip to main content

max / makeover-webview

0.41.0: a column name cannot break out of the class it names `push_column_class` wrote the name into `class="..."` raw. A column named a" onclick="steal() emitted `<div class="cell col-a" onclick="steal() cell-fill cell-keeps">` on every cell of that column, which is a live event handler built from a string the app supplied. It is the one app-supplied value this crate puts in a class rather than in text or an aria-label, and it was the one that went in unchecked -- form.rs escapes its name through `id_for`, and the figure, meter and placeholder emitters escape everything they take. Fixed by reducing the name to identifier characters, not by escaping it. A class is read twice: by the HTML parser, which would decode `&quot;` back to a quote, and by a CSS selector, which `narrowing_css` writes from this same function. Escaping makes the attribute safe and the selector unmatchable, so the two halves of the narrowing would stop meeting -- silently, which is the failure mode every other comment in this module records. This also fixes a plain bug for names nobody thought of as hostile. `Due date` emitted `col-Due date`: two classes to the parser, and a descendant selector out of `narrowing_css` matching neither. Both sides now agree on `col-Due-date`. Alphanumeric in the Unicode sense, so `Größe` keeps its name rather than folding to `Gr--e` and colliding with a neighbour. Substituted rather than dropped, so `a b` and `ab` stay different columns. Minor rather than patch: the emitted class changes for any name that was not already an identifier. Nothing in the tree has one -- every column across quasi-webview and its tests is alphanumerics, `_` and `-` -- so this is a fix in practice and a rename only in principle.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-15 15:23 UTC
Signed with PGP, not checked
Commit: f47e46171022748af6ff682fafbd710fafb972e7
Parent: 1d9e4ca
2 files changed, +121 insertions, -2 deletions
M Cargo.toml +1 -1
@@ -1,6 +1,6 @@
1 1 [package]
2 2 name = "makeover-webview"
3 - version = "0.40.0"
3 + version = "0.41.0"
4 4 edition = "2024"
5 5 # One copy of this renderer per dependency graph, enforced by cargo rather than
6 6 # by remembering. Two versions means the generated stylesheet and the emitted
M src/list.rs +120 -1
@@ -91,6 +91,8 @@
91 91 /// Derived from the column's own name, which is what makes the narrowing rules
92 92 /// addressable. `data-column` would do as well; a class is what both webview
93 93 /// apps already key their cell styling on.
94 + ///
95 + /// The name is reduced to identifier characters first. See [`push_column_name`].
94 96 #[must_use]
95 97 pub fn column_class(column: &Column<'_>, opts: &Emit) -> String {
96 98 let mut out = String::new();
@@ -107,7 +109,57 @@
107 109 pub fn push_column_class(out: &mut String, column: &Column<'_>, opts: &Emit) {
108 110 out.push_str(opts.class_prefix);
109 111 out.push_str("col-");
110 - out.push_str(column.name);
112 + push_column_name(out, column.name);
113 + }
114 +
115 + /// A column's name as the identifier half of its class.
116 + ///
117 + /// # Why this is not escaping
118 + ///
119 + /// The name is the one app-supplied string this crate puts in a class attribute
120 + /// rather than in text or an `aria-label`, and until 0.41.0 it went in raw. A
121 + /// column named `a" onclick="steal()` emitted
122 + ///
123 + /// ```html
124 + /// <div class="cell col-a" onclick="steal() cell-fill cell-keeps">
125 + /// ```
126 + ///
127 + /// which is a live event handler on every cell of that column. HTML escaping is
128 + /// the reflex and it is the wrong tool here, because a class is read twice: once
129 + /// by the HTML parser, which would decode `&quot;` back to a quote, and once by
130 + /// a CSS selector, which [`narrowing_css`] writes from this same function. An
131 + /// escaped name is safe in the attribute and unmatchable from the stylesheet,
132 + /// so the two halves of the narrowing would stop meeting -- silently, the way
133 + /// every other defect this module's comments record did.
134 + ///
135 + /// Reducing the name to identifier characters answers both. What comes out is a
136 + /// valid CSS identifier, so the selector matches, and it holds none of the five
137 + /// characters an attribute value can be ended with, so there is nothing to
138 + /// escape.
139 + ///
140 + /// # What it changes for a name that was already fine
141 + ///
142 + /// Nothing. Alphanumerics, `_` and `-` pass through, and every column name in
143 + /// the tree is made of those. A name that is *not* was already broken rather
144 + /// than merely unsafe: `Due date` emitted `col-Due date`, which the HTML parser
145 + /// reads as the two classes `col-Due` and `date`, and which `narrowing_css`
146 + /// wrote as a descendant selector that matched neither. Both now agree on
147 + /// `col-Due-date`.
148 + ///
149 + /// Alphanumeric in the Unicode sense, not the ASCII one. CSS identifiers admit
150 + /// everything from U+00A0 up, so a column named `Größe` keeps its name; folding
151 + /// it to `Gr--e` would collide with a neighbouring column for nothing.
152 + pub fn push_column_name(out: &mut String, name: &str) {
153 + for ch in name.chars() {
154 + // Substituted rather than dropped. Two columns called `a b` and `ab`
155 + // are different columns, and dropping would give them one class and one
156 + // set of narrowing rules between them.
157 + if ch.is_alphanumeric() || ch == '_' || ch == '-' {
158 + out.push(ch);
159 + } else {
160 + out.push('-');
161 + }
162 + }
111 163 }
112 164
113 165 /// The class saying how wide a cell of this column asks to be.
@@ -407,6 +459,73 @@
407 459 mod tests {
408 460 use super::*;
409 461
462 + #[test]
463 + fn a_column_name_cannot_break_out_of_the_class_attribute() {
464 + // Until 0.41.0 the name went in raw, so this emitted
465 + // `class="cell col-a" onclick="steal() cell-fill ...">` -- a live
466 + // handler on every cell of the column. The name is the one
467 + // app-supplied string this crate puts in a class rather than in text.
468 + let name = "a\" onclick=\"steal()";
469 + let columns = vec![Column::new(name)];
470 + let cells = vec![Cell {
471 + column: name,
472 + part: None,
473 + content: Markup("x"),
474 + }];
475 + let html = cells_html(&columns, &cells, &Emit::default());
476 +
477 + assert!(!html.contains("onclick=\"steal()"), "{html}");
478 + assert!(html.contains("col-a--onclick--steal--"), "{html}");
479 + // Two quotes in the whole cell, both this crate's: the ones opening and
480 + // closing the class attribute. A third would be the name ending it.
481 + assert_eq!(html.matches('"').count(), 2, "{html}");
482 + }
483 +
484 + #[test]
485 + fn the_class_and_the_selector_that_hides_it_agree_on_the_name() {
486 + // The reason the fix is a filter and not an escape. A class is read by
487 + // the HTML parser and again by a CSS selector; an escaped name would be
488 + // safe in the attribute and unmatchable from the stylesheet, so the
489 + // narrowing would stop hiding the column it names.
490 + let columns = vec![Column {
491 + priority: Priority::Optional,
492 + ..Column::new("Due date")
493 + }];
494 + let cells = vec![Cell {
495 + column: "Due date",
496 + part: None,
497 + content: Markup("x"),
498 + }];
499 + let opts = Emit::default();
500 +
501 + let html = cells_html(&columns, &cells, &opts);
502 + let css = narrowing_css(&columns, ".row", &sizing(), Priority::Essential, &opts);
503 +
504 + // One class, not the two `col-Due date` parsed as.
505 + assert!(html.contains("class=\"cell col-Due-date "), "{html}");
506 + assert!(css.contains(".row > .col-Due-date {"), "{css}");
507 + }
508 +
509 + #[test]
510 + fn a_name_already_made_of_identifier_characters_is_untouched() {
511 + // Every column name in the tree is one of these, which is what makes
512 + // 0.41.0 a fix rather than a rename.
513 + for name in ["description", "due", "progress", "Name", "col_2", "a-b"] {
514 + let mut out = String::new();
515 + push_column_name(&mut out, name);
516 + assert_eq!(out, name);
517 + }
518 + }
519 +
520 + #[test]
521 + fn a_name_outside_ascii_keeps_itself() {
522 + // CSS identifiers admit everything from U+00A0 up, so folding these to
523 + // dashes would collide two columns for nothing.
524 + let mut out = String::new();
525 + push_column_name(&mut out, "Größe");
526 + assert_eq!(out, "Größe");
527 + }
528 +
410 529 fn columns() -> Vec<Column<'static>> {
411 530 vec![
412 531 Column {