Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

more string quoting #398

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions README.md
Expand Up @@ -41,8 +41,9 @@ fn main() -> Result<(), serde_yaml::Error> {
map.insert("y".to_string(), 2.0);

// Serialize it to a YAML string.
// y is quoted to avoid ambiguity in parsers that might read it as `true`.
let yaml = serde_yaml::to_string(&map)?;
assert_eq!(yaml, "x: 1.0\ny: 2.0\n");
assert_eq!(yaml, "x: 1.0\n'y': 2.0\n");

// Deserialize it back to a Rust type.
let deserialized_map: BTreeMap<String, f64> = serde_yaml::from_str(&yaml)?;
Expand Down Expand Up @@ -75,7 +76,7 @@ fn main() -> Result<(), serde_yaml::Error> {
let point = Point { x: 1.0, y: 2.0 };

let yaml = serde_yaml::to_string(&point)?;
assert_eq!(yaml, "x: 1.0\ny: 2.0\n");
assert_eq!(yaml, "x: 1.0\n'y': 2.0\n");

let deserialized_point: Point = serde_yaml::from_str(&yaml)?;
assert_eq!(point, deserialized_point);
Expand Down
29 changes: 29 additions & 0 deletions src/de.rs
Expand Up @@ -1096,6 +1096,35 @@ pub(crate) fn digits_but_not_number(scalar: &str) -> bool {
scalar.len() > 1 && scalar.starts_with('0') && scalar[1..].bytes().all(|b| b.is_ascii_digit())
}

/// If a string looks like it could be parsed as some other type by some YAML
/// parser on the round trip, or could otherwise be ambiguous, then we should
/// serialize it with quotes to be safe.
/// This avoids the norway problem https://hitchdev.com/strictyaml/why/implicit-typing-removed/
pub(crate) fn ambiguous_string(scalar: &str) -> bool {
let lower_scalar = scalar.to_lowercase();
parse_bool(&lower_scalar).is_some()
|| parse_null(&lower_scalar.as_bytes()).is_some()
|| lower_scalar.len() == 0
|| lower_scalar.bytes().nth(0).unwrap().is_ascii_digit()
|| lower_scalar.starts_with('-')
|| lower_scalar.starts_with('.')
|| lower_scalar.starts_with("+")
// Things that we don't parse as bool but could be parsed as bool by
// other YAML parsers.
|| lower_scalar == "y"
|| lower_scalar == "yes"
|| lower_scalar == "n"
|| lower_scalar == "no"
|| lower_scalar == "on"
|| lower_scalar == "off"
|| lower_scalar == "true"
|| lower_scalar == "false"
|| lower_scalar == "null"
|| lower_scalar == "nil"
|| lower_scalar == "~"
|| lower_scalar == "nan"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if this would also cover the float or string issue: https://hitchdev.com/strictyaml/why/implicit-typing-removed/#string-or-float

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fzgregor tests added

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

}

pub(crate) fn visit_int<'de, V>(visitor: V, v: &str) -> Result<Result<V::Value>, V>
where
V: Visitor<'de>,
Expand Down
5 changes: 3 additions & 2 deletions src/lib.rs
Expand Up @@ -24,8 +24,9 @@
//! map.insert("y".to_string(), 2.0);
//!
//! // Serialize it to a YAML string.
//! // 'y' is quoted to avoid ambiguity in parsers that might read it as `true`.
//! let yaml = serde_yaml::to_string(&map)?;
//! assert_eq!(yaml, "x: 1.0\ny: 2.0\n");
//! assert_eq!(yaml, "x: 1.0\n'y': 2.0\n");
//!
//! // Deserialize it back to a Rust type.
//! let deserialized_map: BTreeMap<String, f64> = serde_yaml::from_str(&yaml)?;
Expand Down Expand Up @@ -55,7 +56,7 @@
//! let point = Point { x: 1.0, y: 2.0 };
//!
//! let yaml = serde_yaml::to_string(&point)?;
//! assert_eq!(yaml, "x: 1.0\ny: 2.0\n");
//! assert_eq!(yaml, "x: 1.0\n'y': 2.0\n");
//!
//! let deserialized_point: Point = serde_yaml::from_str(&yaml)?;
//! assert_eq!(point, deserialized_point);
Expand Down
8 changes: 4 additions & 4 deletions src/ser.rs
Expand Up @@ -340,11 +340,11 @@ where
}

fn visit_str<E>(self, v: &str) -> Result<Self::Value, E> {
Ok(if crate::de::digits_but_not_number(v) {
ScalarStyle::SingleQuoted
if crate::de::ambiguous_string(v) {
Ok(ScalarStyle::SingleQuoted)
} else {
ScalarStyle::Any
})
Ok(ScalarStyle::Any)
}
}

fn visit_unit<E>(self) -> Result<Self::Value, E> {
Expand Down
60 changes: 58 additions & 2 deletions tests/test_serde.rs
Expand Up @@ -195,7 +195,7 @@ fn test_map() {
thing.insert("y".to_owned(), 2);
let yaml = indoc! {"
x: 1
y: 2
'y': 2
"};
test_serde(&thing, yaml);
}
Expand Down Expand Up @@ -238,7 +238,7 @@ fn test_basic_struct() {
};
let yaml = indoc! {r#"
x: -4
y: "hi\tquoted"
'y': "hi\tquoted"
z: true
"#};
test_serde(&thing, yaml);
Expand Down Expand Up @@ -316,6 +316,62 @@ fn test_strings_needing_quote() {
test_serde(&thing, yaml);
}

#[test]
fn test_moar_strings_needing_quote() {
#[derive(Serialize, Deserialize, PartialEq, Debug)]
struct Struct {
s: String,
}

for s in &[
// Short hex values.
"0x0",
"0x1",
// Long hex values that don't fit in a u64 need to be quoted.
"0xffaed20B7B67e498A3bEEf97386ec1849EFeE6Ac",
// "empty" strings.
"",
" ",
// The norway problem https://hitchdev.com/strictyaml/why/implicit-typing-removed/
"NO",
"no",
"No",
"Yes",
"YES",
"yes",
"True",
"TRUE",
"true",
"False",
"FALSE",
"false",
"y",
"Y",
"n",
"N",
"on",
"On",
"ON",
"off",
"Off",
"OFF",
"0",
"1",
"null",
"Null",
"NULL",
"nil",
"Nil",
"NIL",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test string for float or string would be 2E234567

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fzgregor i added this

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

] {
let thing = Struct {
s: s.to_string(),
};
let yaml = format!("s: '{}'\n", s);
test_serde(&thing, &yaml);
}
}

#[test]
fn test_nested_vec() {
let thing = vec![vec![1, 2, 3], vec![4, 5, 6]];
Expand Down