Fix some collection built in serializer bypass null check. #211

This commit is imported from 0.8.1 branch. Some built-in serializers use UnpackFromCore instead of UnpackFrom for items deserialization. This causes bypass of null check, then null items are deserialized as non-null objects.
msgpack · Feb 2, 2017 · d8b65b8 · d8b65b8
1 parent 8862324
commit d8b65b8
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 17 deletions.
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -599,6 +599,11 @@ Release 0.8.0
 
   Nothing from 0.8.0 beta.
 
+Release 0.8.1 2017/02/02
+
+  BUG FIXES
+  * Fix null items of complex type in List<T> or Dictionary<TKey, TValue> will not be deserialized as null. Issue #211.
+
 Release 0.9.0 beta1 2016/09/24
 
   NEW FEATURES
@@ -635,4 +640,5 @@ Release 0.9.0 (planned)
   * Fix a combination of readonly members and collection members incorrect code generation when the type also have deserialization constructor. Issue #207.
   * Fix Windows Native build error. Issue #206.
   * Fix built-in collection serializers such as List<T> serializer causes SecurityException when the program run in restricted environment like Silverlight. Issue #205.
+  * Fix null items of complex type in List<T> or Dictionary<TKey, TValue> will not be deserialized as null. Issue #211. (from 0.8.1)
 
diff --git a/...tion/DefaultSerializers/System_Collections_Generic_Dictionary_2MessagePackSerializer`2.cs b/...tion/DefaultSerializers/System_Collections_Generic_Dictionary_2MessagePackSerializer`2.cs
@@ -103,12 +103,12 @@ private void UnpackToCore( Unpacker unpacker, Dictionary<TKey, TValue> collectio
 				{
 					using ( var subTreeUnpacker = unpacker.ReadSubtree() )
 					{
-						key = this._keySerializer.UnpackFromCore( subTreeUnpacker );
+						key = this._keySerializer.UnpackFrom( subTreeUnpacker );
 					}
 				}
 				else
 				{
-					key = this._keySerializer.UnpackFromCore( unpacker );
+					key = this._keySerializer.UnpackFrom( unpacker );
 				}
 
 				if ( !unpacker.Read() )
@@ -120,12 +120,12 @@ private void UnpackToCore( Unpacker unpacker, Dictionary<TKey, TValue> collectio
 				{
 					using ( var subTreeUnpacker = unpacker.ReadSubtree() )
 					{
-						collection.Add( key, this._valueSerializer.UnpackFromCore( subTreeUnpacker ) );
+						collection.Add( key, this._valueSerializer.UnpackFrom( subTreeUnpacker ) );
 					}
 				}
 				else
 				{
-					collection.Add( key, this._valueSerializer.UnpackFromCore( unpacker ) );
+					collection.Add( key, this._valueSerializer.UnpackFrom( unpacker ) );
 				}
 			}
 		}
@@ -180,12 +180,12 @@ private async Task UnpackToAsyncCore( Unpacker unpacker, Dictionary<TKey, TValue
 				{
 					using ( var subTreeUnpacker = unpacker.ReadSubtree() )
 					{
-						key = await this._keySerializer.UnpackFromAsyncCore( subTreeUnpacker, cancellationToken ).ConfigureAwait( false );
+						key = await this._keySerializer.UnpackFromAsync( subTreeUnpacker, cancellationToken ).ConfigureAwait( false );
 					}
 				}
 				else
 				{
-					key = await this._keySerializer.UnpackFromAsyncCore( unpacker, cancellationToken ).ConfigureAwait( false );
+					key = await this._keySerializer.UnpackFromAsync( unpacker, cancellationToken ).ConfigureAwait( false );
 				}
 
 				if ( !unpacker.Read() )
@@ -197,12 +197,12 @@ private async Task UnpackToAsyncCore( Unpacker unpacker, Dictionary<TKey, TValue
 				{
 					using ( var subTreeUnpacker = unpacker.ReadSubtree() )
 					{
-						collection.Add( key, await this._valueSerializer.UnpackFromAsyncCore( subTreeUnpacker, cancellationToken ).ConfigureAwait( false ) );
+						collection.Add( key, await this._valueSerializer.UnpackFromAsync( subTreeUnpacker, cancellationToken ).ConfigureAwait( false ) );
 					}
 				}
 				else
 				{
-					collection.Add( key, await this._valueSerializer.UnpackFromAsyncCore( unpacker, cancellationToken ).ConfigureAwait( false ) );
+					collection.Add( key, await this._valueSerializer.UnpackFromAsync( unpacker, cancellationToken ).ConfigureAwait( false ) );
 				}
 			}
 		}

