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
Backport #32277 to 3.1 #32482
Backport #32277 to 3.1 #32482
Conversation
* Stop throwing exception when writing 0 length buffers Fixes: #31299 * Leverage runtime checks & add test https://github.com/dotnet/runtime/blob/7938f9d9fadc3952d792329be602668bef124f23/src/libraries/System.Private.CoreLib/src/System/IO/TextWriter.cs#L159
Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge. |
The release/3.1 branch seems to be failing pretty consistently, any ideas what may be going on there? cc/ @Tratcher |
That's concerning...
|
FYI @ladipro / @rainersigwald |
This looks like a bug introduced with dotnet/msbuild#5997. Did you guys recently upgrade VS/MSBuild? |
Try incorporating 8052396 to work around the MSBuild regression, please. |
Thanks @rainersigwald, added 75b7808. Given this is a servicing release would you mind confirming if anything changes in the servicing template in the initial comment? (ie. Customer Impact, Regression, Risk, Packaging, etc)? |
It does increase risk because it changes more, but I don't think it is a substantial risk. I also don't think you're seeing this because of the change in this PR. I think you're seeing it in this branch because an updated VS was deployed in the pool you're using for CI builds, and that it would affect any PR targeting this branch. As such it's more in the "required infra change to proceed with servicing". |
Thanks @rainersigwald for the clarification, the CI is now passing!
Yes, can confirm we were seeing this on the release/3.1 branch prior to this PR as well.
@mkArtakMSFT do we need to go through the servicing approval process again for the 75b7808 additional change? If so, given the nature of the fix (fixes the branch rather than anything specific to this PR) I think it may be beneficial to create a separate PR with just that change (and revert 75b7808 in this PR). |
@wtgodbe can you please make sure this is picked up 3.1 June release? |
Will do |
Description
Update guard logic to permit writing 0 length buffers. This can happen in certain globalized scenarios (ex.
¿
with Portugese) where the first char needs to be encoded.Patch #31299 for 3.1 by cherry-picking the #32277 squash commit.
Customer Impact
Writing 0-length char buffers leads to an ArgumentOutOfRangeException. This may occur in a globalization context with certain chars such as
¿
.#31299 (comment)
Also impacts Orchard: #31299 (comment)
Regression?
[If yes, specify the version the behavior has regressed from]
Risk
Low risk as we've just updated the logic slightly to permit 0 length buffers.
Verification
Packaging changes reviewed?
Addresses #31299