Skip to content

Commit

Permalink
Only unwrap given IDataObject
Browse files Browse the repository at this point in the history
  • Loading branch information
@lonitra
lonitra committed Jan 9, 2025
1 parent ce6b6c2 commit a4882d5
Show file tree
Hide file tree
Showing 6 changed files with 193 additions and 65 deletions.
2 changes: 1 addition & 1 deletion src/System.Private.Windows.Core/src/Windows/Win32/System/Com/ComHelpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ internal static bool SupportsInterface<T>(object? @object) where T : unmanaged,
/// <summary>
/// Attempts to unwrap a ComWrapper CCW as a particular managed object.
/// </summary>
private static bool TryUnwrapComWrapperCCW<TWrapper>(
public static bool TryUnwrapComWrapperCCW<TWrapper>(
IUnknown* unknown,
[NotNullWhen(true)] out TWrapper? @interface) where TWrapper : class
{
Expand Down
60 changes: 15 additions & 45 deletions src/System.Windows.Forms/src/System/Windows/Forms/OLE/Clipboard.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,13 @@ public static unsafe void SetDataObject(object data, bool copy, int retryTimes,
ArgumentOutOfRangeException.ThrowIfNegative(retryTimes);
ArgumentOutOfRangeException.ThrowIfNegative(retryDelay);

// Always wrap the data in our DataObject since we know how to retrieve our DataObject from the proxy OleGetClipboard returns.
DataObject wrappedData = data is DataObject { IsWrappedForClipboard: true } alreadyWrapped
? alreadyWrapped
: new DataObject(data) { IsWrappedForClipboard = true };
using var dataObject = ComHelpers.GetComScope<Com.IDataObject>(wrappedData);
// Always wrap the data if not already a DataObject. Mark whether the data is an IDataObject so we unwrap it properly on retrieval.
DataObject dataObject = data as DataObject ?? new DataObject(data) { IsOriginalNotIDataObject = data is not IDataObject };
using var iDataObject = ComHelpers.GetComScope<Com.IDataObject>(dataObject);

HRESULT hr;
int retry = retryTimes;
while ((hr = PInvoke.OleSetClipboard(dataObject)).Failed)
while ((hr = PInvoke.OleSetClipboard(iDataObject)).Failed)
{
if (--retry < 0)
{
Expand Down Expand Up @@ -111,52 +109,24 @@ public static unsafe void SetDataObject(object data, bool copy, int retryTimes,

// OleGetClipboard always returns a proxy. The proxy forwards all IDataObject method calls to the real data object,
// without giving out the real data object. If the data placed on the clipboard is not one of our CCWs or the clipboard
// has been flushed, marshal will create a wrapper around the proxy for us to use. However, if the data placed on
// has been flushed, a wrapper around the proxy for us to use will be given. However, if the data placed on
// the clipboard is one of our own and the clipboard has not been flushed, we need to retrieve the real data object
// pointer in order to retrieve the original managed object via ComWrappers. To do this, we must query for an
// interface that is not known to the proxy e.g. IComCallableWrapper. If we are able to query for IComCallableWrapper
// it means that the real data object is one of our CCWs and we've retrieved it successfully,
// pointer in order to retrieve the original managed object via ComWrappers if an IDataObject was set on the clipboard.
// To do this, we must query for an interface that is not known to the proxy e.g. IComCallableWrapper.
// If we are able to query for IComCallableWrapper it means that the real data object is one of our CCWs and we've retrieved it successfully,
// otherwise it is not ours and we will use the wrapped proxy.
IUnknown* target = default;
var realDataObject = proxyDataObject.TryQuery<IComCallableWrapper>(out hr);
if (hr.Succeeded)
{
target = realDataObject.AsUnknown;
}
else
{
target = proxyDataObject.AsUnknown;
}

if (!ComHelpers.TryGetObjectForIUnknown(target, out object? managedDataObject))
{
target->Release();
return null;
}

if (managedDataObject is not Com.IDataObject.Interface dataObject)
{
// We always wrap data set on the Clipboard in a DataObject, so if we do not have
// a IDataObject.Interface this means built-in com support is turned off and
// we have a proxy where there is no way to retrieve the original data object
// pointer from it likely because either the clipboard was flushed or the data on the
// clipboard is from another process. We need to mimic built-in com behavior and wrap the proxy ourselves.
// DataObject will ref count proxyDataObject properly to take ownership.
return new DataObject(proxyDataObject.Value);
}

if (dataObject is DataObject { IsWrappedForClipboard: true } wrappedData)
if (hr.Succeeded
&& ComHelpers.TryUnwrapComWrapperCCW(realDataObject.AsUnknown, out DataObject? dataObject)
&& !dataObject.IsOriginalNotIDataObject)
{
// There is a DataObject on the clipboard that we placed there. If the real data object
// implements IDataObject, we want to unwrap it and return it. Otherwise return
// the DataObject as is.
return wrappedData.TryUnwrapInnerIDataObject();
// An IDataObject was given to us to place on the clipboard. We want to unwrap and return it instead of a proxy.
return dataObject.TryUnwrapInnerIDataObject();
}

// We did not place the data on the clipboard. Fall back to old behavior.
return dataObject is IDataObject ido && !Marshal.IsComObject(dataObject)
? ido
: new DataObject(dataObject);
// Original data given wasn't an IDataObject, give the proxy value back.
return new DataObject(proxyDataObject.Value);
}

/// <summary>
Expand Down
6 changes: 3 additions & 3 deletions src/System.Windows.Forms/src/System/Windows/Forms/OLE/DataObject.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ public DataObject(object data)
internal DataObject(string format, bool autoConvert, object data) : this() => SetData(format, autoConvert, data);

/// <summary>
/// Flags that the original data was wrapped for clipboard purposes.
/// Flags that the original data was not a user passed <see cref="IDataObject"/>.
/// </summary>
internal bool IsWrappedForClipboard { get; init; }
internal bool IsOriginalNotIDataObject { get; init; }

/// <summary>
/// Returns the inner data that the <see cref="DataObject"/> was created with if the original data implemented
Expand All @@ -95,7 +95,7 @@ public DataObject(object data)
/// </summary>
internal IDataObject TryUnwrapInnerIDataObject()
{
Debug.Assert(IsWrappedForClipboard, "This method should only be used for clipboard purposes.");
Debug.Assert(!IsOriginalNotIDataObject, "This method should only be used for clipboard purposes.");
return _innerData.OriginalIDataObject is { } original ? original : this;
}

Expand Down
66 changes: 66 additions & 0 deletions src/System.Windows.Forms/tests/ComDisabledTests/ClipboardComTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@

#nullable enable

using System.Runtime.InteropServices;
using static System.Windows.Forms.TestUtilities.DataObjectTestHelpers;
using ComTypes = System.Runtime.InteropServices.ComTypes;

namespace System.Windows.Forms.Tests;

Expand Down Expand Up @@ -50,4 +52,68 @@ public void Clipboard_SetDataObject_WithJson_ReturnsExpected(bool copy)
returnedDataObject.TryGetData("testData", out SimpleTestData deserialized).Should().BeTrue();
deserialized.Should().BeEquivalentTo(testData);
}

[WinFormsFact]
public void Clipboard_GetSet_IDataObject_RoundTrip_ReturnsExpected()
{
CustomDataObject realDataObject = new();
Clipboard.SetDataObject(realDataObject);

IDataObject clipboardDataObject = Clipboard.GetDataObject().Should().BeAssignableTo<IDataObject>().Subject;
clipboardDataObject.Should().BeSameAs(realDataObject);
clipboardDataObject.GetDataPresent("Foo").Should().BeTrue();
clipboardDataObject.GetData("Foo").Should().Be("Bar");
}

[WinFormsFact]
public void Clipboard_SetDataObject_DerivedDataObject_ReturnsExpected()
{
DerivedDataObject derived = new();
Clipboard.SetDataObject(derived);
Clipboard.GetDataObject().Should().BeSameAs(derived);
}

private class DerivedDataObject : DataObject { }

private class CustomDataObject : IDataObject, ComTypes.IDataObject
{
[DllImport("shell32.dll")]
public static extern int SHCreateStdEnumFmtEtc(uint cfmt, ComTypes.FORMATETC[] afmt, out ComTypes.IEnumFORMATETC ppenumFormatEtc);

int ComTypes.IDataObject.DAdvise(ref ComTypes.FORMATETC pFormatetc, ComTypes.ADVF advf, ComTypes.IAdviseSink adviseSink, out int connection) => throw new NotImplementedException();
void ComTypes.IDataObject.DUnadvise(int connection) => throw new NotImplementedException();
int ComTypes.IDataObject.EnumDAdvise(out ComTypes.IEnumSTATDATA enumAdvise) => throw new NotImplementedException();
ComTypes.IEnumFORMATETC ComTypes.IDataObject.EnumFormatEtc(ComTypes.DATADIR direction)
{
if (direction == ComTypes.DATADIR.DATADIR_GET)
{
// Create enumerator and return it
ComTypes.IEnumFORMATETC enumerator;
if (SHCreateStdEnumFmtEtc(0, [], out enumerator) == 0)
{
return enumerator;
}
}

throw new NotImplementedException();
}

int ComTypes.IDataObject.GetCanonicalFormatEtc(ref ComTypes.FORMATETC formatIn, out ComTypes.FORMATETC formatOut) => throw new NotImplementedException();
object IDataObject.GetData(string format, bool autoConvert) => format == "Foo" ? "Bar" : null!;
object IDataObject.GetData(string format) => format == "Foo" ? "Bar" : null!;
object IDataObject.GetData(Type format) => null!;
void ComTypes.IDataObject.GetData(ref ComTypes.FORMATETC format, out ComTypes.STGMEDIUM medium) => throw new NotImplementedException();
void ComTypes.IDataObject.GetDataHere(ref ComTypes.FORMATETC format, ref ComTypes.STGMEDIUM medium) => throw new NotImplementedException();
bool IDataObject.GetDataPresent(string format, bool autoConvert) => format == "Foo";
bool IDataObject.GetDataPresent(string format) => format == "Foo";
bool IDataObject.GetDataPresent(Type format) => false;
string[] IDataObject.GetFormats(bool autoConvert) => ["Foo"];
string[] IDataObject.GetFormats() => ["Foo"];
int ComTypes.IDataObject.QueryGetData(ref ComTypes.FORMATETC format) => throw new NotImplementedException();
void IDataObject.SetData(string format, bool autoConvert, object? data) => throw new NotImplementedException();
void IDataObject.SetData(string format, object? data) => throw new NotImplementedException();
void IDataObject.SetData(Type format, object? data) => throw new NotImplementedException();
void IDataObject.SetData(object? data) => throw new NotImplementedException();
void ComTypes.IDataObject.SetData(ref ComTypes.FORMATETC formatIn, ref ComTypes.STGMEDIUM medium, bool release) => throw new NotImplementedException();
}
}
120 changes: 108 additions & 12 deletions src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ClipboardTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,19 +574,11 @@ public unsafe void Clipboard_GetClipboard_ReturnsProxy()
}

[WinFormsFact]
public void Clipboard_Set_DoesNotWrapTwice()
public void Clipboard_SetDataObject_DerivedDataObject_ReturnsExpected()
{
string realDataObject = string.Empty;
Clipboard.SetDataObject(realDataObject);

IDataObject? clipboardDataObject = Clipboard.GetDataObject();
var dataObject = clipboardDataObject.Should().BeOfType<DataObject>().Subject;
dataObject.IsWrappedForClipboard.Should().BeTrue();

Clipboard.SetDataObject(clipboardDataObject!);
IDataObject? clipboardDataObject2 = Clipboard.GetDataObject();
clipboardDataObject2.Should().NotBeNull();
clipboardDataObject2.Should().BeSameAs(clipboardDataObject);
DerivedDataObject derived = new();
Clipboard.SetDataObject(derived);
Clipboard.GetDataObject().Should().BeSameAs(derived);
}

[WinFormsFact]
Expand Down Expand Up @@ -664,6 +656,16 @@ public unsafe void Clipboard_RawClipboard_SetClipboardData_ReturnsExpected()

DataObject dataObject = Clipboard.GetDataObject().Should().BeOfType<DataObject>().Subject;
dataObject.GetData(DataFormats.Text).Should().Be(testString);

Clipboard.ContainsText().Should().BeTrue();
Clipboard.ContainsData(DataFormats.Text).Should().BeTrue();
Clipboard.ContainsData(DataFormats.UnicodeText).Should().BeTrue();

Clipboard.GetText().Should().Be(testString);
Clipboard.GetText(TextDataFormat.Text).Should().Be(testString);
Clipboard.GetText(TextDataFormat.UnicodeText).Should().Be(testString);

Clipboard.GetData("System.String").Should().BeNull();
}

[WinFormsFact]
Expand Down Expand Up @@ -981,6 +983,16 @@ public void Clipboard_SetDataObject_WithJson_ReturnsExpected(bool copy)
ITypedDataObject returnedDataObject = Clipboard.GetDataObject().Should().BeAssignableTo<ITypedDataObject>().Subject;
returnedDataObject.TryGetData("testDataFormat", out SimpleTestData deserialized).Should().BeTrue();
deserialized.Should().BeEquivalentTo(testData);
// We don't expose JsonData<T> in legacy API
var legacyResult = Clipboard.GetData("testDataFormat");
if (copy)
{
legacyResult.Should().BeOfType<MemoryStream>();
}
else
{
legacyResult.Should().BeNull();
}
}

