Skip to content

Commit

Permalink
Fix regression in how GetData APIs swallow all exceptions. (#12656)
Browse files Browse the repository at this point in the history
restore behavior of
1. app with BinaryFormatter enabled SetData("custom format", new List<Point>(){})
2. app with BInaryFormatter disabled GetData("custom format")

NET8 and NET9 were returning an empty memory stream, I regressed it to throw the NotSupportedException from the spot where we detect that BinaryFormatter is disabled.
After this change I retain the NotSupportedException only in the Typed Get APIs
  • Loading branch information
Tanya-Solyanik authored Dec 21, 2024
1 parent d15d137 commit 47e5d4e
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ record = stream.Decode(out recordMap);
// 1. Doesn't allow arrays that have a non-zero base index (can't create these in C# or VB)
// 2. Only allows IObjectReference types that contain primitives (to avoid observable cycle
// dependencies to indeterminate state)
// But it usually requires a resolver.
// But it usually requires a resolver. Resolver is not available in the legacy mode,
// so we will fall back to BinaryFormatter in that case.
if (LocalAppContextSwitches.ClipboardDragDropEnableNrbfSerialization)
{
try
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ private static bool TryGetObjectFromDataObject<T>(
result = TryGetIStreamData(dataObject, format, resolver, legacyMode, out data);
}
}
catch (Exception e) when (e is not NotSupportedException)
catch (Exception e) when (legacyMode || e is not NotSupportedException)
{
Debug.Fail(e.ToString());
}
Expand Down Expand Up @@ -337,9 +337,8 @@ private static bool TryGetHGLOBALData<T>(
data = default;
doNotContinue = true;
}
catch (Exception ex) when (ex is not NotSupportedException)
catch (Exception ex) when (legacyMode || ex is not NotSupportedException)
{
// Should we catch SerializationExceptions that wrap NotSupported when called from the typed API?
Debug.WriteLine(ex.ToString());
}
finally
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ public void Clipboard_SetDataAsJson_ReturnsExpected()
{
SimpleTestData testData = new() { X = 1, Y = 1 };

using BinaryFormatterScope scope = new(enable: false);
Clipboard.SetDataAsJson("testData", testData);
ITypedDataObject dataObject = Clipboard.GetDataObject().Should().BeAssignableTo<ITypedDataObject>().Subject;
dataObject.GetDataPresent("testData").Should().BeTrue();
Expand All @@ -40,6 +41,7 @@ public void Clipboard_SetDataObject_WithJson_ReturnsExpected(bool copy)
{
SimpleTestData testData = new() { X = 1, Y = 1 };

using BinaryFormatterScope scope = new(enable: false);
DataObject dataObject = new();
dataObject.SetDataAsJson("testData", testData);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -908,17 +908,31 @@ public void Clipboard_SetDataAsJson_WithGeneric_ReturnsExpected()
string format = "list";
Clipboard.SetDataAsJson(format, generic1);
DataObject dataObject = Clipboard.GetDataObject().Should().BeOfType<DataObject>().Subject;
Action a = () => dataObject.TestAccessor().Dynamic._innerData.GetData(format);
// We do not handle List<Point>
a.Should().Throw<NotSupportedException>();

// We do not handle List<Point>, this is a wrong API to read JSON-serialized payload.
// This call returns an unfilled MemoryStream due to the BinaryFormatter being disabled,
// same as it was in .NET9 for any payload.
dataObject.GetData(format).Should().BeOfType<MemoryStream>();

using (BinaryFormatterInClipboardDragDropScope scope = new(enable: true))
using (BinaryFormatterScope scope2 = new(enable: true))
{
// BinaryFormatter will not find our fake System.Private.Windows.VirtualJson assembly
// and will throw a SerializationException.
var result1 = dataObject.GetData(format);
result1.Should().BeOfType<MemoryStream>();
}

Clipboard.TryGetData(format, out List<Point>? points).Should().BeTrue();
points.Should().BeEquivalentTo(generic1);

// List of primitives is an intrinsic type, formatters are bypassed.
List<int> generic2 = [];
Clipboard.SetDataAsJson(format, generic2);
dataObject = Clipboard.GetDataObject().Should().BeOfType<DataObject>().Subject;
a = () => dataObject.TestAccessor().Dynamic._innerData.GetData(format);
a.Should().NotThrow();

dataObject.GetData(format).Should().BeEquivalentTo(generic2);

Clipboard.TryGetData(format, out List<int>? intList).Should().BeTrue();
intList.Should().BeEquivalentTo(generic2);
}
Expand All @@ -943,11 +957,11 @@ public void Clipboard_SetDataAsJson_GetData()
SimpleTestData testData = new() { X = 1, Y = 1 };
// Note that this simulates out of process scenario.
Clipboard.SetDataAsJson("test", testData);
Action a = () => Clipboard.GetData("test");
a.Should().Throw<NotSupportedException>();

Clipboard.GetData("test").Should().BeOfType<MemoryStream>();

using BinaryFormatterInClipboardDragDropScope scope = new(enable: true);
a.Should().Throw<NotSupportedException>();
Clipboard.GetData("test").Should().BeOfType<MemoryStream>();

using BinaryFormatterScope scope2 = new(enable: true);
Clipboard.GetData("test").Should().BeOfType<MemoryStream>();
Expand Down Expand Up @@ -1001,7 +1015,7 @@ public unsafe void Clipboard_Deserialize_FromStream_Manually()
Clipboard.SetDataAsJson("testFormat", testData);

// Manually retrieve the serialized stream.
ComTypes.IDataObject dataObject = Clipboard.GetDataObject().Should().BeAssignableTo<ComTypes.IDataObject>().Which;
ComTypes.IDataObject dataObject = Clipboard.GetDataObject().Should().BeAssignableTo<ComTypes.IDataObject>().Subject;
ComTypes.FORMATETC formatetc = new()
{
cfFormat = (short)DataFormats.GetFormat("testFormat").Id,
Expand All @@ -1011,38 +1025,45 @@ public unsafe void Clipboard_Deserialize_FromStream_Manually()
};
dataObject.GetData(ref formatetc, out ComTypes.STGMEDIUM medium);
HGLOBAL hglobal = (HGLOBAL)medium.unionmember;
Stream stream;
MemoryStream? stream = null;
try
{
void* buffer = PInvokeCore.GlobalLock(hglobal);
int size = (int)PInvokeCore.GlobalSize(hglobal);
byte[] bytes = new byte[size];
Marshal.Copy((nint)buffer, bytes, 0, size);
// this comes from DataObject.Composition.s_serializedObjectID
int index = 16;
stream = new MemoryStream(bytes, index, bytes.Length - index);
try
{
void* buffer = PInvokeCore.GlobalLock(hglobal);
int size = (int)PInvokeCore.GlobalSize(hglobal);
byte[] bytes = new byte[size];
Marshal.Copy((nint)buffer, bytes, 0, size);
// this comes from DataObject.Composition.s_serializedObjectID
int index = 16;
stream = new MemoryStream(bytes, index, bytes.Length - index);
}
finally
{
PInvokeCore.GlobalUnlock(hglobal);
}

stream.Should().NotBeNull();
// Use NrbfDecoder to decode the stream and rehydrate the type.
SerializationRecord record = NrbfDecoder.Decode(stream);
ClassRecord types = record.Should().BeAssignableTo<ClassRecord>().Subject;
types.HasMember("<JsonBytes>k__BackingField").Should().BeTrue();
types.HasMember("<InnerTypeAssemblyQualifiedName>k__BackingField").Should().BeTrue();
SZArrayRecord<byte> byteData = types.GetRawValue("<JsonBytes>k__BackingField").Should().BeAssignableTo<SZArrayRecord<byte>>().Subject;
string innerTypeAssemblyQualifiedName = types.GetRawValue("<InnerTypeAssemblyQualifiedName>k__BackingField").Should().BeOfType<string>().Subject;
TypeName.TryParse(innerTypeAssemblyQualifiedName, out TypeName? innerTypeName).Should().BeTrue();
TypeName checkedResult = innerTypeName.Should().BeOfType<TypeName>().Subject;
// These should not be the same since we take TypeForwardedFromAttribute name into account during serialization,
// which changes the assembly name.
typeof(SimpleTestData).AssemblyQualifiedName.Should().NotBe(checkedResult.AssemblyQualifiedName);
typeof(SimpleTestData).ToTypeName().Matches(checkedResult).Should().BeTrue();

JsonSerializer.Deserialize(byteData.GetArray(), typeof(SimpleTestData)).Should().BeEquivalentTo(testData);
}
finally
{
PInvokeCore.GlobalUnlock(hglobal);
stream?.Dispose();
}

stream.Should().NotBeNull();
// Use NrbfDecoder to decode the stream and rehydrate the type.
SerializationRecord record = NrbfDecoder.Decode(stream);
ClassRecord types = record.Should().BeAssignableTo<ClassRecord>().Which;
types.HasMember("<JsonBytes>k__BackingField").Should().BeTrue();
types.HasMember("<InnerTypeAssemblyQualifiedName>k__BackingField").Should().BeTrue();
SZArrayRecord<byte> byteData = types.GetRawValue("<JsonBytes>k__BackingField").Should().BeAssignableTo<SZArrayRecord<byte>>().Subject;
string innerTypeAssemblyQualifiedName = types.GetRawValue("<InnerTypeAssemblyQualifiedName>k__BackingField").Should().BeOfType<string>().Subject;
TypeName.TryParse(innerTypeAssemblyQualifiedName, out TypeName? innerTypeName).Should().BeTrue();
TypeName checkedResult = innerTypeName.Should().BeOfType<TypeName>().Subject;
// These should not be the same since we take TypeForwardedFromAttribute name into account during serialization,
// which changes the assembly name.
typeof(SimpleTestData).AssemblyQualifiedName.Should().NotBe(checkedResult.AssemblyQualifiedName);
typeof(SimpleTestData).ToTypeName().Matches(checkedResult).Should().BeTrue();

JsonSerializer.Deserialize(byteData.GetArray(), typeof(SimpleTestData)).Should().BeEquivalentTo(testData);
}

[WinFormsFact]
Expand Down

0 comments on commit 47e5d4e

Please sign in to comment.