Repository: microsoft/terminal (language: cpp)

Issue:
Paste from clipboard doesn't always work
### Windows Terminal version

1.15.2874.0

### Windows build number

10.0.19044.2130

### Other Software

Old Intel workbench internal tool
Notepad++ v8.4.1

### Steps to reproduce

Copy some text out of the old workbench tool and attempt to paste it into Terminal. Nothing is pasted. I *can* paste into Notepad++. If I do, I can then copy that text from Notepad++ and then paste it into terminal. Without knowing any details, it seems to me that Notepad++ is supporting some older clipboard methodology that is not present in Terminal. Please add whatever it is that is missing in Terminal.

### Expected Behavior

What was copied should have been pasted into Terminal as input.

### Actual Behavior

Nothing is pasted when data is copied from that old app.


Discussion / hints:
What application is running in the Terminal that you're pasting into? How are you pasting (<kbd>ctrl+shift+v</kbd>, right mouse button, from the command palette...)/?

Without knowing more about the application in question, I'm not sure how we'd be able to figure out what "older clipboard methodology" they're using which we're apparently not supporting. Maybe they're setting a specific type of clipboard format (RTF? HTML?) that we're not looking for, but notepad++ can?
> On Nov 2, 2022, at 2:46 PM, Mike Griese ***@***.***> wrote:
> 
> What application is running in the Terminal that you're pasting into?

It is an old IDE used for microcode development, maybe based on a very old visual studio? Anyway, very old.

> How are you pasting (ctrl+shift+v, right mouse button, from the command palette...)/?

ctrl+shift+v, which does work after I copy from Notepad++. Which is able to accept the paste from the old app. That is what led me to report this on Terminal rather than grouse about the old program (there are always old programs).

> Without knowing more about the application in question, I'm not sure how we'd be able to figure out what "older clipboard methodology" they're using which we're apparently not supporting. Maybe they're setting a specific type of clipboard format (RTF? HTML?) that we're not looking for, but notepad++ can?

It really can't be anything other than plain text. I know nothing about Windows development, so I can really only observe the difference in behavior between Terminal and Notepad++ for example.

-- 
Mark Rustad (he/him), Ethernet Products Group, Intel Corporation


> On Nov 2, 2022, at 2:46 PM, Mike Griese ***@***.***> wrote:
> 
> Without knowing more about the application in question, I'm not sure how we'd be able to figure out what "older clipboard methodology" they're using which we're apparently not supporting. Maybe they're setting a specific type of clipboard format (RTF? HTML?) that we're not looking for, but notepad++ can?

I had a thought on this. I'm sure that the old app predates UTF-8, which I expect any modern program to be using for text. The program is at least 20 years old, so UTF-8 really wasn't a thing back then. Presumably it would indicate some older encoding. ASCII or whatever PC standard was called (I used to at least know that).

A wild guess. Like I said, I know nothing about Windows apps, but it is a thought.

-- 
Mark Rustad (he/him), Ethernet Products Group, Intel Corporation


Our leading suspicion is #14129 but we can't be 100% sure. Since we're hoping to release a patch for this, we'll circle back and see if that fixes it.

> On Nov 7, 2022, at 1:17 PM, Carlos Zamora ***@***.***> wrote:
> 
> Our leading suspicion is #14129 but we can't be 100% sure. Since we're hoping to release a patch for this, we'll circle back and see if that fixes it.

I was going to say that it didn't seem t be related until I got to the part about uninitialized memory! Anyway, I tried the showkeys -a thing that was mentioned and it returned nothing when I pasted a clipbaord form that old app. Also, FWIW, it also won't paste into a Windows shell in Terminal, but I can into cmd.exe.

So unlikely related to bracketed paste, but possibly somehow related to uninitialized memory (since that could be most anything). Or not.

-- 
Mark Rustad (he/him), Ethernet Products Group, Intel Corporation


>  Anyway, I tried the `showkeys -a` thing that was mentioned and it returned nothing when I pasted a clipbaord form that old app. Also, FWIW, it also won't paste into a Windows shell in Terminal, but I can into cmd.exe.

Well, shiz. That rules out the uninitialized memory one. Basically, whatever data is getting onto the clipboard, Terminal can't get out with 