[WinFormsTheory]
Expand Down Expand Up @@ -1170,4 +1182,88 @@ public void Clipboard_SetDataAsJson_NullData_Throws()
Action clipboardSet = () => Clipboard.SetDataAsJson<string>("format", null!);
clipboardSet.Should().Throw<ArgumentNullException>();
}

[WinFormsFact]
public void Clipboard_SetData_Text_Format_AllUpper()
{
// The fact that casing on input matters is likely incorrect, but behavior has been this way.
Clipboard.SetData("TEXT", "Hello, World!");
Clipboard.ContainsText().Should().BeTrue();
Clipboard.ContainsData("TEXT").Should().BeTrue();
Clipboard.ContainsData(DataFormats.Text).Should().BeTrue();
Clipboard.ContainsData(DataFormats.UnicodeText).Should().BeTrue();

IDataObject dataObject = Clipboard.GetDataObject().Should().BeAssignableTo<IDataObject>().Subject;
string[] formats = dataObject.GetFormats();
formats.Should().BeEquivalentTo(["System.String", "UnicodeText", "Text"]);

formats = dataObject.GetFormats(autoConvert: false);
formats.Should().BeEquivalentTo(["Text"]);

// CLIPBRD_E_BAD_DATA returned when trying to get clipboard data.
Clipboard.GetText().Should().BeEmpty();
Clipboard.GetText(TextDataFormat.Text).Should().BeEmpty();
Clipboard.GetText(TextDataFormat.UnicodeText).Should().BeEmpty();

Clipboard.GetData("System.String").Should().BeNull();
Clipboard.GetData("TEXT").Should().BeNull();
}

