From 7ee1c1b4798f18135ee618e30ccedfdf1f365451 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 7 Jan 2019 09:06:04 -0800 Subject: Fix disallowing duplicate table headers This commit fixes #279 where a case of duplicate table headers slipped through the cracks. This also adds an option to disable this new validation to allow Cargo to preserve backwards compatibility. --- Cargo.toml | 3 +++ src/de.rs | 36 ++++++++++++++++++++++----- test-suite/tests/backcompat.rs | 24 +++++++++++++++++- test-suite/tests/invalid.rs | 2 ++ test-suite/tests/invalid/duplicate-table.toml | 8 ++++++ 5 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 test-suite/tests/invalid/duplicate-table.toml diff --git a/Cargo.toml b/Cargo.toml index 9904e2f..e91d917 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,9 @@ facilitate deserializing and serializing Rust structures. """ categories = ["config", "encoding", "parser-implementations"] +[workspace] +members = ['test-suite'] + [badges] travis-ci = { repository = "alexcrichton/toml-rs" } diff --git a/src/de.rs b/src/de.rs index 5f04027..85aef61 100644 --- a/src/de.rs +++ b/src/de.rs @@ -197,6 +197,7 @@ enum ErrorKind { /// Deserialization implementation for TOML. pub struct Deserializer<'a> { require_newline_after_table: bool, + allow_duplciate_after_longer_table: bool, input: &'a str, tokens: Tokenizer<'a>, } @@ -335,12 +336,24 @@ impl<'de, 'b> de::MapAccess<'de> for MapVisitor<'de, 'b> { // Test to see if we're duplicating our parent's table, and if so // then this is an error in the toml format - if self.cur_parent != pos - && self.tables[self.cur_parent].header == self.tables[pos].header - { - let at = self.tables[pos].at; - let name = self.tables[pos].header.join("."); - return Err(self.de.error(at, ErrorKind::DuplicateTable(name))); + if self.cur_parent != pos { + if self.tables[self.cur_parent].header == self.tables[pos].header { + let at = self.tables[pos].at; + let name = self.tables[pos].header.join("."); + return Err(self.de.error(at, ErrorKind::DuplicateTable(name))); + } + + // If we're here we know we should share the same prefix, and if + // the longer table was defined first then we want to narrow + // down our parent's length if possible to ensure that we catch + // duplicate tables defined afterwards. + if !self.de.allow_duplciate_after_longer_table { + let parent_len = self.tables[self.cur_parent].header.len(); + let cur_len = self.tables[pos].header.len(); + if cur_len < parent_len { + self.cur_parent = pos; + } + } } let table = &mut self.tables[pos]; @@ -965,6 +978,7 @@ impl<'a> Deserializer<'a> { tokens: Tokenizer::new(input), input: input, require_newline_after_table: true, + allow_duplciate_after_longer_table: false, } } @@ -986,6 +1000,16 @@ impl<'a> Deserializer<'a> { self.require_newline_after_table = require; } + /// Historical versions of toml-rs accidentally allowed a duplicate table + /// header after a longer table header was previously defined. This is + /// invalid according to the TOML spec, however. + /// + /// This option can be set to `true` (the default is `false`) to emulate + /// this behavior for backwards compatibility with older toml-rs versions. + pub fn set_allow_duplicate_after_longer_table(&mut self, allow: bool) { + self.allow_duplciate_after_longer_table = allow; + } + fn tables(&mut self) -> Result>, Error> { let mut tables = Vec::new(); let mut cur_table = Table { diff --git a/test-suite/tests/backcompat.rs b/test-suite/tests/backcompat.rs index 1b3f599..e06eefc 100644 --- a/test-suite/tests/backcompat.rs +++ b/test-suite/tests/backcompat.rs @@ -4,7 +4,7 @@ extern crate serde; use serde::de::Deserialize; #[test] -fn main() { +fn newlines_after_tables() { let s = " [a] foo = 1 [[b]] foo = 1 @@ -17,3 +17,25 @@ fn main() { assert_eq!(value["a"]["foo"].as_integer(), Some(1)); assert_eq!(value["b"][0]["foo"].as_integer(), Some(1)); } + +#[test] +fn allow_duplicate_after_longer() { + let s = " + [dependencies.openssl-sys] + version = 1 + + [dependencies] + libc = 1 + + [dependencies] + bitflags = 1 + "; + assert!(s.parse::().is_err()); + + let mut d = toml::de::Deserializer::new(s); + d.set_allow_duplicate_after_longer_table(true); + let value = toml::Value::deserialize(&mut d).unwrap(); + assert_eq!(value["dependencies"]["openssl-sys"]["version"].as_integer(), Some(1)); + assert_eq!(value["dependencies"]["libc"].as_integer(), Some(1)); + assert_eq!(value["dependencies"]["bitflags"].as_integer(), Some(1)); +} diff --git a/test-suite/tests/invalid.rs b/test-suite/tests/invalid.rs index 4679684..9f36e2c 100644 --- a/test-suite/tests/invalid.rs +++ b/test-suite/tests/invalid.rs @@ -96,3 +96,5 @@ test!(text_before_array_separator, include_str!("invalid/text-before-array-separator.toml")); test!(text_in_array, include_str!("invalid/text-in-array.toml")); +test!(duplicate_table, + include_str!("invalid/duplicate-table.toml")); diff --git a/test-suite/tests/invalid/duplicate-table.toml b/test-suite/tests/invalid/duplicate-table.toml new file mode 100644 index 0000000..5bd2571 --- /dev/null +++ b/test-suite/tests/invalid/duplicate-table.toml @@ -0,0 +1,8 @@ +[dependencies.openssl-sys] +version = "0.5.2" + +[dependencies] +libc = "0.1" + +[dependencies] +bitflags = "0.1.1" -- cgit v1.2.3