Skip to content

Commit 87bcbd9

Browse files
jeckersbcgwalters
authored andcommitted
kernel_cmdline: Fix parsing/equivalence of "outside" quoted args
Primary motivation here is that these two should be equivalent: foo="quoted value" "foo=quoted value" This also adds tests for a few more oddball cases that weren't covered before but clarify the expected kernel behavior. Closes: #1737 Signed-off-by: John Eckersberg <[email protected]>
1 parent 3181bb5 commit 87bcbd9

File tree

2 files changed

+58
-14
lines changed

2 files changed

+58
-14
lines changed

crates/kernel_cmdline/src/bytes.rs

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -421,27 +421,31 @@ impl<'a> Parameter<'a> {
421421
/// This is an internal method that assumes the input has already been
422422
/// split into a single parameter (e.g., by CmdlineIterBytes).
423423
fn parse_internal(input: &'a [u8]) -> Option<Self> {
424-
let equals = input.iter().position(|b| *b == b'=');
424+
// *Only* the first and last double quotes are stripped
425+
let dequoted_input = input.strip_prefix(b"\"").unwrap_or(input);
426+
let dequoted_input = dequoted_input.strip_suffix(b"\"").unwrap_or(dequoted_input);
427+
428+
let equals = dequoted_input.iter().position(|b| *b == b'=');
425429

426430
match equals {
427431
None => Some(Self {
428432
parameter: input,
429-
key: ParameterKey(input),
433+
key: ParameterKey(dequoted_input),
430434
value: None,
431435
}),
432436
Some(i) => {
433-
let (key, mut value) = input.split_at(i);
437+
let (key, mut value) = dequoted_input.split_at(i);
434438
let key = ParameterKey(key);
435439

436440
// skip `=`, we know it's the first byte because we
437441
// found it above
438442
value = &value[1..];
439443

440-
// *Only* the first and last double quotes are stripped
441-
value = {
442-
let v = value.strip_prefix(b"\"").unwrap_or(value);
443-
v.strip_suffix(b"\"").unwrap_or(v)
444-
};
444+
// If there is a quote after the equals, skip it. If
445+
// there was a closing quote at the end of the value,
446+
// we would have already removed it in
447+
// `dequoted_input` above
448+
value = value.strip_prefix(b"\"").unwrap_or(value);
445449

446450
Some(Self {
447451
parameter: input,
@@ -528,6 +532,10 @@ mod tests {
528532

529533
let p = param("foo=trailing_quotes\"");
530534
assert_eq!(p.value, Some(b"trailing_quotes".as_slice()));
535+
536+
let outside_quoted = param("\"foo=quoted value\"");
537+
let value_quoted = param("foo=\"quoted value\"");
538+
assert_eq!(outside_quoted, value_quoted);
531539
}
532540

533541
#[test]
@@ -549,9 +557,22 @@ mod tests {
549557
fn test_parameter_pathological() {
550558
// valid things that certified insane people would do
551559

552-
// quotes don't get removed from keys
553-
let p = param("\"\"\"");
554-
assert_eq!(p.key.0, b"\"\"\"");
560+
// you can quote just the key part in a key-value param, but
561+
// the end quote is actually part of the key as far as the
562+
// kernel is concerned...
563+
let p = param("\"foo\"=bar");
564+
assert_eq!(p.key.0, b"foo\"");
565+
assert_eq!(p.value, Some(b"bar".as_slice()));
566+
// and it is definitely not equal to an unquoted foo ...
567+
assert_ne!(p, param("foo=bar"));
568+
569+
// ... but if you close the quote immediately after the
570+
// equals sign, it does get removed.
571+
let p = param("\"foo=\"bar");
572+
assert_eq!(p.key.0, b"foo");
573+
assert_eq!(p.value, Some(b"bar".as_slice()));
574+
// ... so of course this makes sense ...
575+
assert_eq!(p, param("foo=bar"));
555576

556577
// quotes only get stripped from the absolute ends of values
557578
let p = param("foo=\"internal\"quotes\"are\"ok\"");

crates/kernel_cmdline/src/utf8.rs

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,16 @@ mod tests {
426426
fn test_parameter_quoted() {
427427
let p = param("foo=\"quoted value\"");
428428
assert_eq!(p.value(), Some("quoted value"));
429+
430+
let p = param("foo=\"unclosed quotes");
431+
assert_eq!(p.value(), Some("unclosed quotes"));
432+
433+
let p = param("foo=trailing_quotes\"");
434+
assert_eq!(p.value(), Some("trailing_quotes"));
435+
436+
let outside_quoted = param("\"foo=quoted value\"");
437+
let value_quoted = param("foo=\"quoted value\"");
438+
assert_eq!(outside_quoted, value_quoted);
429439
}
430440

431441
#[test]
@@ -447,9 +457,22 @@ mod tests {
447457
fn test_parameter_pathological() {
448458
// valid things that certified insane people would do
449459

450-
// quotes don't get removed from keys
451-
let p = param("\"\"\"");
452-
assert_eq!(p.key(), "\"\"\"".into());
460+
// you can quote just the key part in a key-value param, but
461+
// the end quote is actually part of the key as far as the
462+
// kernel is concerned...
463+
let p = param("\"foo\"=bar");
464+
assert_eq!(p.key(), ParameterKey::from("foo\""));
465+
assert_eq!(p.value(), Some("bar"));
466+
// and it is definitely not equal to an unquoted foo ...
467+
assert_ne!(p, param("foo=bar"));
468+
469+
// ... but if you close the quote immediately after the
470+
// equals sign, it does get removed.
471+
let p = param("\"foo=\"bar");
472+
assert_eq!(p.key(), ParameterKey::from("foo"));
473+
assert_eq!(p.value(), Some("bar"));
474+
// ... so of course this makes sense ...
475+
assert_eq!(p, param("foo=bar"));
453476

454477
// quotes only get stripped from the absolute ends of values
455478
let p = param("foo=\"internal\"quotes\"are\"ok\"");

0 commit comments

Comments
 (0)