Winforms: Regression: Clipboard.GetText("text", TextDataFormat.Html) adds garbage terminator

Created on 20 Sep 2019  路  7Comments  路  Source: dotnet/winforms

Test code from https://github.com/dotnet/winforms/pull/1950

[StaTheory]
[CommonMemberData(nameof(CommonTestHelper.GetEnumTypeTheoryData), typeof(TextDataFormat))]
public void Clipboard_SetText_InvokeStringTextDataFormat_GetReturnsExpected(TextDataFormat format)
{
    Clipboard.SetText("text", format);
    Assert.Equal("text", Clipboard.GetText(format));
    Assert.True(Clipboard.ContainsText(format));
}

Results:

xUnit.net Console Runner v2.4.1 (64-bit .NET Core 5.0.0-alpha1.19468.6)
  Discovering: System.Windows.Forms.Tests
  Discovered:  System.Windows.Forms.Tests
  Starting:    System.Windows.Forms.Tests
    System.Windows.Forms.Tests.ClipboardTests.Clipboard_SetText_InvokeStringTextDataFormat_GetReturnsExpected(format: Html) [FAIL]
      Assert.Equal() Failure
                     (pos 4)
      Expected: text
      Actual:   text\0
                     (pos 4)
      Stack Trace:
        /_/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/ClipboardTests.cs(452,0): at System.Windows.Forms.Tests.ClipboardTests.Clipboard_SetText_InvokeStringTextDataFormat_GetReturnsExpected(TextDataFormat format)
  Finished:    System.Windows.Forms.Tests
=== TEST EXECUTION SUMMARY ===
   System.Windows.Forms.Tests  Total: 12926, Errors: 0, Failed: 1, Skipped: 0, Time: 10.469s
=== COMMAND LINE ===
"D:\a\1\s\.dotnet\dotnet.exe" exec --depsfile "D:\a\1\s\artifacts\bin\System.Windows.Forms.Tests\Release\netcoreapp5.0\System.Windows.Forms.Tests.deps.json" --runtimeconfig "D:\a\1\s\artifacts\bin\System.Windows.Forms.Tests\Release\netcoreapp5.0\System.Windows.Forms.Tests.runtimeconfig.json"  "D:\a\1\s\.packages\xunit.runner.console/2.4.1/tools/netcoreapp2.0/xunit.console.dll" "D:\a\1\s\artifacts\bin\System.Windows.Forms.Tests\Release\netcoreapp5.0\System.Windows.Forms.Tests.dll" -noautoreporters -xml "D:\a\1\s\artifacts\TestResults\Release\System.Windows.Forms.Tests_netcoreapp5.0_x64.xml" -html "D:\a\1\s\artifacts\TestResults\Release\System.Windows.Forms.Tests_netcoreapp5.0_x64.html"  > "D:\a\1\s\artifacts\log\Release\System.Windows.Forms.Tests_netcoreapp5.0_x64.log" 2>&1

In .NET Framework the string correctly roundtrips and does not add '\0' at the end

/cc @JeremyKuhne

bug regression

All 7 comments

Looks like an interop mistake. Guessing it is GetText not SetText. Setting the wrong size of the string.

Possibly related:
The .NET Framework code has extra checks (https://referencesource.microsoft.com/#System.Windows.Forms/winforms/Managed/System/WinForms/DataObject.cs,fd8117b6b59dbe77)

            else if (format.Equals(DataFormats.Html)) {
                if (WindowsFormsUtils.TargetsAtLeast_v4_5) {
                    hr = SaveHtmlToHandle(medium.unionmember, data.ToString());
                }
                else {
                    // This will return UTF-8 strings as an array of ANSI characters, which makes it the wrong behavior.
                    // Since there are enough samples online how to workaround that, we will continue to return the
                    // incorrect value to applications targeting netfx 4.0
                    // DevDiv2 bug 862524
                    hr = SaveStringToHandle(medium.unionmember, data.ToString(), false);
                }
            }

Just a quick look- this could be wrong:

https://github.com/dotnet/winforms/blob/ab6730e76538c28f598bbac2bb009725e9ab69bc/src/System.Windows.Forms/src/System/Windows/Forms/DataObject.cs#L1689

It used to not pass the length, which would automatically null terminate. Worth testing in any case.

Fyi, it is a common mistake to switch from a string building API that wasn't sized to a sized one and end up with garbage at the end. I've seen this with moving from T* to Span<T> a few times.

Definitely want to get this fix in the next possible bug-fix train.

Changed the title to reflect that it is GetText that has the issue.

Verified with the .Net Core SDK 3.0.100-rc1-014190 from release branch, the issue has been fixed.

Was this page helpful?
0 / 5 - 0 ratings