std: fix panic in Windows Stdin::read_vectored with pending surrogate#159134
Conversation
|
Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @Darksonn (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
The small-buffer path in Stdin::read allocates a one-element UTF-16 scratch buffer. When a pending surrogate exists, read_u16s_fixup_surrogates requires temporary storage for both the buffered surrogate and one newly read UTF-16 code unit. Increase the scratch buffer to two UTF-16 code units so the surrogate path can complete without panicking.
0652710 to
5d637fd
Compare
|
You should consider adding a regression test. |
|
Yes please add a test. @rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
|
Sure, I'll add a ssimple regression test. |
|
@hkBst and @Darksonn Thanks! I looked into adding a regression test before making further changes. The unit test I initially explored wasn't actually a valid regression test because it exercised I also looked through the existing Windows std test infrastructure, but I couldn't find a way to exercise the Is there a preferred testing approach for this code path, or would manual verification be acceptable here? |
|
I guess we can't really test it due to #93055. Can you confirm that you've tested this code manually? |
|
Yes. I manually verified the fix on Windows using a reproduction based on the program from #158096. Before the change, the second After increasing the scratch buffer from 1 to 2 UTF-16 code units, the same reproduction completed successfully without panicking. |
|
I've read the code and it makes sense to me. @bors r+ |
|
Thanks for taking the time to review it! |
…, r=Darksonn std: fix panic in Windows Stdin::read_vectored with pending surrogate Fixes rust-lang#158096. The small-buffer path in `Stdin::read_vectored` allocates a one-element UTF-16 scratch buffer before calling `read_u16s_fixup_surrogates`. When a pending surrogate exists, `read_u16s_fixup_surrogates` needs temporary storage for both the buffered surrogate and one newly read UTF-16 code unit. The one-element buffer causes an out-of-bounds slice before `ReadConsoleW` is called. Increase the scratch buffer to two UTF-16 code units, matching the function's requirements while leaving the normal read path unchanged.
…, r=Darksonn std: fix panic in Windows Stdin::read_vectored with pending surrogate Fixes rust-lang#158096. The small-buffer path in `Stdin::read_vectored` allocates a one-element UTF-16 scratch buffer before calling `read_u16s_fixup_surrogates`. When a pending surrogate exists, `read_u16s_fixup_surrogates` needs temporary storage for both the buffered surrogate and one newly read UTF-16 code unit. The one-element buffer causes an out-of-bounds slice before `ReadConsoleW` is called. Increase the scratch buffer to two UTF-16 code units, matching the function's requirements while leaving the normal read path unchanged.
…uwer Rollup of 7 pull requests Successful merges: - #158732 (Apply MCP 1003 and move diagnostics.rs into its own module) - #159010 (Simplify the unwind crate) - #159131 (bootstrap: Allow path-based skipping of the coverage test suite) - #159134 (std: fix panic in Windows Stdin::read_vectored with pending surrogate) - #159178 (Print duration of BOLT instrumentation and optimization steps) - #159112 (Fix segfault in env access in statically linked FreeBSD binaries) - #159124 (compiletest: use VecDeque::pop_front_if)
…uwer Rollup of 6 pull requests Successful merges: - #158732 (Apply MCP 1003 and move diagnostics.rs into its own module) - #159131 (bootstrap: Allow path-based skipping of the coverage test suite) - #159134 (std: fix panic in Windows Stdin::read_vectored with pending surrogate) - #159178 (Print duration of BOLT instrumentation and optimization steps) - #159112 (Fix segfault in env access in statically linked FreeBSD binaries) - #159124 (compiletest: use VecDeque::pop_front_if)
Rollup merge of #159134 - rabindra789:fix-158096-stdin-panic, r=Darksonn std: fix panic in Windows Stdin::read_vectored with pending surrogate Fixes #158096. The small-buffer path in `Stdin::read_vectored` allocates a one-element UTF-16 scratch buffer before calling `read_u16s_fixup_surrogates`. When a pending surrogate exists, `read_u16s_fixup_surrogates` needs temporary storage for both the buffered surrogate and one newly read UTF-16 code unit. The one-element buffer causes an out-of-bounds slice before `ReadConsoleW` is called. Increase the scratch buffer to two UTF-16 code units, matching the function's requirements while leaving the normal read path unchanged.
Fixes #158096.
The small-buffer path in
Stdin::read_vectoredallocates a one-element UTF-16 scratch buffer before callingread_u16s_fixup_surrogates.When a pending surrogate exists,
read_u16s_fixup_surrogatesneeds temporary storage for both the buffered surrogate and one newly read UTF-16 code unit. The one-element buffer causes an out-of-bounds slice beforeReadConsoleWis called.Increase the scratch buffer to two UTF-16 code units, matching the function's requirements while leaving the normal read path unchanged.