Skip to content

Conversation

@alinpahontu2912
Copy link
Member

Updates MemoryStream to cap capacity at 0x7FFFFFC7 (the true max byte array length) and adds a test to check:

  • Capacity just under the max succeeds
  • Capacity above the max throws

Fixes #43542

@Copilot Copilot AI review requested due to automatic review settings August 26, 2025 10:02
Copy link
Contributor

Copilot AI left a 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

Copy link
Contributor

@teo-tsirpanis teo-tsirpanis left a 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.

Co-authored-by: Theodore Tsirpanis <[email protected]>
@alinpahontu2912
Copy link
Member Author

alinpahontu2912 commented Aug 26, 2025

Thanks for the input @teo-tsirpanis, I replaced the hardcoded value 👍

@teo-tsirpanis teo-tsirpanis requested a review from a team August 26, 2025 13:37
@alinpahontu2912 alinpahontu2912 requested a review from jkotas August 27, 2025 13:35
@jkotas jkotas removed their request for review August 27, 2025 18:15
@ericstj ericstj requested a review from jozkee September 2, 2025 15:08
Copy link
Member

@jozkee jozkee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@jozkee jozkee merged commit f610923 into dotnet:main Sep 2, 2025
144 checks passed
@jozkee jozkee added this to the 11.0.0 milestone Sep 2, 2025
Copy link
Member

@jozkee jozkee left a 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>
Copy link
Member

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?

Suggested change
<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>

Copy link
Member

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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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);
Copy link
Member

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.

@jozkee jozkee added the breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. label Sep 3, 2025
@dotnet-policy-service dotnet-policy-service bot added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Sep 3, 2025
@dotnet-policy-service
Copy link
Contributor

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

@tannergooding
Copy link
Member

I will also mark this as breaking change since we are changing the exception type being thrown.

Where is the exception type being changed? Looks like it is still ArgumentOutOfRangeException

@jkotas
Copy link
Member

jkotas commented Sep 3, 2025

Where is the exception type being changed? Looks like it is still ArgumentOutOfRangeException

new MemoryStream().SetLength(Int32.MaxValue) threw OutOfMemoryException before this change and throws ArgumentOutOfRangeException exception after this change.

@tannergooding
Copy link
Member

new MemoryStream().SetLength(Int32.MaxValue) threw OutOfMemoryException before this change and throws ArgumentOutOfRangeException exception after this change.

Ah, I see. Because the old one was > int.MaxValue so it would've tried and failed previously

👍

filipnavara pushed a commit to filipnavara/runtime that referenced this pull request Sep 5, 2025
* 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 <[email protected]>

* fix failing tests

* Replace hardcoded value

Co-authored-by: Theodore Tsirpanis <[email protected]>

* update error message

---------

Co-authored-by: Copilot <[email protected]>
Co-authored-by: Theodore Tsirpanis <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Oct 4, 2025
@ericstj
Copy link
Member

ericstj commented Oct 24, 2025

📋 Breaking Change Documentation Required

Create a breaking change issue with AI-generated content

Generated by Breaking Change Documentation Tool - 2025-10-24 14:46:42

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.IO breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

More helpful exception when compressing large files in seek mode

6 participants