maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

html: make sure we allocate enough space for the \0, always clo… (#3215)

Browse source 
conhost has been leaving the clipboard open for all HTML copies because
StringCchCopyA needs an extra byte for the null terminator and we
haven't been giving it one. We should also make sure that we always
close the clipboard (always).
This commit is contained in:
Dustin L. Howett (MSFT) 2019-10-17 14:56:59 -07:00 committed by Michael Niksa
parent d80500f112
commit a02a5d9986
1 changed files with 33 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

61
src/interactivity/win32/Clipboard.cpp
View file

@ -270,44 +270,49 @@ void Clipboard::CopyTextToSystemClipboard(const TextBuffer::TextAndColor& rows,
// Set global data to clipboard
THROW_LAST_ERROR_IF(!OpenClipboard(ServiceLocator::LocateConsoleWindow()->GetWindowHandle()));
THROW_LAST_ERROR_IF(!EmptyClipboard());
THROW_LAST_ERROR_IF_NULL(SetClipboardData(CF_UNICODETEXT, globalHandle.get()));
if (fAlsoCopyHtml)
{
const auto& fontData = ServiceLocator::LocateGlobals().getConsoleInformation().GetActiveOutputBuffer().GetCurrentFont();
int const iFontHeightPoints = fontData.GetUnscaledSize().Y * 72 / ServiceLocator::LocateGlobals().dpi;
const COLORREF bgColor = ServiceLocator::LocateGlobals().getConsoleInformation().GetDefaultBackground();
std::string HTMLToPlaceOnClip = TextBuffer::GenHTML(rows, iFontHeightPoints, fontData.GetFaceName(), bgColor, "Windows Console Host");
const size_t cbNeededHTML = HTMLToPlaceOnClip.size();
if (cbNeededHTML)
{ // Clipboard Scope
auto clipboardCloser = wil::scope_exit([]() {
THROW_LAST_ERROR_IF(!CloseClipboard());
});
THROW_LAST_ERROR_IF(!EmptyClipboard());
THROW_LAST_ERROR_IF_NULL(SetClipboardData(CF_UNICODETEXT, globalHandle.get()));
if (fAlsoCopyHtml)
{
wil::unique_hglobal globalHandleHTML(GlobalAlloc(GMEM_MOVEABLE | GMEM_DDESHARE, cbNeededHTML));
THROW_LAST_ERROR_IF_NULL(globalHandleHTML.get());
const auto& fontData = ServiceLocator::LocateGlobals().getConsoleInformation().GetActiveOutputBuffer().GetCurrentFont();
int const iFontHeightPoints = fontData.GetUnscaledSize().Y * 72 / ServiceLocator::LocateGlobals().dpi;
const COLORREF bgColor = ServiceLocator::LocateGlobals().getConsoleInformation().GetDefaultBackground();
std::string HTMLToPlaceOnClip = TextBuffer::GenHTML(rows, iFontHeightPoints, fontData.GetFaceName(), bgColor, "Windows Console Host");
const size_t cbNeededHTML = HTMLToPlaceOnClip.size() + 1;
if (cbNeededHTML)
{
wil::unique_hglobal globalHandleHTML(GlobalAlloc(GMEM_MOVEABLE | GMEM_DDESHARE, cbNeededHTML));
THROW_LAST_ERROR_IF_NULL(globalHandleHTML.get());
PSTR pszClipboardHTML = (PSTR)GlobalLock(globalHandleHTML.get());
THROW_LAST_ERROR_IF_NULL(pszClipboardHTML);
PSTR pszClipboardHTML = (PSTR)GlobalLock(globalHandleHTML.get());
THROW_LAST_ERROR_IF_NULL(pszClipboardHTML);
// The pattern gets a bit strange here because there's no good wil built-in for global lock of this type.
// Try to copy then immediately unlock. Don't throw until after (so the hglobal won't be freed until we unlock).
const HRESULT hr2 = StringCchCopyA(pszClipboardHTML, cbNeededHTML, HTMLToPlaceOnClip.data());
GlobalUnlock(globalHandleHTML.get());
THROW_IF_FAILED(hr2);
// The pattern gets a bit strange here because there's no good wil built-in for global lock of this type.
// Try to copy then immediately unlock. Don't throw until after (so the hglobal won't be freed until we unlock).
const HRESULT hr2 = StringCchCopyA(pszClipboardHTML, cbNeededHTML, HTMLToPlaceOnClip.data());
GlobalUnlock(globalHandleHTML.get());
THROW_IF_FAILED(hr2);
UINT const CF_HTML = RegisterClipboardFormatW(L"HTML Format");
THROW_LAST_ERROR_IF(0 == CF_HTML);
UINT const CF_HTML = RegisterClipboardFormatW(L"HTML Format");
THROW_LAST_ERROR_IF(0 == CF_HTML);
THROW_LAST_ERROR_IF_NULL(SetClipboardData(CF_HTML, globalHandleHTML.get()));
THROW_LAST_ERROR_IF_NULL(SetClipboardData(CF_HTML, globalHandleHTML.get()));
// only free if we failed.
// the memory has to remain allocated if we successfully placed it on the clipboard.
// Releasing the smart pointer will leave it allocated as we exit scope.
globalHandleHTML.release();
// only free if we failed.
// the memory has to remain allocated if we successfully placed it on the clipboard.
// Releasing the smart pointer will leave it allocated as we exit scope.
globalHandleHTML.release();
}
}
}
THROW_LAST_ERROR_IF(!CloseClipboard());
// only free if we failed.
// the memory has to remain allocated if we successfully placed it on the clipboard.
// Releasing the smart pointer will leave it allocated as we exit scope.