[WinFormsFact]
public void Clipboard_SetData_Text_Format_CanonicalCase()
{
string expected = "Hello, World!";
Clipboard.SetData("Text", expected);
Clipboard.ContainsText().Should().BeTrue();
Clipboard.ContainsData("TEXT").Should().BeTrue();
Clipboard.ContainsData(DataFormats.Text).Should().BeTrue();
Clipboard.ContainsData(DataFormats.UnicodeText).Should().BeTrue();

IDataObject dataObject = Clipboard.GetDataObject().Should().BeAssignableTo<IDataObject>().Subject;
string[] formats = dataObject.GetFormats();
formats.Should().BeEquivalentTo(["System.String", "UnicodeText", "Text"]);

formats = dataObject.GetFormats(autoConvert: false);
formats.Should().BeEquivalentTo(["System.String", "UnicodeText", "Text"]);

Clipboard.GetText().Should().Be(expected);
Clipboard.GetText(TextDataFormat.Text).Should().Be(expected);
Clipboard.GetText(TextDataFormat.UnicodeText).Should().Be(expected);

Clipboard.GetData("System.String").Should().Be(expected);

// Case sensitivity matters so we end up reading stream/object from HGLOBAL instead of string.
MemoryStream stream = Clipboard.GetData("TEXT").Should().BeOfType<MemoryStream>().Subject;
byte[] array = stream.ToArray();
array.Should().BeEquivalentTo("Hello, World!\0"u8.ToArray());
}

