Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix input sequences split across the buffer boundary #17738

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

j4james
Copy link
Collaborator

@j4james j4james commented Aug 18, 2024

Summary of the Pull Request

When conhost receives input from a conpty connection, and that input
arrives in a block larger than our 4K buffer, we can end up with a VT
sequence that's split at the buffer boundary. Previously that could
result in the start of the sequence being dropped, and the remaining
characters being interpreted as individual key presses.

This PR attempts to fix the issue by caching the unprocessed characters
from the start of the sequence, and then combining them with the second
half of the sequence when it's later received.

Validation Steps Performed

I've confirmed that pasting into vim now works correctly with the sample
data from issue #16655. I've also tested with a DECCTR report larger
than 4K which would previously have been corrupted, and which now works
as expected.

PR Checklist

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Input Related to input processing (key presses, mouse, etc.) Area-VT Virtual Terminal sequence support Priority-1 A description (P1) Product-Conhost For issues in the Console codebase labels Aug 18, 2024
@j4james j4james marked this pull request as ready for review August 18, 2024 22:39
@j4james
Copy link
Collaborator Author

j4james commented Aug 18, 2024

I'm not 100% sure I know what I'm doing here, but I think this is the right solution.

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can no longer claim to understand this code completely, but this fix sounds right to me. Let's try it out... and thanks for digging in!

@DHowett DHowett merged commit 131728b into microsoft:main Aug 20, 2024
15 checks passed
DHowett pushed a commit that referenced this pull request Sep 26, 2024
## Summary of the Pull Request

When conhost receives input from a conpty connection, and that input
arrives in a block larger than our 4K buffer, we can end up with a VT
sequence that's split at the buffer boundary. Previously that could
result in the start of the sequence being dropped, and the remaining
characters being interpreted as individual key presses.

This PR attempts to fix the issue by caching the unprocessed characters
from the start of the sequence, and then combining them with the second
half of the sequence when it's later received.

## Validation Steps Performed

I've confirmed that pasting into vim now works correctly with the sample
data from issue #16655. I've also tested with a `DECCTR` report larger
than 4K which would previously have been corrupted, and which now works
as expected.

## PR Checklist
- [x] Closes #16655

(cherry picked from commit 131728b)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmszgSCpCk
Service-Version: 1.21
@j4james j4james deleted the fix-split-input-sequence branch December 15, 2024 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Conhost For issues in the Console codebase
Projects
Status: To Consider
Development

Successfully merging this pull request may close these issues.

Pasting specific texts into vim (WSL) breaks some keys
3 participants