Skip to content

Fix unpack skipping when extra fields are present #98

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 9, 2015
Merged

Fix unpack skipping when extra fields are present #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions src/MsgPack/ItemsUnpacker.Skipping.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,60 @@ partial class ItemsUnpacker
#endregion DrainValue
continue;
}
case MessagePackCode.Str8:
case MessagePackCode.Bin8:
{
skipped += 1;
byte length;
var read = source.Read( buffer, 0, 1 );
if ( read == 1 )
{
length = buffer[0];
skipped += 1;
}
else
{
return null;
}
#region DrainValue

long bytesRead = 0;
while( length > bytesRead )
{
var remaining = ( length - bytesRead );
var reading = remaining > DummyBufferForSkipping.Length ? DummyBufferForSkipping.Length : unchecked( ( int )remaining );
bytesRead += source.Read( DummyBufferForSkipping, 0, reading );
if ( bytesRead < reading )
{
return null;
}
}

skipped += bytesRead;

#endregion DrainValue
#region TryPopContextCollection

remainingItems--;

if( remainingCollections != null )
{
while ( remainingItems == 0 && remainingCollections.Count > 0 )
{
if( remainingCollections.Count == 0 )
{
break;
}

remainingItems = remainingCollections.Pop();
remainingItems--;
}
}

#endregion TryPopContextCollection
continue;
}
case MessagePackCode.Bin16:
case MessagePackCode.Raw16:
{
skipped += 1;
Expand Down Expand Up @@ -511,6 +565,7 @@ partial class ItemsUnpacker
#endregion TryPopContextCollection
continue;
}
case MessagePackCode.Bin32:
case MessagePackCode.Raw32:
{
skipped += 1;
Expand Down
34 changes: 33 additions & 1 deletion src/MsgPack/ItemsUnpacker.Skipping.tt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,21 @@ this.PopIndent();
#>
continue;
}
case MessagePackCode.Str8:
case MessagePackCode.Bin8:
{
skipped += 1;
byte length;
<#
this.PushIndent( 6 );
this.WriteUnpackByte( "length" );
this.WriteDrainValue( "length" );
this.WriteTryPopCollection();
this.PopIndent();
#>
continue;
}
case MessagePackCode.Bin16:
case MessagePackCode.Raw16:
{
skipped += 1;
Expand All @@ -242,6 +257,7 @@ this.PopIndent();
#>
continue;
}
case MessagePackCode.Bin32:
case MessagePackCode.Raw32:
{
skipped += 1;
Expand Down Expand Up @@ -463,4 +479,20 @@ else
}
<#+
}
#>

private void WriteUnpackByte( string lengthVariable )
{
#>
var read = source.Read( buffer, 0, 1 );
if ( read == 1 )
{
<#= lengthVariable #> = buffer[0];
skipped += 1;
}
else
{
return null;
}
<#+
}
#>
54 changes: 31 additions & 23 deletions test/MsgPack.UnitTest/Serialization/VersioningTest.Cases.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ partial class VersioningTest
#if !NETFX_CORE

[Test]
public void TestExtraField_NotExtensible_Array_FieldBased_Fail()
public void TestExtraField_NotExtensible_Array_FieldBased_Classic_Fail()
{
TestExtraFieldCore<VersioningTestTarget>( SerializationMethod.Array, EmitterFlavor.FieldBased );
TestExtraFieldCore( SerializationMethod.Array, EmitterFlavor.FieldBased, PackerCompatibilityOptions.Classic );
}

[Test]
Expand All @@ -59,9 +59,9 @@ public void TestFieldInvalidType_Array_FieldBased_Fail()
#if !NETFX_CORE

[Test]
public void TestExtraField_NotExtensible_Array_ContextBased_Fail()
public void TestExtraField_NotExtensible_Array_ContextBased_Classic_Fail()
{
TestExtraFieldCore<VersioningTestTarget>( SerializationMethod.Array, EmitterFlavor.ContextBased );
TestExtraFieldCore( SerializationMethod.Array, EmitterFlavor.ContextBased, PackerCompatibilityOptions.Classic );
}

[Test]
Expand All @@ -79,9 +79,9 @@ public void TestFieldInvalidType_Array_ContextBased_Fail()
#if !NETFX_35

[Test]
public void TestExtraField_NotExtensible_Array_ExpressionBased_Fail()
public void TestExtraField_NotExtensible_Array_ExpressionBased_Classic_Fail()
{
TestExtraFieldCore<VersioningTestTarget>( SerializationMethod.Array, EmitterFlavor.ExpressionBased );
TestExtraFieldCore( SerializationMethod.Array, EmitterFlavor.ExpressionBased, PackerCompatibilityOptions.Classic );
}

[Test]
Expand All @@ -99,9 +99,15 @@ public void TestFieldInvalidType_Array_ExpressionBased_Fail()
#if !NETFX_CORE

[Test]
public void TestExtraField_NotExtensible_Map_FieldBased_Fail()
public void TestExtraField_NotExtensible_Map_FieldBased_Classic_Fail()
{
TestExtraFieldCore<VersioningTestTarget>( SerializationMethod.Map, EmitterFlavor.FieldBased );
TestExtraFieldCore( SerializationMethod.Map, EmitterFlavor.FieldBased, PackerCompatibilityOptions.Classic );
}

