diff --git a/src/System.Private.Windows.Core/src/Windows/Win32/System/Com/ComHelpers.cs b/src/System.Private.Windows.Core/src/Windows/Win32/System/Com/ComHelpers.cs index b29b447eaa9..e2d63a8dd96 100644 --- a/src/System.Private.Windows.Core/src/Windows/Win32/System/Com/ComHelpers.cs +++ b/src/System.Private.Windows.Core/src/Windows/Win32/System/Com/ComHelpers.cs @@ -132,7 +132,7 @@ internal static bool SupportsInterface(object? @object) where T : unmanaged, /// /// Attempts to unwrap a ComWrapper CCW as a particular managed object. /// - private static bool TryUnwrapComWrapperCCW( + public static bool TryUnwrapComWrapperCCW( IUnknown* unknown, [NotNullWhen(true)] out TWrapper? @interface) where TWrapper : class { diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/OLE/Clipboard.cs b/src/System.Windows.Forms/src/System/Windows/Forms/OLE/Clipboard.cs index 5e05add2412..d2364923796 100644 --- a/src/System.Windows.Forms/src/System/Windows/Forms/OLE/Clipboard.cs +++ b/src/System.Windows.Forms/src/System/Windows/Forms/OLE/Clipboard.cs @@ -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(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(dataObject); HRESULT hr; int retry = retryTimes; - while ((hr = PInvoke.OleSetClipboard(dataObject)).Failed) + while ((hr = PInvoke.OleSetClipboard(iDataObject)).Failed) { if (--retry < 0) { @@ -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(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); } /// diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/OLE/DataObject.cs b/src/System.Windows.Forms/src/System/Windows/Forms/OLE/DataObject.cs index d5d589a5375..160dd95f12b 100644 --- a/src/System.Windows.Forms/src/System/Windows/Forms/OLE/DataObject.cs +++ b/src/System.Windows.Forms/src/System/Windows/Forms/OLE/DataObject.cs @@ -84,9 +84,9 @@ public DataObject(object data) internal DataObject(string format, bool autoConvert, object data) : this() => SetData(format, autoConvert, data); /// - /// Flags that the original data was wrapped for clipboard purposes. + /// Flags that the original data was not a user passed . /// - internal bool IsWrappedForClipboard { get; init; } + internal bool IsOriginalNotIDataObject { get; init; } /// /// Returns the inner data that the was created with if the original data implemented @@ -95,7 +95,7 @@ public DataObject(object data) /// 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; } diff --git a/src/System.Windows.Forms/tests/ComDisabledTests/ClipboardComTests.cs b/src/System.Windows.Forms/tests/ComDisabledTests/ClipboardComTests.cs index f6182184488..58ed6e9930e 100644 --- a/src/System.Windows.Forms/tests/ComDisabledTests/ClipboardComTests.cs +++ b/src/System.Windows.Forms/tests/ComDisabledTests/ClipboardComTests.cs @@ -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; @@ -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().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(); + } } diff --git a/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ClipboardTests.cs b/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ClipboardTests.cs index d9e660e423a..bed601d039f 100644 --- a/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ClipboardTests.cs +++ b/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ClipboardTests.cs @@ -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().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] @@ -664,6 +656,16 @@ public unsafe void Clipboard_RawClipboard_SetClipboardData_ReturnsExpected() DataObject dataObject = Clipboard.GetDataObject().Should().BeOfType().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] @@ -981,6 +983,16 @@ public void Clipboard_SetDataObject_WithJson_ReturnsExpected(bool copy) ITypedDataObject returnedDataObject = Clipboard.GetDataObject().Should().BeAssignableTo().Subject; returnedDataObject.TryGetData("testDataFormat", out SimpleTestData deserialized).Should().BeTrue(); deserialized.Should().BeEquivalentTo(testData); + // We don't expose JsonData in legacy API + var legacyResult = Clipboard.GetData("testDataFormat"); + if (copy) + { + legacyResult.Should().BeOfType(); + } + else + { + legacyResult.Should().BeNull(); + } } [WinFormsTheory] @@ -1170,4 +1182,88 @@ public void Clipboard_SetDataAsJson_NullData_Throws() Action clipboardSet = () => Clipboard.SetDataAsJson("format", null!); clipboardSet.Should().Throw(); } + + [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().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().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().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().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().Subject; + byte[] array = stream.ToArray(); + array.Should().BeEquivalentTo("Hello, World!\0"u8.ToArray()); + } } diff --git a/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/DataObjectTests.cs b/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/DataObjectTests.cs index 72155782228..305698466de 100644 --- a/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/DataObjectTests.cs +++ b/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/DataObjectTests.cs @@ -3032,10 +3032,6 @@ public void DataObject_SetDataAsJson_NonRestrictedFormat_JsonSerialized(string f // We don't expose JsonData in public legacy API data.GetData(format).Should().BeNull(); - - // For the clipboard, we don't expose JsonData either for in proc scenarios. - Clipboard.SetDataObject(data, copy: false); - Clipboard.GetData(format).Should().BeNull(); } [WinFormsFact]