-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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 corruption for high code points #13667
Conversation
// The DEC STD 070 reference recommends supporting up to at least 16384 | ||
// for parameter values. 65535 is what XTerm and VTE support. | ||
// GH#12977: We must use 65535 to properly parse win32-input-mode | ||
// sequences, which transmit the UTF-16 character value as a parameter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[...] so 32767 should be more than enough.
Ah, the classic famous last words... "should be more than enough". lol
I'm a lot less concerned about this change since we mostly aren't using short
s within the project anymore (for most parts).
@msftbot make sure @j4james signs off |
Hello @DHowett! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as we don't go above the 65535 mark, I think we're good. I've done a brief scan of the code, and I can't see any obvious risky areas - the code has changed considerably from the days when parameter overflows were a real problem. I've also run a few test sequences with large parameter values to try and trigger a crash, and they all seemed OK (at least in the sense that nothing died).
We must use 65535 as `MAX_PARAMETER_VALUE` in order for us to properly parse win32-input-mode sequences, which transmit UTF-16 characters as parameters. Closes #12977 ## Validation Steps Performed * Call `SendInput` with 🙁 (`L'\xD83D'`, `L'\xDE41'`) * 🙁 appears on the input line ✅ (cherry picked from commit 74cdffe) Service-Card-Id: 84772549 Service-Version: 1.15
🎉 Handy links: |
🎉 Handy links: |
We must use 65535 as `MAX_PARAMETER_VALUE` in order for us to properly parse win32-input-mode sequences, which transmit UTF-16 characters as parameters. Closes microsoft#12977 ## Validation Steps Performed * Call `SendInput` with 🙁 (`L'\xD83D'`, `L'\xDE41'`) * 🙁 appears on the input line ✅ (cherry picked from commit 74cdffe) Service-Card-Id: 84772549 Service-Version: 1.15 (cherry picked from commit f3f9eba) Service-Card-Id: 84772548 Service-Version: 1.14
We must use 65535 as
MAX_PARAMETER_VALUE
in order for us to properly parsewin32-input-mode sequences, which transmit UTF-16 characters as parameters.
Closes #12977
Validation Steps Performed
SendInput
with 🙁 (L'\xD83D'
,L'\xDE41'
)