From cff2580bc6b540ff1bb9b890d9cf79c1bcc563a5 Mon Sep 17 00:00:00 2001 From: David P Date: Fri, 16 May 2025 19:08:45 +0000 Subject: [PATCH] Bug 1966443: Use proper length for Windows clipboard elements r=win-reviewers,emilio,gstoll NULLs are expected in the clipboard data. Clipboard code tries to use spans in a lot of places but there is a lot of old string stuff remaining as well. Differential Revision: https://phabricator.services.mozilla.com/D249702 --- widget/windows/nsClipboard.cpp | 50 +++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/widget/windows/nsClipboard.cpp b/widget/windows/nsClipboard.cpp index 8c53b0ae3cb6..575be7accd43 100644 --- a/widget/windows/nsClipboard.cpp +++ b/widget/windows/nsClipboard.cpp @@ -578,6 +578,7 @@ NS_IMETHODIMP nsClipboard::SetNativeClipboardData( } //------------------------------------------------------------------------- +/* static */ nsresult nsClipboard::GetGlobalData(HGLOBAL aHGBL, void** aData, uint32_t* aLen) { MOZ_CLIPBOARD_LOG("%s", __FUNCTION__); @@ -700,6 +701,29 @@ HRESULT nsClipboard::FillSTGMedium(IDataObject* aDataObject, UINT aFormat, } //------------------------------------------------------------------------- +// Get the data out of the global data handle. The size we +// return should be the size returned from GetGlobalData(), since the +// string returned from Windows may have nulls inserted -- it may not +// even be null-terminated. GetGlobalData adds bytes for null +// termination to the buffer but they are not considered in the returned +// byte count. We check and skip the last counted byte if it is a null +// since Windows also appears to add null termination. See GetGlobalData. +template +static nsresult GetCharDataFromGlobalData(STGMEDIUM& aStm, CharType** aData, + uint32_t* aLen) { + uint32_t nBytes = 0; + MOZ_TRY(nsClipboard::GetGlobalData(aStm.hGlobal, + reinterpret_cast(aData), &nBytes)); + auto nChars = nBytes / sizeof(CharType); + if (nChars < 1) { + *aLen = 0; + return NS_OK; + } + bool hasNullTerminator = (*aData)[nChars - 1] == CharType(0); + *aLen = hasNullTerminator ? nBytes - sizeof(CharType) : nBytes; + return NS_OK; +} + // If aFormat is CF_DIBV5, aMIMEImageFormat must be a type for which we have // an image encoder (e.g. image/png). // For other values of aFormat, it is OK to pass null for aMIMEImageFormat. @@ -764,31 +788,13 @@ nsresult nsClipboard::GetNativeDataOffClipboard(IDataObject* aDataObject, // compile-time-constant format indicators: switch (fe.cfFormat) { case CF_TEXT: { - // Get the data out of the global data handle. The size we - // return should not include the null because the other - // platforms don't use nulls, so just return the length we get - // back from strlen(), since we know CF_TEXT is null - // terminated. Recall that GetGlobalData() returns the size of - // the allocated buffer, not the size of the data (on 98, these - // are not the same) so we can't use that. - uint32_t allocLen = 0; - MOZ_TRY(GetGlobalData(stm.hGlobal, aData, &allocLen)); - *aLen = strlen(reinterpret_cast(*aData)); - return NS_OK; + return GetCharDataFromGlobalData(stm, reinterpret_cast(aData), + aLen); } case CF_UNICODETEXT: { - // Get the data out of the global data handle. The size we - // return should not include the null because the other - // platforms don't use nulls, so just return the length we get - // back from strlen(), since we know CF_UNICODETEXT is null - // terminated. Recall that GetGlobalData() returns the size of - // the allocated buffer, not the size of the data (on 98, these - // are not the same) so we can't use that. - uint32_t allocLen = 0; - MOZ_TRY(GetGlobalData(stm.hGlobal, aData, &allocLen)); - *aLen = NS_strlen(reinterpret_cast(*aData)) * 2; - return NS_OK; + return GetCharDataFromGlobalData(stm, reinterpret_cast(aData), + aLen); } case CF_DIBV5: {