-
Couldn't load subscription status.
- Fork 5.2k
Update MemoryStream max capacity #119089
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
Update MemoryStream max capacity #119089
Conversation
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.
Pull Request Overview
This PR updates the MemoryStream maximum capacity from int.MaxValue to 0x7FFFFFC7 (the actual maximum byte array length supported by the CLR). This change aligns MemoryStream capacity limits with the underlying array implementation constraints.
Key changes:
- Updated the maximum capacity constant to reflect the true CLR byte array limit
- Added comprehensive boundary testing for capacity validation
- Updated related comments and validation logic to use the new limit
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs |
Updates MemStreamMaxLength constant from int.MaxValue to 0x7FFFFFC7 and adjusts validation logic |
src/libraries/System.Runtime/tests/System.IO.Tests/MemoryStream/MemoryStreamTests.cs |
Adds test to verify capacity boundary behavior at the new maximum limit |
src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs
Outdated
Show resolved
Hide resolved
…m.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ontu2912/runtime into fix_update_mode_max_size
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.
Instead of hardcoding a constant, we can use the Array.MaxLength property.
src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime/tests/System.IO.Tests/BinaryWriter/BinaryWriterTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime/tests/System.IO.Tests/MemoryStream/MemoryStreamTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime/tests/System.IO.Tests/MemoryStream/MemoryStreamTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Theodore Tsirpanis <teo@tsirpanis.gr>
|
Thanks for the input @teo-tsirpanis, I replaced the hardcoded value 👍 |
…ontu2912/runtime into fix_update_mode_max_size
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.
LGTM, thanks.
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.
Sorry for the comments post-merge but I think there's some items to address.
I will also mark this as breaking change since we are changing the exception type being thrown.
| </data> | ||
| <data name="ArgumentOutOfRange_StreamLength" xml:space="preserve"> | ||
| <value>Stream length must be non-negative and less than 2^31 - 1 - origin.</value> | ||
| <value>Stream length must be non-negative and less than the maximum array length (0x7FFFFFC7) - origin.</value> |
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.
There's been efforts to not hardcode 0x7FFFFFC7 and instead use Array.MaxLength. Can we pass $"0x{Array.MaxLength:X}" using SR.Format?
| <value>Stream length must be non-negative and less than the maximum array length (0x7FFFFFC7) - origin.</value> | |
| <value>Stream length must be non-negative and less than the maximum array length {0} - origin.</value> |
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.
Should this do something like the following instead, to help clarify that the value may change over time:
maximum array length (currently 0x{0})
Perhaps we could just explicitly use the text Array.MaxLength instead?
| // Origin wasn't publicly exposed above. | ||
| Debug.Assert(MemStreamMaxLength == int.MaxValue); // Check parameter validation logic in this method if this fails. | ||
| if (value > (int.MaxValue - _origin)) | ||
| Debug.Assert(MemStreamMaxLength == 0x7FFFFFC7); // Check parameter validation logic in this method if this fails. |
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.
| Debug.Assert(MemStreamMaxLength == 0x7FFFFFC7); // Check parameter validation logic in this method if this fails. | |
| Debug.Assert(MemStreamMaxLength == Array.MaxLength); // Check parameter validation logic in this method if this fails. | |
| if (value > (int.MaxValue - _origin)) | ||
| Debug.Assert(MemStreamMaxLength == 0x7FFFFFC7); // Check parameter validation logic in this method if this fails. | ||
| if (value > (MemStreamMaxLength - _origin)) | ||
| throw new ArgumentOutOfRangeException(nameof(value), SR.ArgumentOutOfRange_StreamLength); |
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.
I believe we will also need to update the ctor that takes capacity because it will keep throwing OOM while we throw OutOfRange everywhere else.
|
Added When you commit this breaking change:
Tagging @dotnet/compat for awareness of the breaking change. |
Where is the exception type being changed? Looks like it is still |
|
Ah, I see. Because the old one was 👍 |
* cap memory stream max length and add unit test for max capacity * Update src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix failing tests * Replace hardcoded value Co-authored-by: Theodore Tsirpanis <teo@tsirpanis.gr> * update error message --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Theodore Tsirpanis <teo@tsirpanis.gr>
📋 Breaking Change Documentation RequiredCreate a breaking change issue with AI-generated content Generated by Breaking Change Documentation Tool - 2025-10-24 14:46:42 |
Updates MemoryStream to cap capacity at 0x7FFFFFC7 (the true max byte array length) and adds a test to check:
Fixes #43542