diff --git a/CHANGELOG.md b/CHANGELOG.md index b50f039..e8cab9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- `DistantManagerRouter` no longer silently fails when `distant.args` is + provided that includes double quotes within it + +### Changed + +- `Map` implementation of `Display` now escapes `\` and `"` +- `Map` implementation of `FromStr` now handles escaped `\` and `"` + ## [0.17.1] - 2022-08-16 ### Added diff --git a/distant-core/src/data/map.rs b/distant-core/src/data/map.rs index 41fc9ed..2d55185 100644 --- a/distant-core/src/data/map.rs +++ b/distant-core/src/data/map.rs @@ -1,5 +1,5 @@ use crate::serde_str::{deserialize_from_str, serialize_to_str}; -use derive_more::{From, IntoIterator}; +use derive_more::{Display, Error, From, IntoIterator}; use serde::{de::Deserializer, ser::Serializer, Deserialize, Serialize}; use std::{ collections::HashMap, @@ -55,7 +55,7 @@ impl fmt::Display for Map { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let len = self.0.len(); for (i, (key, value)) in self.0.iter().enumerate() { - write!(f, "{}=\"{}\"", key, value)?; + write!(f, "{}=\"{}\"", key, encode_value(value))?; // Include a comma after each but the last pair if i + 1 < len { @@ -66,8 +66,20 @@ impl fmt::Display for Map { } } +#[derive(Clone, Debug, Display, Error)] +pub enum MapParseError { + #[display(fmt = "Missing = after key ('{}')", key)] + MissingEqualsAfterKey { key: String }, + + #[display(fmt = "Key ('{}') must start with alphabetic character", key)] + KeyMustStartWithAlphabeticCharacter { key: String }, + + #[display(fmt = "Missing closing \" for value")] + MissingClosingQuoteForValue, +} + impl FromStr for Map { - type Err = &'static str; + type Err = MapParseError; /// Parses a series of `key=value` pairs in the form `key="value",key2=value2` where /// the quotes around the value are optional @@ -77,13 +89,17 @@ impl FromStr for Map { let mut s = s.trim(); while !s.is_empty() { // Find {key}={tail...} where tail is everything after = - let (key, tail) = s.split_once('=').ok_or("Missing = after key")?; + let (key, tail) = s + .split_once('=') + .ok_or_else(|| MapParseError::MissingEqualsAfterKey { key: s.to_string() })?; // Remove whitespace around the key and ensure it starts with a proper character let key = key.trim(); if !key.starts_with(char::is_alphabetic) { - return Err("Key must start with alphabetic character"); + return Err(MapParseError::KeyMustStartWithAlphabeticCharacter { + key: key.to_string(), + }); } // Remove any map whitespace at the front of the tail @@ -93,14 +109,41 @@ impl FromStr for Map { let (value, tail) = match tail.strip_prefix('"') { // If quoted, we maintain the whitespace within the quotes Some(tail) => { - // Skip the quote so we can look for the trailing quote - let (value, tail) = - tail.split_once('"').ok_or("Missing closing \" for value")?; - - // Skip comma if we have one - let tail = tail.strip_prefix(',').unwrap_or(tail); - - (value, tail) + let mut backslash_cnt: usize = 0; + let mut split_idx = None; + for (i, b) in tail.bytes().enumerate() { + // If we get a backslash, increment our count + if b == b'\\' { + backslash_cnt += 1; + + // If we get a quote and have an even number of preceding backslashes, + // this is considered a closing quote and we've found our split index + } else if b == b'"' && backslash_cnt % 2 == 0 { + split_idx = Some(i); + break; + + // Otherwise, we've encountered some other character, so reset our backlash + // count + } else { + backslash_cnt = 0; + } + } + + match split_idx { + Some(i) => { + // Splitting at idx will result in the double quote being at the + // beginning of the tail str, so we want to have the tail start + // one after the beginning of the slice + let (value, tail) = tail.split_at(i); + let tail = &tail[1..].trim_start(); + + // Also remove a trailing comma if it exists + let tail = tail.strip_prefix(',').unwrap_or(tail).trim_start(); + + (value, tail) + } + None => return Err(MapParseError::MissingClosingQuoteForValue), + } } // If not quoted, we remove all whitespace around the value @@ -111,7 +154,7 @@ impl FromStr for Map { }; // Insert our new pair and update the slice to be the tail (removing whitespace) - map.insert(key.to_string(), value.to_string()); + map.insert(key.to_string(), decode_value(value)); s = tail.trim(); } @@ -119,6 +162,24 @@ impl FromStr for Map { } } +/// Escapes double-quotes of a value str +/// * `\` -> `\\` +/// * `"` -> `\"` +#[inline] +fn encode_value(value: &str) -> String { + // \ -> \\ + // " -> \" + value.replace('\\', "\\\\").replace('"', "\\\"") +} + +/// Translates escaped double-quotes back into quotes +/// * `\\` -> `\` +/// * `\"` -> `"` +#[inline] +fn decode_value(value: &str) -> String { + value.replace("\\\\", "\\").replace("\\\"", "\"") +} + impl Serialize for Map { fn serialize(&self, serializer: S) -> Result where @@ -139,7 +200,7 @@ impl<'de> Deserialize<'de> for Map { #[macro_export] macro_rules! map { - ($($key:literal -> $value:literal),*) => {{ + ($($key:literal -> $value:literal),* $(,)?) => {{ let mut _map = ::std::collections::HashMap::new(); $( @@ -211,6 +272,18 @@ mod tests { let map = "key=value key2=value2".parse::().unwrap(); assert_eq!(map, map!("key" -> "value key2=value2")); + // Support escaped quotes within value + let map = r#"key="\"va\"lue\"""#.parse::().unwrap(); + assert_eq!(map, map!("key" -> r#""va"lue""#)); + + // Support escaped backslashes within value + let map = r#"key="a\\b\\c""#.parse::().unwrap(); + assert_eq!(map, map!("key" -> r"a\b\c")); + + // Backslashes and double quotes in a value are escaped together + let map = r#"key="\"\\\"\\\"""#.parse::().unwrap(); + assert_eq!(map, map!("key" -> r#""\"\""#)); + // Variety of edge cases that should fail let _ = ",".parse::().unwrap_err(); let _ = ",key=value".parse::().unwrap_err(); @@ -225,6 +298,18 @@ mod tests { let map = map!("key" -> "value").to_string(); assert_eq!(map, r#"key="value""#); + // Double quotes in a value are escaped + let map = map!("key" -> r#""va"lue""#).to_string(); + assert_eq!(map, r#"key="\"va\"lue\"""#); + + // Backslashes in a value are also escaped + let map = map!("key" -> r"a\b\c").to_string(); + assert_eq!(map, r#"key="a\\b\\c""#); + + // Backslashes and double quotes in a value are escaped together + let map = map!("key" -> r#""\"\""#).to_string(); + assert_eq!(map, r#"key="\"\\\"\\\"""#); + // Order of key=value output is not guaranteed let map = map!("key" -> "value", "key2" -> "value2").to_string(); assert!(