max / alloy
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
1 file changed,
+112 insertions,
-1 deletion
| @@ -257,11 +257,24 @@ | |||
| 257 | 257 | /// name for the same reason. `Unknown Unknown Unknown` matches every such | |
| 258 | 258 | /// output at once, which is the one identifier that could apply a stanza to | |
| 259 | 259 | /// hardware it was never written for. | |
| 260 | + | /// | |
| 261 | + | /// A triple carrying a character sway's quoting cannot hold falls back the | |
| 262 | + | /// same way. See [`safely_quotable`]: the make and model come from EDID, | |
| 263 | + | /// which is vendor-supplied text, and [`config_file`] writes the result into | |
| 264 | + | /// a file sway loads at login. Handling it here rather than in [`quote`] is | |
| 265 | + | /// deliberate — every caller of `quote` is handed an identifier this method | |
| 266 | + | /// produced, so refusing to build an unquotable one makes the property | |
| 267 | + | /// structural instead of something each writer has to remember. | |
| 260 | 268 | pub(crate) fn identifier(&self) -> String { | |
| 261 | 269 | if self.built_in() || !self.identifiable() { | |
| 262 | 270 | return self.name.clone(); | |
| 263 | 271 | } | |
| 264 | - | format!("{} {} {}", self.make, self.model, self.serial) | |
| 272 | + | let triple = format!("{} {} {}", self.make, self.model, self.serial); | |
| 273 | + | if safely_quotable(&triple) { | |
| 274 | + | triple | |
| 275 | + | } else { | |
| 276 | + | self.name.clone() | |
| 277 | + | } | |
| 265 | 278 | } | |
| 266 | 279 | ||
| 267 | 280 | /// Whether the triple says anything specific about this output. | |
| @@ -420,6 +433,11 @@ | |||
| 420 | 433 | /// needs this, and quoting it anyway would make the log pane's line noisier than | |
| 421 | 434 | /// the one a person would have typed. | |
| 422 | 435 | fn quote(identifier: &str) -> String { | |
| 436 | + | debug_assert!( | |
| 437 | + | safely_quotable(identifier), | |
| 438 | + | "an unquotable identifier reached quote(); Output::identifier should have \ | |
| 439 | + | fallen back to the connector name" | |
| 440 | + | ); | |
| 423 | 441 | if identifier.contains(char::is_whitespace) { | |
| 424 | 442 | format!("\"{identifier}\"") | |
| 425 | 443 | } else { | |
| @@ -427,6 +445,34 @@ | |||
| 427 | 445 | } | |
| 428 | 446 | } | |
| 429 | 447 | ||
| 448 | + | /// Whether sway's quoting can carry this identifier without being broken by it. | |
| 449 | + | /// | |
| 450 | + | /// [`quote`] wraps in double quotes and escapes nothing, so a `"` inside the | |
| 451 | + | /// string closes it early and the rest of the line becomes stray tokens. That | |
| 452 | + | /// text is not ours: make and model come from EDID, and [`config_file`] writes | |
| 453 | + | /// the line into `~/.config/sway/config.d/50-display.conf`, which | |
| 454 | + | /// `templates/etc/skel/.config/sway/config.in` includes. A malformed line is | |
| 455 | + | /// therefore in the config sway loads at login, not confined to one output's | |
| 456 | + | /// settings. Same class as [`refuse`], which exists because a per-user config | |
| 457 | + | /// survives the reboot that would otherwise recover the session. | |
| 458 | + | /// | |
| 459 | + | /// Three things are refused, and a newline is the worst of them: it would end | |
| 460 | + | /// the directive and start a second one, which is injection rather than | |
| 461 | + | /// corruption. A backslash is refused because whether sway honours escapes | |
| 462 | + | /// inside a quoted string is exactly what cannot be checked from a machine with | |
| 463 | + | /// no sway on it, and a trailing one would eat the closing quote if it does. | |
| 464 | + | /// | |
| 465 | + | /// The alternative fix was to escape rather than refuse, and it was not taken | |
| 466 | + | /// for that reason: it is correct only under an assumption about sway's parser | |
| 467 | + | /// that this file has already been bitten by twice. Falling back costs | |
| 468 | + | /// replug-portability for one monitor and is right whichever way the parser | |
| 469 | + | /// behaves. | |
| 470 | + | fn safely_quotable(identifier: &str) -> bool { | |
| 471 | + | !identifier | |
| 472 | + | .chars() | |
| 473 | + | .any(|c| c == '"' || c == '\\' || c.is_control()) | |
| 474 | + | } | |
| 475 | + | ||
| 430 | 476 | /// Why a change must not be made, or `None` when it may be. | |
| 431 | 477 | /// | |
| 432 | 478 | /// One rule, stated once, checked before the runtime apply and before the write: | |
| @@ -1324,6 +1370,71 @@ | |||
| 1324 | 1370 | assert!(!external("DP-3", "Example Co", "PA279CV", "S1").built_in()); | |
| 1325 | 1371 | } | |
| 1326 | 1372 | ||
| 1373 | + | /// EDID is vendor-supplied text and the identifier it feeds is written into | |
| 1374 | + | /// a file sway loads at login. A make carrying a double quote would close | |
| 1375 | + | /// sway's quoting early and leave a line the compositor cannot parse, so the | |
| 1376 | + | /// triple is abandoned for the connector name instead. | |
| 1377 | + | #[test] | |
| 1378 | + | fn a_quote_in_the_edid_falls_back_to_the_connector() { | |
| 1379 | + | let hostile = external("DP-1", "Ex\"Co", "PA279CV", "S1"); | |
| 1380 | + | assert_eq!(hostile.identifier(), "DP-1"); | |
| 1381 | + | // The bad text must not survive into the line by any route. | |
| 1382 | + | assert!(!hostile.identifier().contains('"')); | |
| 1383 | + | } | |
| 1384 | + | ||
| 1385 | + | /// The worst case is not a broken line but a second directive. A newline in | |
| 1386 | + | /// make or model would end the `output` stanza and start whatever followed | |
| 1387 | + | /// it, which is injection rather than corruption. | |
| 1388 | + | #[test] | |
| 1389 | + | fn a_newline_in_the_edid_cannot_start_a_second_directive() { | |
| 1390 | + | let injected = external("DP-2", "Ex\noutput * scale 3", "PA279CV", "S1"); | |
| 1391 | + | assert_eq!(injected.identifier(), "DP-2"); | |
| 1392 | + | } | |
| 1393 | + | ||
| 1394 | + | /// A backslash is refused because whether sway honours escapes inside a | |
| 1395 | + | /// quoted string cannot be checked from a machine with no sway, and a | |
| 1396 | + | /// trailing one would eat the closing quote if it does. | |
| 1397 | + | #[test] | |
| 1398 | + | fn a_backslash_in_the_edid_falls_back_too() { | |
| 1399 | + | assert_eq!( | |
| 1400 | + | external("DP-3", "Ex\\", "PA279CV", "S1").identifier(), | |
| 1401 | + | "DP-3" | |
| 1402 | + | ); | |
| 1403 | + | } | |
| 1404 | + | ||
| 1405 | + | /// The fallback must stay narrow. Punctuation that cannot break out of the | |
| 1406 | + | /// quoting keeps the triple, or a monitor loses replug-portability for no | |
| 1407 | + | /// reason. | |
| 1408 | + | #[test] | |
| 1409 | + | fn ordinary_punctuation_keeps_the_triple() { | |
| 1410 | + | for make in [ | |
| 1411 | + | "Example Co.", | |
| 1412 | + | "Ex-Co", | |
| 1413 | + | "Ex_Co", | |
| 1414 | + | "Ex+Co (EU)", | |
| 1415 | + | "Ex/Co", | |
| 1416 | + | "Ex#Co", | |
| 1417 | + | ] { | |
| 1418 | + | let output = external("DP-1", make, "PA279CV", "S1"); | |
| 1419 | + | assert_eq!( | |
| 1420 | + | output.identifier(), | |
| 1421 | + | format!("{make} PA279CV S1"), | |
| 1422 | + | "{make} was refused but cannot break sway's quoting" | |
| 1423 | + | ); | |
| 1424 | + | } | |
| 1425 | + | } | |
| 1426 | + | ||
| 1427 | + | #[test] | |
| 1428 | + | fn safely_quotable_names_exactly_the_three_hazards() { | |
| 1429 | + | assert!(safely_quotable("Example Co PA279CV S1")); | |
| 1430 | + | assert!(safely_quotable("eDP-1")); | |
| 1431 | + | assert!(!safely_quotable("Ex\"Co")); | |
| 1432 | + | assert!(!safely_quotable("Ex\\Co")); | |
| 1433 | + | assert!(!safely_quotable("Ex\nCo")); | |
| 1434 | + | assert!(!safely_quotable("Ex\rCo")); | |
| 1435 | + | assert!(!safely_quotable("Ex\tCo"), "a tab is a control character"); | |
| 1436 | + | } | |
| 1437 | + | ||
| 1327 | 1438 | // `Unknown Unknown Unknown` matches every anonymous output at once, which | |
| 1328 | 1439 | // is the one identifier that could apply a stanza to hardware it was never | |
| 1329 | 1440 | // written for. Such an output falls back to its connector name. |