https://github.com/microsoft/terminal/blob/a798a603e1256b2823483b0459077df97ff0eb3a/src/cascadia/TerminalApp/TerminalPage.cpp#L2173-L2176

[`Clipboard.GetContent` docs](https://learn.microsoft.com/en-us/uwp/api/windows.applicationmodel.datatransfer.clipboard.getcontent?view=winrt-22621)

I'm kinda lost at this point. Without the application that repros this, I don't know how we could possibly investigate more. Maybe a Feedback Hub trace, with the area being the "clipboard" might get more info, but I wouldn't know what part of that to dig through. 
We would basically need to know what the format of the clipboard content was, to be able to look for it. [more docs](https://learn.microsoft.com/en-us/uwp/api/Windows.ApplicationModel.DataTransfer.StandardDataFormats?view=winrt-22621#remarks).

Also curious: https://github.com/microsoft/terminal/blob/a798a603e1256b2823483b0459077df97ff0eb3a/src/interactivity/win32/Clipboard.cpp#L67-L72

the vintage console (what you're thinking of when you think `cmd.exe`) is only ever looking for `CF_UNICODETEXT`, which I'm under the impression is the same as `StandardDataFormats::Text()`
> On Nov 8, 2022, at 7:56 AM, Mike Griese ***@***.***> wrote:
> 
> the vintage console (what you're thinking of when you think cmd.exe) is only ever looking for CF_UNICODETEXT, which I'm under the impression is the same as StandardDataFormats::Text()

Maybe somewhere along the line the ability to convert 8-bit chars, ala ASCII, UTF-8, etc., into WCHAR was missed? It seems likely to be in the libraries somewhere. I don't know what is different between cmd and Terminal, but there should be a clue there as to what ingredient has the issue. It seems to me like Terminal would be using the newest stuff and newer is not always better.

Hmm. One other thing I should mention is that I am accessing this system via Remote Desktop from a Mac. Though in this case the copying and pasting is between two apps on the target, maybe the remote access has an effect on the clipboard somehow?

-- 
Mark Rustad (he/him), Ethernet Products Group, Intel Corporation


> accessing this system via Remote Desktop from a Mac

That certainly might have something to do with it! Some rudimentary googling suggests [restarting `rdpclip.exe`](http://woshub.com/cant-copy-paste-via-remote-desktop-rdp-clipboard/). No idea if that would actually help, but the modern app platform has definitely taken dependencies on unexpected parts of the OS in the past.


Maybe, but I kind of doubt it. I can copy/paste between everything else successfully. It is just copying from that old workbench app into Terminal in the same system that doesn't work. Recall I can paste into Notepad++ in the same environment. It is only Terminal that has the problem. The regular command prompt also receives the paste just fine. All accessed via RDP on the same system.
FWIW, I just tested this on a system without RDP involved and got the same result. I am unable to paste into Terminal that which was copied from a text window in that old IDE. The paste works fine in the traditional command prompt, Notepad++ and even Word. Only the new Terminal seems to have the problem.
Does pasting into any other UWP-like apps work? Something like... the builtin Mail or Calendar apps (I think they're UWPs). Sticky Notes might be a built-in UWP too. Any of those _should_ be using the modern clipboard APIs. 

I know it's an internal tool, but with the name of it, I might be able to search our internal bug database for issues, if any had been opened before. 

Another theory - perhaps this internal tool is using EDP to try and protect the clipboard contents, and Terminal is either obeying / not obeying those APIs. I know that's something we added support to the console in the past, but I don't think we ever did anything for the Terminal. 


Found on an internal thread

> Clipboard does not support UTF-8. Apps are doing a workaround of using CF_UNICODETEXT and doing manual conversion from UTF-8 to UTF-16. 

Interesting. I wonder if the tool you're using is setting the content as UTF-8,


... but honestly that doesn't sound right at all...
From my contact on the Clipboard team: 

> We would need clipboard logs from feedback hub repro under the "Copy and Paste" node to diagnose. 
>
> They could try using free clipboard viewer to see what formats are placed on the clipboard: 
> [Free Clipboard Viewer for Windows - Download](https://freeclipboardviewer.com/) 


CAVEAT: This isn't an application from Microsoft. I honestly don't know how legit it is. But they did mention it has been used to view clipboard content since they have worked in the area

But if you file /feedback under "Copy and Paste", and share the aka.ms link here, I can make sure it gets in the right folks hands. 

Hi there!<br><br>Can you please send us feedback with the [Feedback Hub](https://support.microsoft.com/en-us/windows/send-feedback-to-microsoft-with-the-feedback-hub-app-f59187f8-8739-22d6-ba93-f66612949332) with this issue? Make sure to click the "Start recording" button, then reproduce the issue before submitting the feedback. Once it's submitted, paste the link here so we can more easily find your crash information on the back end?<br><br>Thanks!<br><br>![image](https://user-images.githubusercontent.com/18356694/140811502-a068f78b-89d2-4587-925a-73e19652b830.png)<br><br>![image](https://user-images.githubusercontent.com/18356694/140811557-cdc22a0f-fa6a-4f6a-953e-73b51f5548a3.png)<br><br>![image](https://user-images.githubusercontent.com/18221333/62478649-6de55400-b760-11e9-806e-5aab7e085a9f.png)
> Does pasting into any other UWP-like apps work? Something like... the builtin Mail or Calendar apps (I think they're UWPs). Sticky Notes might be a built-in UWP too. Any of those _should_ be using the modern clipboard APIs.

I can confirm that it also doesn't work in Sticky Notes.

> I know it's an internal tool, but with the name of it, I might be able to search our internal bug database for issues, if any had been opened before.

The name of it is "Developer Workbench".

> Another theory - perhaps this internal tool is using EDP to try and protect the clipboard contents, and Terminal is either obeying / not obeying those APIs. I know that's something we added support to the console in the past, but I don't think we ever did anything for the Terminal.

I have no idea and no way to know.

I did run the Feedback Hub. The link is: https://aka.ms/AAj021g
Subscribed since I’ve been having the same issue.  If a block of text is already selected in the terminal this will remain in the clipboard (clip1) buffer even after selecting text from another application but only in the terminal clipboard (terminal1), so after you’ve copied (clip2) from another application and try to paste it to terminal1 you end up pasting the text from clip1. 

Is there an option to clear the clipboard buffer if text is copied from another application? Putty clears the clipboard before copying form another application.

Windows Terminal Version: 1.15.3465.0  

`"copyOnSelect": true,`

clipboard test
https://youtu.be/r2bklE0naT4
Hmm. Your issue is different, though I have observed that behavior, as well. In my case there is no selection in Terminal to override the clipboard contents for the pastes that aren't working for me. I hadn't yet thought to complain about your issue because this other issue causes me much more grief.
@4nqyi You're probably seeing the same thing as #14464 tbh
Pre-emptively, did you make sure to capture the recording with the Feedback Hub when submitting feedback? I don't think I'd be able to do anything with the logs if they're there, BUT I'm also not seeing the normal kinds of logs attached. Again, maybe clipboard tracing is attached differently, but I'm guessing it didn't get attached 😞 
Like I said, I ran the Feedback Hub and it gave me this link: https://aka.ms/AAj021g
I was going to open a new bug report but found this one.
I have the exact same behavior, except in my case it's not an "Old app" rather a WSLg window.

### Reproducing:
I'm running Windows 11 with Microsoft store latest WSL - that includes WSLg.
1. I open Eclipse CDT (latest) from within a WSL terminal window (running CentOS 7).
2. I copy text from Eclipse.
3. Trying to paste to the WSL terminal window fails - nothing pasted.
4. Also nothing pasted to ANY Windows Terminal tab I open (CMD, Powershell, etc.),
neither to a new Terminal window I open.
#### But, it will paste to:
1. The find box of Windows Terminal
![image](https://user-images.githubusercontent.com/11839190/214826717-c09756f2-cae9-48b3-a162-79e42a2bba99.png)
2. To a `conhost.exe` windows started from windows "Run" bar.
![image](https://user-images.githubusercontent.com/11839190/214826991-5dcd9819-df97-45c5-aba6-861e99a03c31.png)
3. To `Notepad`


I was having a similar issue with WSL and discovered that pasting text with a leading tab character, or even manually entering a tab key at the command prompt will crash the terminal.  Ctl-C does not clear or recover.  ^C is displayed, but no other keys are accepted.  You must close this window and open another.
> I was having a similar issue with WSL and discovered that pasting text with a leading tab character, or even manually entering a tab key at the command prompt will crash the terminal. Ctl-C does not clear or recover. ^C is displayed, but no other keys are accepted. You must close this window and open another.

Turns out that's because _tab completion in WSL is occasionally very slow._ The tab character is just triggering tab completion. That's not a great experience though.

Your shell should be activating "bracketed paste mode" so that it doesn't attempt tab completion while you're pasting.

Fix patch:
diff --git a/.github/actions/spelling/allow/allow.txt b/.github/actions/spelling/allow/allow.txt
index 20999b3a54e..28c46840207 100644
--- a/.github/actions/spelling/allow/allow.txt
+++ b/.github/actions/spelling/allow/allow.txt
@@ -13,6 +13,7 @@ clickable
 clig
 CMMI
 copyable
+Counterintuitively
 CtrlDToClose
 cybersecurity
 dalet
diff --git a/.github/actions/spelling/allow/apis.txt b/.github/actions/spelling/allow/apis.txt
index e58df6e202b..b25346b9ce1 100644
--- a/.github/actions/spelling/allow/apis.txt
+++ b/.github/actions/spelling/allow/apis.txt
@@ -226,6 +226,7 @@ Vcpp
 Viewbox
 virtualalloc
 vsnwprintf
+wcsnlen
 wcsstr
 wcstoui
 WDJ
diff --git a/.github/actions/spelling/allow/microsoft.txt b/.github/actions/spelling/allow/microsoft.txt
index b79f49c3781..80335179b32 100644
--- a/.github/actions/spelling/allow/microsoft.txt
+++ b/.github/actions/spelling/allow/microsoft.txt
@@ -59,12 +59,13 @@ PRIINFO
 propkey
 pscustomobject
 QWORD
+rdpclip
 regedit
 resfiles
 robocopy
 SACLs
-segoe
 sdkddkver
+segoe
 Shobjidl
 sid
 Skype
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
index 45b7f34ab66..64dd32c41f4 100644
--- a/src/cascadia/TerminalApp/TerminalPage.cpp
+++ b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -2640,62 +2640,91 @@ namespace winrt::TerminalApp::implementation
     // - Does some of this in a background thread, as to not hang/crash the UI thread.
     // Arguments:
     // - eventArgs: the PasteFromClipboard event sent from the TermControl
-    fire_and_forget TerminalPage::_PasteFromClipboardHandler(const IInspectable /*sender*/,
-                                                             const PasteFromClipboardEventArgs eventArgs)
+    fire_and_forget TerminalPage::_PasteFromClipboardHandler(const IInspectable /*sender*/, const PasteFromClipboardEventArgs eventArgs)
+    try
     {
-        const auto data = Clipboard::GetContent();
+        // The old Win32 clipboard API as used below is somewhere in the order of 300-1000x faster than
+        // the WinRT one on average, depending on CPU load. Don't use the WinRT clipboard API if you can.
+        const auto weakThis = get_weak();
+        const auto dispatcher = Dispatcher();
+        const auto globalSettings = _settings.GlobalSettings();
+        winrt::hstring text;
 
-        // This will switch the execution of the function to a background (not
-        // UI) thread. This is IMPORTANT, because the getting the clipboard data
-        // will crash on the UI thread, because the main thread is a STA.
+        // GetClipboardData might block for up to 30s for delay-rendered contents.
         co_await winrt::resume_background();
 
-        try
         {
-            hstring text = L"";
-            if (data.Contains(StandardDataFormats::Text()))
-            {
-                text = co_await data.GetTextAsync();
-            }
-            // Windows Explorer's "Copy address" menu item stores a StorageItem in the clipboard, and no text.
-            else if (data.Contains(StandardDataFormats::StorageItems()))
+            // According to various reports on the internet, OpenClipboard might
+            // fail to acquire the internal lock, for instance due to rdpclip.exe.
+            for (int attempts = 1;;)
             {
-                auto items = co_await data.GetStorageItemsAsync();
-                if (items.Size() > 0)
+                if (OpenClipboard(nullptr))
                 {
-                    auto item = items.GetAt(0);
-                    text = item.Path();
+                    break;
                 }
-            }
 
-            if (_settings.GlobalSettings().TrimPaste())
-            {
-                text = { Utils::TrimPaste(text) };
-                if (text.empty())
+                if (attempts > 5)
                 {
-                    // Text is all white space, nothing to paste
                     co_return;
                 }
+
+                attempts++;
+                Sleep(10 * attempts);
             }
 
-            // If the requesting terminal is in bracketed paste mode, then we don't need to warn about a multi-line paste.
-            auto warnMultiLine = _settings.GlobalSettings().WarnAboutMultiLinePaste() &&
-                                 !eventArgs.BracketedPasteEnabled();
-            if (warnMultiLine)
+            const auto clipboardCleanup = wil::scope_exit([]() {
+                CloseClipboard();
+            });
+
+            const auto data = GetClipboardData(CF_UNICODETEXT);
+            if (!data)
+            {
+                co_return;
+            }
+
+            const auto str = static_cast<const wchar_t*>(GlobalLock(data));
+            if (!str)
             {
-                const auto isNewLineLambda = [](auto c) { return c == L'\n' || c == L'\r'; };
-                const auto hasNewLine = std::find_if(text.cbegin(), text.cend(), isNewLineLambda) != text.cend();
-                warnMultiLine = hasNewLine;
+                co_return;
             }
 
-            constexpr const std::size_t minimumSizeForWarning = 1024 * 5; // 5 KiB
-            const auto warnLargeText = text.size() > minimumSizeForWarning &&
-                                       _settings.GlobalSettings().WarnAboutLargePaste();
+            const auto dataCleanup = wil::scope_exit([&]() {
+                GlobalUnlock(data);
+            });
+
+            const auto maxLength = GlobalSize(data) / sizeof(wchar_t);
+            const auto length = wcsnlen(str, maxLength);
+            text = winrt::hstring{ str, gsl::narrow_cast<uint32_t>(length) };
+        }
 
-            if (warnMultiLine || warnLargeText)
+        if (globalSettings.TrimPaste())
+        {
+            text = { Utils::TrimPaste(text) };
+            if (text.empty())
             {
-                co_await wil::resume_foreground(Dispatcher());
+                // Text is all white space, nothing to paste
+                co_return;
+            }
+        }
 
+        // If the requesting terminal is in bracketed paste mode, then we don't need to warn about a multi-line paste.
+        auto warnMultiLine = globalSettings.WarnAboutMultiLinePaste() && !eventArgs.BracketedPasteEnabled();
+        if (warnMultiLine)
+        {
+            const auto isNewLineLambda = [](auto c) { return c == L'\n' || c == L'\r'; };
+            const auto hasNewLine = std::find_if(text.cbegin(), text.cend(), isNewLineLambda) != text.cend();
+            warnMultiLine = hasNewLine;
+        }
+
+        constexpr const std::size_t minimumSizeForWarning = 1024 * 5; // 5 KiB
+        const auto warnLargeText = text.size() > minimumSizeForWarning && globalSettings.WarnAboutLargePaste();
+
+        if (warnMultiLine || warnLargeText)
+        {
+            co_await wil::resume_foreground(dispatcher);
+
+            if (const auto strongThis = weakThis.get())
+            {
                 // We have to initialize the dialog here to be able to change the text of the text block within it
                 FindName(L"MultiLinePasteDialog").try_as<WUX::Controls::ContentDialog>();
                 ClipboardText().Text(text);
@@ -2723,10 +2752,15 @@ namespace winrt::TerminalApp::implementation
                 }
             }
 
-            eventArgs.HandleClipboardData(text);
+            co_await winrt::resume_background();
         }
-        CATCH_LOG();
+
+        // This will end up calling ConptyConnection::WriteInput which calls WriteFile which may block for
+        // an indefinite amount of time. Avoid freezes and deadlocks by running this on a background thread.
+        assert(!dispatcher.HasThreadAccess());
+        eventArgs.HandleClipboardData(std::move(text));
     }
+    CATCH_LOG();
 
     void TerminalPage::_OpenHyperlinkHandler(const IInspectable /*sender*/, const Microsoft::Terminal::Control::OpenHyperlinkEventArgs eventArgs)
     {
diff --git a/src/cascadia/TerminalControl/ControlInteractivity.cpp b/src/cascadia/TerminalControl/ControlInteractivity.cpp
index d0b74edfaa2..a97a700162a 100644
--- a/src/cascadia/TerminalControl/ControlInteractivity.cpp
+++ b/src/cascadia/TerminalControl/ControlInteractivity.cpp
@@ -224,22 +224,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // - Initiate a paste operation.
     void ControlInteractivity::RequestPasteTextFromClipboard()
     {
-        // attach ControlInteractivity::_sendPastedTextToConnection() as the
-        // clipboardDataHandler. This is called when the clipboard data is
-        // loaded.
-        auto clipboardDataHandler = std::bind(&ControlInteractivity::_sendPastedTextToConnection, this, std::placeholders::_1);
-        auto pasteArgs = winrt::make_self<PasteFromClipboardEventArgs>(clipboardDataHandler, _core->BracketedPasteEnabled());
+        auto args = winrt::make<PasteFromClipboardEventArgs>(
+            [core = _core](const winrt::hstring& wstr) {
+                core->PasteText(wstr);
+            },
+            _core->BracketedPasteEnabled());
 
         // send paste event up to TermApp
-        _PasteFromClipboardHandlers(*this, *pasteArgs);
-    }
-
-    // Method Description:
-    // - Pre-process text pasted (presumably from the clipboard)
-    //   before sending it over the terminal's connection.
-    void ControlInteractivity::_sendPastedTextToConnection(std::wstring_view wstr)
-    {
-        _core->PasteText(winrt::hstring{ wstr });
+        _PasteFromClipboardHandlers(*this, std::move(args));
     }
 
     void ControlInteractivity::PointerPressed(Control::MouseButtonState buttonState,
diff --git a/src/cascadia/TerminalControl/ControlInteractivity.h b/src/cascadia/TerminalControl/ControlInteractivity.h
index dee2fe9beed..cafd8972c63 100644
--- a/src/cascadia/TerminalControl/ControlInteractivity.h
+++ b/src/cascadia/TerminalControl/ControlInteractivity.h
@@ -155,7 +155,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         bool _canSendVTMouseInput(const ::Microsoft::Terminal::Core::ControlKeyStates modifiers);
         bool _shouldSendAlternateScroll(const ::Microsoft::Terminal::Core::ControlKeyStates modifiers, const int32_t delta);
 
-        void _sendPastedTextToConnection(std::wstring_view wstr);
         til::point _getTerminalPosition(const til::point pixelPosition);
 
         bool _sendMouseEventHelper(const til::point terminalPosition,
diff --git a/src/cascadia/TerminalControl/EventArgs.h b/src/cascadia/TerminalControl/EventArgs.h
index 12632871fdb..1c135cdf4f6 100644
--- a/src/cascadia/TerminalControl/EventArgs.h
+++ b/src/cascadia/TerminalControl/EventArgs.h
@@ -86,7 +86,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     struct PasteFromClipboardEventArgs : public PasteFromClipboardEventArgsT<PasteFromClipboardEventArgs>
     {
     public:
-        PasteFromClipboardEventArgs(std::function<void(std::wstring_view)> clipboardDataHandler, bool bracketedPasteEnabled) :
+        PasteFromClipboardEventArgs(std::function<void(const hstring&)> clipboardDataHandler, bool bracketedPasteEnabled) :
             m_clipboardDataHandler(clipboardDataHandler),
             _BracketedPasteEnabled{ bracketedPasteEnabled } {}
 
@@ -98,7 +98,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         WINRT_PROPERTY(bool, BracketedPasteEnabled, false);
 
     private:
-        std::function<void(std::wstring_view)> m_clipboardDataHandler;
+        std::function<void(const hstring&)> m_clipboardDataHandler;
     };
 
     struct OpenHyperlinkEventArgs : public OpenHyperlinkEventArgsT<OpenHyperlinkEventArgs>


Test patch: