Merge pull request #280 from alexcrichton/fix-duplicate

Fix disallowing duplicate table headers

5 files changed, 66 insertions, 7 deletions

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<Vec<Table<'a>>, 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::<toml::Value>().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"