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

Jump to bottom

Stop throwing exception when writing 0 length buffers #32277

Merged
merged 2 commits into from Apr 29, 2021
Merged

Stop throwing exception when writing 0 length buffers #32277

merged 2 commits into from Apr 29, 2021

Conversation

TanayParikh
Copy link
Contributor

Fixes: #31299

@TanayParikh TanayParikh added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Apr 29, 2021
@TanayParikh TanayParikh enabled auto-merge (squash) April 29, 2021 21:26
@TanayParikh TanayParikh merged commit 80f0a03 into main Apr 29, 2021
@TanayParikh TanayParikh deleted the TanayParikh-patch-1 branch April 29, 2021 21:48
@ghost ghost added this to the 6.0-preview5 milestone Apr 29, 2021
@GrabYourPitchforks
Copy link
Member

See also my comment at #31299 (comment). You may be leaving some performance on the floor given the current method implementations.

@ghost
Copy link

ghost commented Apr 29, 2021

Hi @GrabYourPitchforks. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@TanayParikh
Copy link
Contributor Author

See also my comment at #31299 (comment). You may be leaving some performance on the floor given the current method implementations.

Thanks @GrabYourPitchforks, I created #32284 to track that separately.

@TanayParikh TanayParikh mentioned this pull request May 5, 2021
Closed
TanayParikh added a commit that referenced this pull request May 6, 2021
@TanayParikh
Stop throwing exception when writing 0 length buffers (#32277)
881ff14 
* 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
@TanayParikh TanayParikh mentioned this pull request May 6, 2021
Merged
10 tasks
TanayParikh added a commit that referenced this pull request May 6, 2021
@TanayParikh
Stop throwing exception when writing 0 length buffers (#32277)
4fcb9e6 
* 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
@TanayParikh TanayParikh mentioned this pull request May 6, 2021
Merged
10 tasks
mkArtakMSFT pushed a commit that referenced this pull request May 9, 2021
@TanayParikh
Stop throwing exception when writing 0 length buffers (#32277) (#32470)
b124d50 
## 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 5.0 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?
- [ ] Yes
- [x] No

[If yes, specify the version the behavior has regressed from]

## Risk
- [ ] High
- [ ] Medium
- [x] Low

Low risk as we've just updated the logic slightly to permit 0 length buffers.

## Verification
- [x] Manual (required)
- [x] Automated

## Packaging changes reviewed?
- [ ] Yes
- [ ] No
- [x] N/A


Addresses #31299
mkArtakMSFT pushed a commit that referenced this pull request May 10, 2021
@TanayParikh
Backport #32277 to 3.1 (#32482)
9041b5d 
## 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?
- [ ] Yes
- [x] No

[If yes, specify the version the behavior has regressed from]

## Risk
- [ ] High
- [ ] Medium
- [x] Low

Low risk as we've just updated the logic slightly to permit 0 length buffers.

## Verification
- [x] Manual (required)
- [x] Automated

## Packaging changes reviewed?
- [ ] Yes
- [ ] No
- [x] N/A


Addresses #31299
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pranavkm pranavkm pranavkm approved these changes

@sebastienros sebastienros sebastienros approved these changes

@javiercn javiercn Awaiting requested review from javiercn

Assignees
No one assigned
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Milestone
6.0-preview5
Development

Successfully merging this pull request may close these issues.

ArgumentOutOfRangeException in ViewBufferTextWriter with latest servicing release
4 participants
@TanayParikh @GrabYourPitchforks @pranavkm @sebastienros