[WinFormsFact]
public void Clipboard_SetDataObject_Text()
{
string expected = "Hello, World!";
Clipboard.SetDataObject(expected);
Clipboard.ContainsText().Should().BeTrue();
Clipboard.ContainsData("TEXT").Should().BeTrue();
Clipboard.ContainsData(DataFormats.Text).Should().BeTrue();
Clipboard.ContainsData(DataFormats.UnicodeText).Should().BeTrue();

IDataObject dataObject = Clipboard.GetDataObject().Should().BeAssignableTo<IDataObject>().Subject;
string[] formats = dataObject.GetFormats();
formats.Should().BeEquivalentTo(["System.String", "UnicodeText", "Text"]);

formats = dataObject.GetFormats(autoConvert: false);
formats.Should().BeEquivalentTo(["System.String", "UnicodeText", "Text"]);

Clipboard.GetText().Should().Be(expected);
Clipboard.GetText(TextDataFormat.Text).Should().Be(expected);
Clipboard.GetText(TextDataFormat.UnicodeText).Should().Be(expected);

Clipboard.GetData("System.String").Should().Be(expected);

// Case sensitivity matters so we end up reading stream/object from HGLOBAL instead of string.
MemoryStream stream = Clipboard.GetData("TEXT").Should().BeOfType<MemoryStream>().Subject;
byte[] array = stream.ToArray();
array.Should().BeEquivalentTo("Hello, World!\0"u8.ToArray());
}
}
4 changes: 0 additions & 4 deletions src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/DataObjectTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3032,10 +3032,6 @@ public void DataObject_SetDataAsJson_NonRestrictedFormat_JsonSerialized(string f

// We don't expose JsonData<T> in public legacy API
data.GetData(format).Should().BeNull();

// For the clipboard, we don't expose JsonData<T> either for in proc scenarios.
Clipboard.SetDataObject(data, copy: false);
Clipboard.GetData(format).Should().BeNull();
}

[WinFormsFact]
Expand Down

0 comments on commit a4882d5

Please sign in to comment.