From 11b1ec3da2973df12befabf684187f509204afb9 Mon Sep 17 00:00:00 2001 From: Eric StJohn Date: Wed, 30 Oct 2019 12:19:28 -0700 Subject: [PATCH] Handle binary formatted payloads with ResX mangled generic type names (#42102) (#42209) * Handle binary formatted payloads with ResX mangled generic type names ResXSerializationBinder on desktop corrupted type names for generic parameters in the binary formatted payload. It would also undo this in the reader, but we don't benefit from that in .NETCore since we don't deserialize during build: we just copy the preserialized payload into the resources. To handle this, we use a serialization binder to un-mangle the type names in a way similar to ResXSerializationBinder. We do it slightly differently so that we only do it when needed, since relying on the binder to resolve the type bypasses the type cache in BinaryFormatter. * Respond to feedback. release/3.1 - regenerate TestData.resources --- .../Extensions/DeserializingResourceReader.cs | 62 ++++++++- .../Extensions/PreserializedResourceWriter.cs | 8 +- .../tests/BinaryResourceWriterUnitTest.cs | 46 ++----- .../tests/TestData.cs | 129 ++++++++++++++++-- .../tests/TestData.resources | Bin 3382 -> 8184 bytes 5 files changed, 194 insertions(+), 51 deletions(-) diff --git a/src/System.Resources.Extensions/src/System/Resources/Extensions/DeserializingResourceReader.cs b/src/System.Resources.Extensions/src/System/Resources/Extensions/DeserializingResourceReader.cs index f6a8210dff9a..057ace872c9d 100644 --- a/src/System.Resources.Extensions/src/System/Resources/Extensions/DeserializingResourceReader.cs +++ b/src/System.Resources.Extensions/src/System/Resources/Extensions/DeserializingResourceReader.cs @@ -5,6 +5,7 @@ #nullable enable using System.ComponentModel; using System.IO; +using System.Runtime.Serialization; using System.Runtime.Serialization.Formatters.Binary; namespace System.Resources.Extensions @@ -38,12 +39,71 @@ private object ReadBinaryFormattedObject() { if (_formatter == null) { - _formatter = new BinaryFormatter(); + _formatter = new BinaryFormatter() + { + Binder = new UndoTruncatedTypeNameSerializationBinder() + }; } return _formatter.Deserialize(_store.BaseStream); } + + internal class UndoTruncatedTypeNameSerializationBinder : SerializationBinder + { + public override Type BindToType(string assemblyName, string typeName) + { + Type type = null; + + // determine if we have a mangled generic type name + if (typeName != null && assemblyName != null && !AreBracketsBalanced(typeName)) + { + // undo the mangling that may have happened with .NETFramework's + // incorrect ResXSerialization binder. + typeName = typeName + ", " + assemblyName; + + type = Type.GetType(typeName, throwOnError: false, ignoreCase:false); + } + + // if type is null we'll fall back to the default type binder which is preferable + // since it is backed by a cache + return type; + } + + private static bool AreBracketsBalanced(string typeName) + { + // make sure brackets are balanced + int firstBracket = typeName.IndexOf('['); + + if (firstBracket == -1) + { + return true; + } + + int brackets = 1; + for (int i = firstBracket + 1; i < typeName.Length; i++) + { + if (typeName[i] == '[') + { + brackets++; + } + else if (typeName[i] == ']') + { + brackets--; + + if (brackets < 0) + { + // unbalanced, closing bracket without opening + break; + } + } + } + + return brackets == 0; + } + + } + private object DeserializeObject(int typeIndex) { Type type = FindType(typeIndex); diff --git a/src/System.Resources.Extensions/src/System/Resources/Extensions/PreserializedResourceWriter.cs b/src/System.Resources.Extensions/src/System/Resources/Extensions/PreserializedResourceWriter.cs index 267043ca845a..23c284ca9adc 100644 --- a/src/System.Resources.Extensions/src/System/Resources/Extensions/PreserializedResourceWriter.cs +++ b/src/System.Resources.Extensions/src/System/Resources/Extensions/PreserializedResourceWriter.cs @@ -155,12 +155,14 @@ public void AddBinaryFormattedResource(string name, byte[] value, string? typeNa // and reserializing when writing the resources. We don't want to do that so instead // we just omit the type. typeName = UnknownObjectTypeName; - - // ResourceReader will validate the type so we must use the new reader. - _requiresDeserializingResourceReader = true; } AddResourceData(name, typeName, new ResourceDataRecord(SerializationFormat.BinaryFormatter, value)); + + // Even though ResourceReader can handle BinaryFormatted resources, the resource may contain + // type names that were mangled by the ResXWriter's SerializationBinder, which we need to fix + + _requiresDeserializingResourceReader = true; } /// diff --git a/src/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs b/src/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs index 68e60c911438..0bd7b4c0a9e6 100644 --- a/src/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs +++ b/src/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs @@ -267,23 +267,7 @@ public static void PrimitiveResourcesAsStrings() public static void BinaryFormattedResources() { var values = TestData.BinaryFormatted; - byte[] writerBuffer, binaryWriterBuffer; - using (MemoryStream ms = new MemoryStream()) - using (ResourceWriter writer = new ResourceWriter(ms)) - { - BinaryFormatter binaryFormatter = new BinaryFormatter(); - - foreach (var pair in values) - { - using (MemoryStream memoryStream = new MemoryStream()) - { - binaryFormatter.Serialize(memoryStream, pair.Value); - writer.AddResourceData(pair.Key, TestData.GetSerializationTypeName(pair.Value.GetType()), memoryStream.ToArray()); - } - } - writer.Generate(); - writerBuffer = ms.ToArray(); - } + byte[] binaryWriterBuffer; using (MemoryStream ms = new MemoryStream()) using (PreserializedResourceWriter writer = new PreserializedResourceWriter(ms)) @@ -302,24 +286,8 @@ public static void BinaryFormattedResources() binaryWriterBuffer = ms.ToArray(); } - // PreserializedResourceWriter should write ResourceWriter/ResourceReader format - Assert.Equal(writerBuffer, binaryWriterBuffer); - - using (MemoryStream ms = new MemoryStream(writerBuffer, false)) - using (ResourceReader reader = new ResourceReader(ms)) - { - typeof(ResourceReader).GetField("_permitDeserialization", BindingFlags.Instance | BindingFlags.NonPublic)?.SetValue(reader, true); - - IDictionaryEnumerator dictEnum = reader.GetEnumerator(); - - while (dictEnum.MoveNext()) - { - ResourceValueEquals(values[(string)dictEnum.Key], dictEnum.Value); - } - } - - // DeserializingResourceReader can read ResourceReader format - using (MemoryStream ms = new MemoryStream(writerBuffer, false)) + // DeserializingResourceReader can read BinaryFormatted resources with type names. + using (MemoryStream ms = new MemoryStream(binaryWriterBuffer, false)) using (DeserializingResourceReader reader = new DeserializingResourceReader(ms)) { IDictionaryEnumerator dictEnum = reader.GetEnumerator(); @@ -510,7 +478,13 @@ public static void EmbeddedResourcesAreUpToDate() { TestData.WriteResourcesStream(actualData); resourcesStream.CopyTo(expectedData); - Assert.Equal(expectedData.ToArray(), actualData.ToArray()); + + if (!PlatformDetection.IsFullFramework) + { + // Some types rely on SerializationInfo.SetType on .NETCore + // which result in a different binary format + Assert.Equal(expectedData.ToArray(), actualData.ToArray()); + } } } diff --git a/src/System.Resources.Extensions/tests/TestData.cs b/src/System.Resources.Extensions/tests/TestData.cs index 0ddef55b607b..e83d0f31ab13 100644 --- a/src/System.Resources.Extensions/tests/TestData.cs +++ b/src/System.Resources.Extensions/tests/TestData.cs @@ -4,13 +4,16 @@ using System.Collections.Generic; using System.ComponentModel; +using System.Diagnostics; using System.Drawing; using System.Drawing.Imaging; using System.Globalization; using System.IO; using System.Linq; using System.Runtime.CompilerServices; +using System.Runtime.Serialization; using System.Runtime.Serialization.Formatters.Binary; +using System.Text; namespace System.Resources.Extensions.Tests { @@ -49,7 +52,15 @@ public static IReadOnlyDictionary BinaryFormatted new Dictionary() { ["enum_bin"] = DayOfWeek.Friday, - ["point_bin"] = new Point(4, 8) + ["point_bin"] = new Point(4, 8), + ["array_int_bin"] = new int[] { 1, 2, 3, 4, 5, 6 }, + ["list_int_bin"] = new List() { 1, 2, 3, 4, 5, 6 }, + ["stack_Point_bin"] = new Stack(new [] { new Point(4, 8), new Point(2, 5) }), + ["dict_string_string_bin"] = new Dictionary() + { + { "key1", "value1" }, + { "key2", "value2" } + } }; public static Dictionary BinaryFormattedWithoutDrawingNoType { get; } = @@ -142,21 +153,75 @@ public static string GetStringValue(object value) return converter.ConvertToInvariantString(value); } - public static string GetSerializationTypeName(Type runtimeType) + + // Copied from FormatterServices.cs + internal static string GetClrTypeFullName(Type type) + { + return type.IsArray ? + GetClrTypeFullNameForArray(type) : + GetClrTypeFullNameForNonArrayTypes(type); + } + + private static string GetClrTypeFullNameForArray(Type type) + { + int rank = type.GetArrayRank(); + Debug.Assert(rank >= 1); + string typeName = GetClrTypeFullName(type.GetElementType()); + return rank == 1 ? + typeName + "[]" : + typeName + "[" + new string(',', rank - 1) + "]"; + } + + private static string GetClrTypeFullNameForNonArrayTypes(Type type) + { + if (!type.IsGenericType) + { + return type.FullName; + } + + var builder = new StringBuilder(type.GetGenericTypeDefinition().FullName).Append("["); + + foreach (Type genericArgument in type.GetGenericArguments()) + { + builder.Append("[").Append(GetClrTypeFullName(genericArgument)).Append(", "); + builder.Append(GetClrAssemblyName(genericArgument)).Append("],"); + } + + //remove the last comma and close typename for generic with a close bracket + return builder.Remove(builder.Length - 1, 1).Append("]").ToString(); + } + + private static string GetClrAssemblyName(Type type) { - object[] typeAttributes = runtimeType.GetCustomAttributes(typeof(TypeForwardedFromAttribute), false); - if (typeAttributes != null && typeAttributes.Length > 0) + if (type == null) + { + throw new ArgumentNullException(nameof(type)); + } + + // Special case types like arrays + Type attributedType = type; + while (attributedType.HasElementType) { - TypeForwardedFromAttribute typeForwardedFromAttribute = (TypeForwardedFromAttribute)typeAttributes[0]; - return $"{runtimeType.FullName}, {typeForwardedFromAttribute.AssemblyFullName}"; + attributedType = attributedType.GetElementType(); } - else if (runtimeType.Assembly == typeof(object).Assembly) + + foreach (Attribute first in attributedType.GetCustomAttributes(typeof(TypeForwardedFromAttribute), false)) { - // no attribute and in corelib. Strip the assembly name and hope its in CoreLib on other frameworks - return runtimeType.FullName; + return ((TypeForwardedFromAttribute)first).AssemblyFullName; } - return runtimeType.AssemblyQualifiedName; + return type.Assembly.FullName; + } + + public static string GetSerializationTypeName(Type runtimeType) + { + string typeName = GetClrTypeFullName(runtimeType); + if (runtimeType.Assembly == typeof(object).Assembly) + { + // In corelib. Strip the assembly name and hope its in CoreLib on other frameworks + return typeName; + } + return $"{typeName}, {GetClrAssemblyName(runtimeType)}"; } public static void WriteResources(string file) @@ -178,7 +243,11 @@ public static void WriteResourcesStream(Stream stream) writer.AddResource(pair.Key, GetStringValue(pair.Value), GetSerializationTypeName(pair.Value.GetType())); } - var formatter = new BinaryFormatter(); + var formatter = new BinaryFormatter() + { + Binder = new TypeNameManglingSerializationBinder() + }; + foreach (var pair in BinaryFormattedWithoutDrawing) { using (MemoryStream memoryStream = new MemoryStream()) @@ -217,5 +286,43 @@ public static void WriteResourcesStream(Stream stream) writer.Generate(); } } + + /// + /// An approximation of ResXSerializationBinder's behavior (without retargeting) + /// + internal class TypeNameManglingSerializationBinder : SerializationBinder + { + static readonly string s_coreAssemblyName = typeof(object).Assembly.FullName; + static readonly string s_mscorlibAssemblyName = "mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089"; + + public override void BindToName(Type serializedType, out string assemblyName, out string typeName) + { + typeName = null; + // Apply type-forwarded from here, so that we mimic what would happen in ResXWriter which runs in VS on desktop + string assemblyQualifiedTypeName = GetSerializationTypeName(serializedType); + + // workaround for https://github.com/dotnet/corefx/issues/42092 + assemblyQualifiedTypeName = assemblyQualifiedTypeName.Replace(s_coreAssemblyName, s_mscorlibAssemblyName); + + int pos = assemblyQualifiedTypeName.IndexOf(','); + if (pos > 0 && pos < assemblyQualifiedTypeName.Length - 1) + { + assemblyName = assemblyQualifiedTypeName.Substring(pos + 1).TrimStart(); + string newTypeName = assemblyQualifiedTypeName.Substring(0, pos); + if (!string.Equals(newTypeName, serializedType.FullName, StringComparison.InvariantCulture)) + { + typeName = newTypeName; + } + return; + } + base.BindToName(serializedType, out assemblyName, out typeName); + } + + public override Type BindToType(string assemblyName, string typeName) + { + // We should never be using this binder during Deserialization + throw new NotSupportedException($"{nameof(TypeNameManglingSerializationBinder)}.{nameof(BindToType)} should not be used during testing."); + } + } } } diff --git a/src/System.Resources.Extensions/tests/TestData.resources b/src/System.Resources.Extensions/tests/TestData.resources index e946372c3748ab7dd6e5c3d7ecd492577ac2bbe2..8ea7850e4ff63ba48a42826039dc2ed0e0894878 100644 GIT binary patch literal 8184 zcmeHM3v3%j8lH_G**u!$ppBxE%ekdGpfYak#7U_LYKhaNrH?cYK`64^^?ICcXoBK##{!-cTVP;#Cl+AW)$KAr9$Et4cg8Q5}!=a^I|X9ow5) zcOj8l(M@J&=KsH$`RD(?ota&~_s+L3bA*sfaNEg+Gn!6Ee0@|?(yB-`U)Ls`%9^Ce znlD5(s!Bpq+APUK7NU;|ajJSfB^r7?15{;QbhP@K*`L?bnNI3ym3GK9t*b)P>*-C$ zl9G58&Gak7RPGSP_L#pVkr3m7csw5T-%`@BzO<}MBh)e}O!euurwk4YBF#XmF{i#v z)>{IDk&8-r!gsYiEGrx3{!EGvnYfTDY=l33ok~&}_oFGDgzI9>Es0>FJ>mDqgUv#V zU`3`osl-_91Ras^nE_ZPCzO9#zJ&!zDl)a^>!vbdE&4)|ArREe+Q8tTX>wRsVX@aU zqKS%{lwy;yvb|jhiorJjWi73=dEsRd?{sz^sk59dWL77xp>+5LQ!vq-S4f(^)}NcL z5!b1mY-ES!bBnVD4_ZIfbj`OiV<{P3N;2AKv?vUNe=^TmF)4c$NgjWgrjC@uRy<7d z_knkUJd_-t=G2WV|be9M0peIXmBU#W51G zW>`qR|7nqr6|DiiOG5bHx97x7f85fz{jpTj-OGC)-@E@(|C2AayztcWcaH5_D?I$t z7kk844@W(3?QQVB{qCdN4;;Lnd+(9+Hy&Ko^x-%2KCk%xz>YV6R39E2+w!{`$dOgs zoF~13%3!Fia`s>UQ&}JWQ`KEJKUlT%r^l*RIIF6|4?JJ@+QHXmI(G$U9UHr?F@5i? zzrFCoC+D_*QWN;<=eezG2E4)jA4oxE*ZeC|&-QeFdic+wMZ4!*yWze@F?PI7I_X-l z{@8-8>y_|D`s#Vg=$DtLMpr+S8vX2OYV>f2KDuxB*l6m={WqS%HnGl0$Sv4o49sq8|qtuJBqX$n2(UpLLURjuYf)R+#9HW zhtvpt2J-cgbs_JnA*3!u1fmm4`iVqFkh5!Uu`o)O5RHTZQ~{I85UBvH&T11TDfA+f z^H|$rp}p+|fXx=rm5Npw5;eNik;jbEJg6@|y*f%(5e4REFeoL10P`^j6p|#{S;tnz zKJIN}hBfyh%RvX8zWAa5LPL<~^aWpOR*-$Q7y=>KU@= z3C4u_;t?>q5AM_q*Qy97dnmIkXD-V!RU^j%o?jZA^%I3-M#xFP&r6|CYrJ}5r`T{t<{GdkOOw*sn4Eobu}s*U4S`}`wW{5n{vGOuUxxvFjPi2=}{ zVMd!V{L&)QQz+{#W#JQ$f~twlL4eW2XxD4=%SJ%Fr6Gkry1o{VL%v`B@EdawYpJx) z_b-6(X)_{!LuIyAv14~YUCCCDER*g;wQ;;^$3Fl-bs8+P+_x8krhI|D%@css-gIcR zr)=)4e6CPOUXkX%jm08YSj@E&>>*?f7t7gI%L;BJo)xeg5$rAmI}gD+VJNDWOs-iO zSH?NZo#l=g!T*(xiY0izq*o@>wA10D@}*aYYi{Q5I^KB+7;ap(#3l%FPxCgDtC^LL zaA(iEkBhmst;G9IiFZ5}iE{Jpo$pjeQi4iVzDLm3vD57gysr-kaA~g~sTxA(;J24L zotiI|UDu`YBC2Ps{_@VN)A42*;g6OwDt``V^*iohgfWw)3ic4ORKeosWTi7PE-;If zGYgGpj2Z4+Sc0Y{1I1dqe#Xl3i+XSj(~N5aV>HLH9^Gtdazf~Lm$Ry&@^e3695$94 z8@izmxUpZG>Ht=(bIZoLW#er5GT(zWZ@b#o}C{i(6znemL1~Zu^|c zgyqKS6pa#|ot>}>{es3_4);=3iVKHM3Y~GmrN7dN zSCTKP`kCi)h3+!`AMbw4UD!3Wt=pL^_HcKeh8b%LKaNi3Yhlsv8U{ON7IvHc&dChr zHgOpiZ&qeU3>IL-?~FFD$KeckJ#EVz)h)=p9zT943pBe`x>?%6fPRLW%)Ht(FsLf|FLs~56elky=3}CMG=U7nilc%vX{MuN+nJ+Hc Xj3*TxSoCnK0$q$NS19lK2LJyBy7(B# delta 1004 zcmZ8fYe-XJ7=FL+%z1XU*<{`}XWJ^oYArLn7*cAwB#;yni7qmAUP!IxMo#ET1`=Ig zsXt{9_9H60Q6l-H^iKpqBt(S$2qOB?jVLI}>N(Ra+RppE&*eSu_kJJe>zX^x>jgqx zL_{V8cZuF)TXn265^DFC24fqVYD1yzTq$zoX~9*(b&HiOnO0dmbn(mnLkC@_26xUr zx4!b))sdxlZ}JtRk0*?-lR6zA4b(zG3X@OwrU?gCyI9NzOfHef{id9h)i9G9(V>A_ zsU7ug6yWEkdG=ZumPYIsrFw`VG~)xBH+>g$TZlTKw^JRwR^V>pVa+Rth$l2h@@JH2 zO?YVs`bB7x*l2d!B~ESBizAc>m;uN(bE?AvqUyCB)J(}B&S|30DxHGI%nRjQ!5__w zq+hVxl4>kKm1ie!v%2Lo!J93a%Xj0d(!%j$L@-q+HVMZkYo@)hv`!2abrxjpg4>FG)UO_0GBuU5&;E ziO`Vbd)BPv5olU8emnY!?@NAfb=x269ikZg&m7~DJxN~qRPx%S>`W=7FeDlhm3qQc zq?n4^S{p(oZP8FMe@dEfw{{%P{P@jlDw+W$d^WvMtku^*iDC?dTN{Hh%h;FlqWGf5 zKM{S==#_aRcK-`XrF!|%2Qjy-xwq6X1IxI=*^h=;^U4%Mp3k#9pjeh7`8-Q