diff --git a/...on/DefaultSerializers/System_Collections_Generic_KeyValuePair_2MessagePackSerializer`2.cs b/...on/DefaultSerializers/System_Collections_Generic_KeyValuePair_2MessagePackSerializer`2.cs
@@ -61,14 +61,14 @@ protected internal override KeyValuePair<TKey, TValue> UnpackFromCore( Unpacker
 				SerializationExceptions.ThrowUnexpectedEndOfStream( unpacker );
 			}
 
-			var key = unpacker.LastReadData.IsNil ? default( TKey ) : this._keySerializer.UnpackFromCore( unpacker );
+			var key = unpacker.LastReadData.IsNil ? default( TKey ) : this._keySerializer.UnpackFrom( unpacker );
 
 			if ( !unpacker.Read() )
 			{
 				SerializationExceptions.ThrowUnexpectedEndOfStream( unpacker );
 			}
 
-			var value = unpacker.LastReadData.IsNil ? default( TValue ) : this._valueSerializer.UnpackFromCore( unpacker );
+			var value = unpacker.LastReadData.IsNil ? default( TValue ) : this._valueSerializer.UnpackFrom( unpacker );
 
 			return new KeyValuePair<TKey, TValue>( key, value );
 		}
@@ -89,14 +89,14 @@ protected internal override async Task<KeyValuePair<TKey, TValue>> UnpackFromAsy
 				SerializationExceptions.ThrowUnexpectedEndOfStream( unpacker );
 			}
 
-			var key = unpacker.LastReadData.IsNil ? default( TKey ) : await this._keySerializer.UnpackFromAsyncCore( unpacker, cancellationToken ).ConfigureAwait( false );
+			var key = unpacker.LastReadData.IsNil ? default( TKey ) : await this._keySerializer.UnpackFromAsync( unpacker, cancellationToken ).ConfigureAwait( false );
 
 			if ( !await unpacker.ReadAsync( cancellationToken ).ConfigureAwait( false ) )
 			{
 				SerializationExceptions.ThrowUnexpectedEndOfStream( unpacker );
 			}
 
-			var value = unpacker.LastReadData.IsNil ? default( TValue ) : await this._valueSerializer.UnpackFromAsyncCore( unpacker, cancellationToken ).ConfigureAwait( false );
+			var value = unpacker.LastReadData.IsNil ? default( TValue ) : await this._valueSerializer.UnpackFromAsync( unpacker, cancellationToken ).ConfigureAwait( false );
 
 			return new KeyValuePair<TKey, TValue>( key, value );
 		}

diff --git a/...ialization/DefaultSerializers/System_Collections_Generic_List_1MessagePackSerializer`1.cs b/...ialization/DefaultSerializers/System_Collections_Generic_List_1MessagePackSerializer`1.cs
@@ -99,12 +99,12 @@ private void UnpackToCore( Unpacker unpacker, List<T> collection, int count )
 				{
 					using ( var subTreeUnpacker = unpacker.ReadSubtree() )
 					{
-						collection.Add( this._itemSerializer.UnpackFromCore( subTreeUnpacker ) );
+						collection.Add( this._itemSerializer.UnpackFrom( subTreeUnpacker ) );
 					}
 				}
 				else
 				{
-					collection.Add( this._itemSerializer.UnpackFromCore( unpacker ) );
+					collection.Add( this._itemSerializer.UnpackFrom( unpacker ) );
 				}
 			}
 		}
