Repository: microsoft/terminal (language: cpp)

Issue:
Cooked read can hang when editing a line with control characters
### Windows Terminal version

1.22.2362.0

### Windows build number

10.0.19045.4651

### Other Software

_No response_

### Steps to reproduce

1. Open a cmd shell in Windows Terminal.
2. Type <kbd>Ctrl</kbd>+<kbd>A</kbd>
3. Move the cursor left so it's before the `^A`.
4. Type a bunch of characters so the `^A` is pushed past the end of the line.

### Expected Behavior

The `^A` should eventually wrap over onto the next line.

### Actual Behavior

Once the `^A` reaches the rightmost column, the line editor stops responding to any further keystrokes. In the debugger I can see it's stuck in an infinite loop here:

https://github.com/microsoft/terminal/blob/4aa1624cd2cfd5cdd4106feb26a2aa0f089a0e3d/src/host/readDataCooked.cpp#L922-L941


Discussion / hints:

Fix patch:
diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp
index 9e1e70e3d04..802e48e7c3e 100644
--- a/src/buffer/out/textBuffer.cpp
+++ b/src/buffer/out/textBuffer.cpp
@@ -451,14 +451,9 @@ size_t TextBuffer::FitTextIntoColumns(const std::wstring_view& chars, til::Coord
         cwd.GraphemeNext(state, chars);
         col += state.width;
 
-        // If we ran out of columns, we need to always return `columnLimit` and not `cols`,
-        // because if we tried inserting a wide glyph into just 1 remaining column it will
-        // fail to fit, but that remaining column still has been used up. When the caller sees
-        // `columns == columnLimit` they will line-wrap and continue inserting into the next row.
         if (col > columnLimit)
         {
-            columns = columnLimit;
-            return dist;
+            break;
         }
 
         dist += state.len;
@@ -466,7 +461,7 @@ size_t TextBuffer::FitTextIntoColumns(const std::wstring_view& chars, til::Coord
 
     // But if we simply ran out of text we just need to return the actual number of columns.
     columns = col;
-    return chars.size();
+    return dist;
 }
 
 // Pretend as if `position` is a regular cursor in the TextBuffer.
diff --git a/src/host/readDataCooked.cpp b/src/host/readDataCooked.cpp
index 117482f42e8..30515f7274d 100644
--- a/src/host/readDataCooked.cpp
+++ b/src/host/readDataCooked.cpp
@@ -1267,17 +1267,30 @@ COOKED_READ_DATA::LayoutResult COOKED_READ_DATA::_layoutLine(std::wstring& outpu
             output.append(text, 0, len);
             column += cols;
             it += len;
-        }
 
-        const auto nextPlainChar = std::find_if(it, end, [](const auto& wch) { return wch >= L' '; });
-        for (; it != nextPlainChar; ++it)
-        {
-            if (column >= columnLimit)
+            if (it != nextControlChar)
             {
+                // The only reason that not all text could be fit into the line is if the last character was a wide glyph.
+                // In that case we want to return the columnLimit, to indicate that the row is full and a line wrap is required,
+                // BUT DON'T want to pad the line with a whitespace to actually fill the line to the columnLimit.
+                // This is because copying the prompt contents (Ctrl-A, Ctrl-C) should not copy any trailing padding whitespace.
+                //
+                // Thanks to this lie, the _redisplay() code will not use a CRLF sequence or similar to move to the next line,
+                // as it thinks that this row has naturally wrapped. This causes it to print the wide glyph on the preceding line
+                // which causes the terminal to insert the padding whitespace for us.
                 column = columnLimit;
-                goto outerLoopExit;
+                break;
+            }
+
+            if (column >= columnLimit)
+            {
+                break;
             }
+        }
 
+        const auto nextPlainChar = std::find_if(it, end, [](const auto& wch) { return wch >= L' '; });
+        for (; it != nextPlainChar; ++it)
+        {
             const auto wch = *it;
             wchar_t buf[8];
             til::CoordType len = 0;
@@ -1297,11 +1310,20 @@ COOKED_READ_DATA::LayoutResult COOKED_READ_DATA::_layoutLine(std::wstring& outpu
 
             if (column + len > columnLimit)
             {
+                // Unlike above with regular text we can't avoid padding the line with whitespace, because a string
+                // like "^A" is not a wide glyph, and so we cannot trick the terminal to insert the padding for us.
+                output.append(columnLimit - column, L' ');
+                column = columnLimit;
                 goto outerLoopExit;
             }
 
             output.append(buf, len);
             column += len;
+
+            if (column >= columnLimit)
+            {
+                goto outerLoopExit;
+            }
         }
     }
 


Test patch: