Fix serde-rs#1495: Do not buffer content of the internally tagged enu…

…ms if tag is the first field

serde/src/de/value.rs

@@ -1439,9 +1439,21 @@ where
         visitor.visit_enum(self)
     }
 
+    fn deserialize_newtype_struct<V>(
+        self,
+        _name: &'static str,
+        visitor: V,
+    ) -> Result<V::Value, Self::Error>
+    where
+        V: de::Visitor<'de>,
+    {
+        // For test_internally_tagged_enum
+        visitor.visit_newtype_struct(self)
+    }
+
     forward_to_deserialize_any! {
         bool i8 i16 i32 i64 i128 u8 u16 u32 u64 u128 f32 f64 char str string
-        bytes byte_buf option unit unit_struct newtype_struct seq tuple
+        bytes byte_buf option unit unit_struct seq tuple
         tuple_struct map struct identifier ignored_any
     }
 }

serde/src/private/de.rs

@@ -220,8 +220,7 @@ mod content {
     /// # Parameters
     /// - `map`: map that will be drained
     /// - `tag_name`: name of tag in the `#[serde(tag = "tag_name")]` attribute
-    /// - `tag`: tag, if first value in map was a tag
-    /// - `vec`: placeholder for all other key-value pairs
+    /// - `first_key`: the first key already fetched from the map
     ///
     /// # Returns
     /// A tuple with two values:
@@ -234,13 +233,16 @@ mod content {
     pub fn drain_map<'de, T, A>(
         mut map: A,
         tag_name: &'static str,
-        mut tag: Option<T>,
-        mut vec: Vec<(Content<'de>, Content<'de>)>,
+        first_key: Content<'de>,
     ) -> Result<(T, ContentDeserializer<'de, A::Error>), A::Error>
     where
         T: Deserialize<'de>,
         A: MapAccess<'de>,
     {
+        let mut tag = None;
+        let mut vec = Vec::with_capacity(size_hint::cautious(map.size_hint()));
+
+        vec.push((first_key, try!(map.next_value())));
         while let Some(key) = try!(map.next_key_seed(TagOrContentVisitor::new(tag_name))) {
             match key {
                 TagOrContent::Tag => {

serde_derive/src/de.rs

@@ -1470,9 +1470,7 @@ fn deserialize_internally_tagged_enum(
             {
                 match try!(_serde::de::SeqAccess::next_element(&mut __seq)) {
                     _serde::__private::Some(__tag) => {
-                        let __rest = _serde::de::value::SeqAccessDeserializer::new(__seq);
-                        let __content = try!(_serde::__private::de::Content::deserialize(__rest));
-                        let __deserializer = _serde::__private::de::ContentDeserializer::<__S::Error>::new(__content);
+                        let __deserializer = _serde::de::value::SeqAccessDeserializer::new(__seq);
 
                         match __tag {
                             #(#variant_arms)*
@@ -1486,28 +1484,25 @@ fn deserialize_internally_tagged_enum(
             where
                 __M: _serde::de::MapAccess<#delife>,
             {
-                let mut __vec = _serde::__private::Vec::with_capacity(
-                    _serde::de::MapAccess::size_hint(&__map).unwrap_or(0)
-                );
-
+                // Read the first field. If it is a tag, immediately deserialize the typed data.
+                // Otherwise, we collect everything until we find the tag, and then deserialize
+                // using ContentDeserializer.
                 match try!(_serde::de::MapAccess::next_key_seed(
                     &mut __map, _serde::__private::de::TagOrContentVisitor::new(#tag)
                 )) {
                     _serde::__private::Some(_serde::__private::de::TagOrContent::Tag) => {
                         let __tag = try!(_serde::de::MapAccess::next_value(&mut __map));
-                        let (__tag, __deserializer) = try!(_serde::__private::de::drain_map(
-                            __map, #tag, _serde::__private::Some(__tag), __vec
-                        ));
+                        let __deserializer = _serde::de::value::MapAccessDeserializer::new(__map);
 
                         match __tag {
                             #(#variant_arms)*
                         }
                     },
                     _serde::__private::Some(_serde::__private::de::TagOrContent::Content(__key)) => {
-                        let __val = try!(_serde::de::MapAccess::next_value(&mut __map));
-                        __vec.push((__key, __val));
+                        // Drain map to Content::Map, convert it to ContentDeserializer
+                        // Special handling for tag key -- search them and return as separate result
                         let (__tag, __deserializer) = try!(_serde::__private::de::drain_map(
-                            __map, #tag, _serde::__private::None, __vec
+                            __map, #tag, __key
                         ));
 
                         match __tag {

serde_test/src/de.rs

@@ -524,12 +524,13 @@ impl<'de, 'a> VariantAccess<'de> for DeserializerEnumVisitor<'a, 'de> {
             Some(self.de.peek_token())
         };
         match token {
+            // See test_internally_tagged_struct_variant_containing_unit_variant
+            Some(Token::SeqEnd) | Some(Token::MapEnd) | Some(Token::StructEnd) | None => Ok(()),
             Some(Token::UnitVariant { .. }) => {
                 self.de.next_token();
                 Ok(())
             }
             Some(_) => Deserialize::deserialize(self.de),
-            None => Ok(()),
         }
     }
 

test_suite/tests/test_macros.rs

@@ -754,8 +754,6 @@ fn test_internally_tagged_enum() {
             Token::Seq { len: Some(2) },
             Token::Str("C"),
             Token::Map { len: Some(0) },
-            Token::MapEnd,
-            Token::SeqEnd,
         ],
         "invalid type: sequence, expected a map",
     );
@@ -785,6 +783,33 @@ fn test_internally_tagged_enum() {
         ],
     );
 
+    // General case: tag field ("type") is not first
+    assert_de_tokens(
+        &InternallyTagged::E(Struct { f: 6 }),
+        &[
+            Token::Struct {
+                name: "Struct",
+                len: 2,
+            },
+            Token::Str("f"),
+            Token::U8(6),
+            Token::Str("type"),
+            Token::Str("E"),
+            Token::StructEnd,
+        ],
+    );
+    assert_de_tokens(
+        &InternallyTagged::E(Struct { f: 6 }),
+        &[
+            Token::Map { len: Some(2) },
+            Token::Str("f"),
+            Token::U8(6),
+            Token::Str("type"),
+            Token::Str("E"),
+            Token::MapEnd,
+        ],
+    );
+
     assert_de_tokens(
         &InternallyTagged::E(Struct { f: 6 }),
         &[
@@ -984,6 +1009,7 @@ fn test_internally_tagged_struct_variant_containing_unit_variant() {
         Log { level: Level },
     }
 
+    // Special case: tag field ("action") is the first field
     assert_de_tokens(
         &Message::Log { level: Level::Info },
         &[
@@ -998,7 +1024,23 @@ fn test_internally_tagged_struct_variant_containing_unit_variant() {
             Token::StructEnd,
         ],
     );
+    // General case: tag field ("action") is not the first field
+    assert_de_tokens(
+        &Message::Log { level: Level::Info },
+        &[
+            Token::Struct {
+                name: "Message",
+                len: 2,
+            },
+            Token::Str("level"),
+            Token::BorrowedStr("Info"),
+            Token::Str("action"),
+            Token::Str("Log"),
+            Token::StructEnd,
+        ],
+    );
 
+    // Special case: tag field ("action") is the first field
     assert_de_tokens(
         &Message::Log { level: Level::Info },
         &[
@@ -1010,6 +1052,18 @@ fn test_internally_tagged_struct_variant_containing_unit_variant() {
             Token::MapEnd,
         ],
     );
+    // General case: tag field ("action") is not the first field
+    assert_de_tokens(
+        &Message::Log { level: Level::Info },
+        &[
+            Token::Map { len: Some(2) },
+            Token::Str("level"),
+            Token::BorrowedStr("Info"),
+            Token::Str("action"),
+            Token::Str("Log"),
+            Token::MapEnd,
+        ],
+    );
 
     assert_de_tokens(
         &Message::Log { level: Level::Info },