@@ -158,12 +158,12 @@ private async Task UnpackToAsyncCore( Unpacker unpacker, List<T> collection, int
 				{
 					using ( var subTreeUnpacker = unpacker.ReadSubtree() )
 					{
-						collection.Add( await this._itemSerializer.UnpackFromAsyncCore( subTreeUnpacker, cancellationToken ).ConfigureAwait( false ) );
+						collection.Add( await this._itemSerializer.UnpackFromAsync( subTreeUnpacker, cancellationToken ).ConfigureAwait( false ) );
 					}
 				}
 				else
 				{
-					collection.Add( await this._itemSerializer.UnpackFromAsyncCore( unpacker, cancellationToken ).ConfigureAwait( false ) );
+					collection.Add( await this._itemSerializer.UnpackFromAsync( unpacker, cancellationToken ).ConfigureAwait( false ) );
 				}
 			}
 		}

diff --git a/...alization/DefaultSerializers/System_Collections_Generic_Stack_1MessagePackSerializer`1.cs b/...alization/DefaultSerializers/System_Collections_Generic_Stack_1MessagePackSerializer`1.cs
@@ -111,7 +111,7 @@ protected internal override async Task PackToAsyncCore( Packer packer, Stack<TIt
 			await packer.PackArrayHeaderAsync( objectTree.Count, cancellationToken ).ConfigureAwait( false );
 			foreach ( var item in objectTree )
 			{
-				await this._itemSerializer.PackToAsyncCore( packer, item, cancellationToken ).ConfigureAwait( false );
+				await this._itemSerializer.PackToAsync( packer, item, cancellationToken ).ConfigureAwait( false );
 			}
 		}
 

diff --git a/test/MsgPack.UnitTest/Serialization/RegressionTests.cs b/test/MsgPack.UnitTest/Serialization/RegressionTests.cs
@@ -381,5 +381,70 @@ public void TestDateTimeTypeError_SerializationException()
 				}
 			}
 		}
+
+		[Test]
+		public void TestIssue211_ListOfT()
+		{
+			var target = new SerializationContext().GetSerializer<List<SingleValueObject>>();
+			using ( var buffer = new MemoryStream() )
+			{
+				target.Pack( buffer, new List<SingleValueObject> { null } );
+				buffer.Position = 0;
+				var result = target.Unpack( buffer );
+				Assert.That( result.Count, Is.EqualTo( 1 ) );
+				Assert.That( result[ 0 ], Is.Null );
+			}
+		}
+
+		[Test]
+		public void TestIssue211_DictionaryOfT()
+		{
+			var target = new SerializationContext().GetSerializer<Dictionary<string, SingleValueObject>>();
+			using ( var buffer = new MemoryStream() )
+			{
+				target.Pack( buffer, new Dictionary<string, SingleValueObject> { { String.Empty, null } } );
+				buffer.Position = 0;
+				var result = target.Unpack( buffer );
+				Assert.That( result.Count, Is.EqualTo( 1 ) );
+				Assert.That( result.First().Value, Is.Null );
+			}
+		}
+
+		[Test]
+		public void TestIssue211_StackOfT()
+		{
+			var target = new SerializationContext().GetSerializer<Stack<SingleValueObject>>();
+			using ( var buffer = new MemoryStream() )
+			{
+				var obj = new Stack<SingleValueObject>();
+				obj.Push( null );
+				target.Pack( buffer, obj );
+				buffer.Position = 0;
+				var result = target.Unpack( buffer );
+				Assert.That( result.Count, Is.EqualTo( 1 ) );
+				Assert.That( result.Pop(), Is.Null );
+			}
+		}
+
+		[Test]
+		public void TestIssue211_QueueOfT()
+		{
+			var target = new SerializationContext().GetSerializer<Queue<SingleValueObject>>();
+			using ( var buffer = new MemoryStream() )
+			{
+				var obj = new Queue<SingleValueObject>();
+				obj.Enqueue( null );
+				target.Pack( buffer, obj );
+				buffer.Position = 0;
+				var result = target.Unpack( buffer );
+				Assert.That( result.Count, Is.EqualTo( 1 ) );
+				Assert.That( result.Dequeue(), Is.Null );
+			}
+		}
+
+		public class SingleValueObject
+		{
+			public string Value { get; set; }
+		}
 	}
 }