[Test]
public void TestExtraField_NotExtensible_Map_FieldBased_None_Fail ()
{
TestExtraFieldCore( SerializationMethod.Map, EmitterFlavor.FieldBased, PackerCompatibilityOptions.None );
}

[Test]
Expand All @@ -125,9 +131,15 @@ public void TestFieldModified_Map_FieldBased_ExtraIsStoredAsExtensionData_Missin
#if !NETFX_CORE

[Test]
public void TestExtraField_NotExtensible_Map_ContextBased_Fail()
public void TestExtraField_NotExtensible_Map_ContextBased_Classic_Fail()
{
TestExtraFieldCore( SerializationMethod.Map, EmitterFlavor.ContextBased, PackerCompatibilityOptions.Classic );
}

[Test]
public void TestExtraField_NotExtensible_Map_ContextBased_None_Fail ()
{
TestExtraFieldCore<VersioningTestTarget>( SerializationMethod.Map, EmitterFlavor.ContextBased );
TestExtraFieldCore( SerializationMethod.Map, EmitterFlavor.ContextBased, PackerCompatibilityOptions.None );
}

[Test]
Expand All @@ -151,9 +163,15 @@ public void TestFieldModified_Map_ContextBased_ExtraIsStoredAsExtensionData_Miss
#if !NETFX_35

[Test]
public void TestExtraField_NotExtensible_Map_ExpressionBased_Fail()
public void TestExtraField_NotExtensible_Map_ExpressionBased_Classic_Fail()
{
TestExtraFieldCore<VersioningTestTarget>( SerializationMethod.Map, EmitterFlavor.ExpressionBased );
TestExtraFieldCore( SerializationMethod.Map, EmitterFlavor.ExpressionBased, PackerCompatibilityOptions.Classic );
}

[Test]
public void TestExtraField_NotExtensible_Map_ExpressionBased_None_Fail ()
{
TestExtraFieldCore( SerializationMethod.Map, EmitterFlavor.ExpressionBased, PackerCompatibilityOptions.None );
}

[Test]
Expand All @@ -175,14 +193,4 @@ public void TestFieldModified_Map_ExpressionBased_ExtraIsStoredAsExtensionData_M
}
#endif // !NETFX_35
}

public class VersioningTestTarget
{
[MessagePackMember( 0 )]
public Int32 Field1 { get; set; }
[MessagePackMember( 1 )]
public Int32 Field2 { get; set; }
[MessagePackMember( 2 )]
public String Field3 { get; set; }
}
}
}
44 changes: 32 additions & 12 deletions test/MsgPack.UnitTest/Serialization/VersioningTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ private static MessagePackSerializer<T> CreateSerializer<T>( EmitterFlavor flavo
#endif // !XAMIOS && !UNITY_IPHONE
}

private static void TestExtraFieldCore<T>( SerializationMethod method, EmitterFlavor flavor )
private static void TestExtraFieldCore( SerializationMethod method, EmitterFlavor flavor, PackerCompatibilityOptions compat )
{
var serializer = CreateSerializer<T>( flavor );
var serializer = CreateSerializer<VersioningTestTarget>(flavor);

using ( var stream = new MemoryStream() )
{
Expand All @@ -63,23 +63,33 @@ private static void TestExtraFieldCore<T>( SerializationMethod method, EmitterFl
}
else
{
var packer = Packer.Create( stream, false );
packer.PackMapHeader( 5 );
const string String10 = "1234567890"; // packed as MinimumFixedRaw or Bin8
const string String40 = "1234567890123456789012345678901234567890"; // packed as Bin8 or Str8
var packer = Packer.Create( stream, compat, false );
packer.PackMapHeader( 7 );
packer.Pack( "Field1" );
packer.Pack( 1 );
packer.Pack( "Extra" );
packer.PackNull();
packer.Pack( "Field2" );
packer.Pack("Extra1");
packer.Pack(String40);
packer.Pack("Extra2");
packer.Pack(System.Text.UTF8Encoding.Default.GetBytes(String40));
packer.Pack("Field2");
packer.Pack( -1 );
packer.Pack( "Field3" );
packer.Pack("Extra3");
packer.Pack(String10);
packer.Pack("Field3");
packer.Pack( "a" );
packer.Pack( "Extra" );
packer.Pack( "Extra4" );
packer.PackNull();
}

stream.Position = 0;

serializer.Unpack( stream );
var result = serializer.Unpack( stream );

Assert.That(result.Field1, Is.EqualTo(1));
Assert.That(result.Field2, Is.EqualTo(-1));
Assert.That(result.Field3, Is.EqualTo("a"));
}
}

Expand Down Expand Up @@ -157,7 +167,7 @@ private static void TestFieldSwappedCore( EmitterFlavor flavor )
packer.Pack( "Field2" );
packer.Pack( -1 );
packer.Pack( "Extra" );
packer.Pack( 2 ); // Issue6
packer.Pack( 2 ); // Issue6

stream.Position = 0;

Expand All @@ -169,4 +179,14 @@ private static void TestFieldSwappedCore( EmitterFlavor flavor )
}
}
}
}


public class VersioningTestTarget {
[MessagePackMember(0)]
public Int32 Field1 { get; set; }
[MessagePackMember(1)]
public Int32 Field2 { get; set; }
[MessagePackMember(2)]
public String Field3 { get; set; }
}
}