Clarify why and make text and sequence streams non-seekable

[jozkee]

Summary

• clarify that ReadOnlyMemoryStream and WritableMemoryStream immediately expose their backing memory contents
• make ReadOnlySequenceStream intentionally non-seekable and remove its seek state and traversal logic
• keep StringStream intentionally non-seekable and document why backward positioning would require rerunning the encoder
• make Length, Position, and Seek consistently throw the standard unseekable-stream NotSupportedException
• update stream conformance and focused unit coverage
• For Writable memory stream, added ctor remarks suggesting users to clear rented or reused memory before constructing the stream if its existing contents should not be exposed.

Rationale

Backward positioning requires replaying work from the beginning. For ReadOnlySequenceStream, that means traversing segments whose boundaries may be indirectly controlled by an untrusted network client through packet framing. Even correct stitching can therefore produce adversarial fragmentation, and consumers must tolerate the worst technically compliant segmentation rather than assuming ASP.NET-like behavior. For StringStream, backward positioning requires rerunning the encoder. Repeated seeks can turn both cases into worst-case O(N) work.

Validation

• checked and Release CoreLib builds
• System.Memory build
• 405 focused ReadOnlySequenceStream conformance tests
• 275 focused StringStream conformance/unit tests

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR aligns several Stream implementations with an intentionally non-seekable contract (notably ReadOnlySequenceStream, and reinforcing StringStream expectations) and updates docs/tests accordingly, including clarifying that the memory-backed streams expose existing buffer contents immediately.

Changes:

• Make ReadOnlySequenceStream intentionally non-seekable by removing seek state/traversal and having Length/Position/Seek throw NotSupportedException consistently.
• Clarify ReadOnlyMemoryStream / WritableMemoryStream docs that backing memory contents are immediately readable.
• Update conformance and focused unit tests to reflect the non-seekable behavior and exception expectations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/libraries/System.Runtime/tests/System.IO.Tests/StringStream/StringStreamTests_String.cs	Extends capability tests to assert unseekable members throw NotSupportedException.
src/libraries/System.Private.CoreLib/src/System/IO/WritableMemoryStream.cs	Doc updates clarifying immediate exposure of existing buffer contents.
src/libraries/System.Private.CoreLib/src/System/IO/StringStream.cs	Adds rationale comment for keeping the stream non-seekable.
src/libraries/System.Private.CoreLib/src/System/IO/ReadOnlyMemoryStream.cs	Doc updates clarifying immediate exposure (and minor wording tweak needed).
src/libraries/System.Memory/tests/ReadOnlyBuffer/ReadOnlySequenceStream.ConformanceTests.cs	Updates conformance configuration to treat the stream as non-seekable; removes seek-specific override.
src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs	Implements non-seekable contract; removes seek/position bookkeeping and logic.
src/libraries/System.Memory/src/Resources/Strings.resx	Adds NotSupported_UnseekableStream resource; removes no-longer-needed seek-related resource strings.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:173

• CopyToAsyncCore only assigns _position = _sequence.End after the full loop completes. If WriteAsync throws/cancels after some segments were written, the stream’s _position stays at the original value and subsequent reads can replay already-transferred bytes.

Advance _position after each successfully completed WriteAsync call.

 {
await destination.WriteAsync(segment, cancellationToken).ConfigureAwait(false);
}
_position = _sequence.End;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:44

• This change makes a previously-seekable public Stream (ReadOnlySequenceStream is in System.Memory ref) report CanSeek == false and throw NotSupportedException from Length/Position/Seek. That’s a behavioral breaking change for existing consumers.

The PR description doesn’t reference a breaking-change tracking issue. Per docs/project/breaking-change-process.md, please link/create an issue marked breaking-change (with before/after behavior and mitigations) and reference it from the PR so reviewers can evaluate compatibility impact.

 // segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:143

• CopyTo writes multiple segments but only updates _position after the loop completes. If destination.Write(...) throws after some segments were successfully written, the stream will keep its old _position even though bytes were already consumed, so subsequent reads / retries can duplicate data.

Update _position after each successfully-written segment so the stream’s internal read position always reflects the bytes already transferred.

This issue also appears on line 169 of the same file.

 {
destination.Write(segment.Span);
}
_position = _sequence.End;

[adamsitnik]

Overall it LGTM, but I wonder if we should improve the XML doc comments before merging. Thank you for fixing it before the .NET 11 release @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:45

• Changing ReadOnlySequenceStream to be non-seekable (CanSeek=false, and Length/Position/Seek throwing NotSupportedException) is a behavioral breaking change for a public type (see ref/System.Memory.cs). Per repo guidelines, this should go through the breaking-change process and include the required documentation/metadata updates (e.g., add the appropriate breaking-change entry per docs/project/breaking-change-process.md).

 /// <summary>Gets a value indicating whether the <see cref="ReadOnlySequenceStream"/> supports seeking.</summary>
// Keep this intentionally non-seekable: backward positioning requires traversing segments
// again from the beginning, making repeated seeks worst-case O(N). ReadOnlySequence<T>
// segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

[adamsitnik]

I left a comment, PTAL @jozkee

[ViveliDuCh]

LGTM. Thanks for the documentation effort!

[jozkee]

Summary

• clarify that ReadOnlyMemoryStream and WritableMemoryStream immediately expose their backing memory contents
• make ReadOnlySequenceStream intentionally non-seekable and remove its seek state and traversal logic
• keep StringStream intentionally non-seekable and document why backward positioning would require rerunning the encoder
• make Length, Position, and Seek consistently throw the standard unseekable-stream NotSupportedException
• update stream conformance and focused unit coverage
• For Writable memory stream, added ctor remarks suggesting users to clear rented or reused memory before constructing the stream if its existing contents should not be exposed.

Rationale

Backward positioning requires replaying work from the beginning. For ReadOnlySequenceStream, that means traversing segments whose boundaries may be indirectly controlled by an untrusted network client through packet framing. Even correct stitching can therefore produce adversarial fragmentation, and consumers must tolerate the worst technically compliant segmentation rather than assuming ASP.NET-like behavior. For StringStream, backward positioning requires rerunning the encoder. Repeated seeks can turn both cases into worst-case O(N) work.

Validation

• checked and Release CoreLib builds
• System.Memory build
• 405 focused ReadOnlySequenceStream conformance tests
• 275 focused StringStream conformance/unit tests

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR aligns several Stream implementations with an intentionally non-seekable contract (notably ReadOnlySequenceStream, and reinforcing StringStream expectations) and updates docs/tests accordingly, including clarifying that the memory-backed streams expose existing buffer contents immediately.

Changes:

• Make ReadOnlySequenceStream intentionally non-seekable by removing seek state/traversal and having Length/Position/Seek throw NotSupportedException consistently.
• Clarify ReadOnlyMemoryStream / WritableMemoryStream docs that backing memory contents are immediately readable.
• Update conformance and focused unit tests to reflect the non-seekable behavior and exception expectations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/libraries/System.Runtime/tests/System.IO.Tests/StringStream/StringStreamTests_String.cs	Extends capability tests to assert unseekable members throw NotSupportedException.
src/libraries/System.Private.CoreLib/src/System/IO/WritableMemoryStream.cs	Doc updates clarifying immediate exposure of existing buffer contents.
src/libraries/System.Private.CoreLib/src/System/IO/StringStream.cs	Adds rationale comment for keeping the stream non-seekable.
src/libraries/System.Private.CoreLib/src/System/IO/ReadOnlyMemoryStream.cs	Doc updates clarifying immediate exposure (and minor wording tweak needed).
src/libraries/System.Memory/tests/ReadOnlyBuffer/ReadOnlySequenceStream.ConformanceTests.cs	Updates conformance configuration to treat the stream as non-seekable; removes seek-specific override.
src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs	Implements non-seekable contract; removes seek/position bookkeeping and logic.
src/libraries/System.Memory/src/Resources/Strings.resx	Adds NotSupported_UnseekableStream resource; removes no-longer-needed seek-related resource strings.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:173

• CopyToAsyncCore only assigns _position = _sequence.End after the full loop completes. If WriteAsync throws/cancels after some segments were written, the stream’s _position stays at the original value and subsequent reads can replay already-transferred bytes.

Advance _position after each successfully completed WriteAsync call.

 {
await destination.WriteAsync(segment, cancellationToken).ConfigureAwait(false);
}
_position = _sequence.End;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:44

• This change makes a previously-seekable public Stream (ReadOnlySequenceStream is in System.Memory ref) report CanSeek == false and throw NotSupportedException from Length/Position/Seek. That’s a behavioral breaking change for existing consumers.

The PR description doesn’t reference a breaking-change tracking issue. Per docs/project/breaking-change-process.md, please link/create an issue marked breaking-change (with before/after behavior and mitigations) and reference it from the PR so reviewers can evaluate compatibility impact.

 // segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:143

• CopyTo writes multiple segments but only updates _position after the loop completes. If destination.Write(...) throws after some segments were successfully written, the stream will keep its old _position even though bytes were already consumed, so subsequent reads / retries can duplicate data.

Update _position after each successfully-written segment so the stream’s internal read position always reflects the bytes already transferred.

This issue also appears on line 169 of the same file.

 {
destination.Write(segment.Span);
}
_position = _sequence.End;

[adamsitnik]

Overall it LGTM, but I wonder if we should improve the XML doc comments before merging. Thank you for fixing it before the .NET 11 release @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:45

• Changing ReadOnlySequenceStream to be non-seekable (CanSeek=false, and Length/Position/Seek throwing NotSupportedException) is a behavioral breaking change for a public type (see ref/System.Memory.cs). Per repo guidelines, this should go through the breaking-change process and include the required documentation/metadata updates (e.g., add the appropriate breaking-change entry per docs/project/breaking-change-process.md).

 /// <summary>Gets a value indicating whether the <see cref="ReadOnlySequenceStream"/> supports seeking.</summary>
// Keep this intentionally non-seekable: backward positioning requires traversing segments
// again from the beginning, making repeated seeks worst-case O(N). ReadOnlySequence<T>
// segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

[adamsitnik]

I left a comment, PTAL @jozkee

[ViveliDuCh]

LGTM. Thanks for the documentation effort!

[jozkee]

Summary

• clarify that ReadOnlyMemoryStream and WritableMemoryStream immediately expose their backing memory contents
• make ReadOnlySequenceStream intentionally non-seekable and remove its seek state and traversal logic
• keep StringStream intentionally non-seekable and document why backward positioning would require rerunning the encoder
• make Length, Position, and Seek consistently throw the standard unseekable-stream NotSupportedException
• update stream conformance and focused unit coverage
• For Writable memory stream, added ctor remarks suggesting users to clear rented or reused memory before constructing the stream if its existing contents should not be exposed.

Rationale

Backward positioning requires replaying work from the beginning. For ReadOnlySequenceStream, that means traversing segments whose boundaries may be indirectly controlled by an untrusted network client through packet framing. Even correct stitching can therefore produce adversarial fragmentation, and consumers must tolerate the worst technically compliant segmentation rather than assuming ASP.NET-like behavior. For StringStream, backward positioning requires rerunning the encoder. Repeated seeks can turn both cases into worst-case O(N) work.

Validation

• checked and Release CoreLib builds
• System.Memory build
• 405 focused ReadOnlySequenceStream conformance tests
• 275 focused StringStream conformance/unit tests

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR aligns several Stream implementations with an intentionally non-seekable contract (notably ReadOnlySequenceStream, and reinforcing StringStream expectations) and updates docs/tests accordingly, including clarifying that the memory-backed streams expose existing buffer contents immediately.

Changes:

• Make ReadOnlySequenceStream intentionally non-seekable by removing seek state/traversal and having Length/Position/Seek throw NotSupportedException consistently.
• Clarify ReadOnlyMemoryStream / WritableMemoryStream docs that backing memory contents are immediately readable.
• Update conformance and focused unit tests to reflect the non-seekable behavior and exception expectations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/libraries/System.Runtime/tests/System.IO.Tests/StringStream/StringStreamTests_String.cs	Extends capability tests to assert unseekable members throw NotSupportedException.
src/libraries/System.Private.CoreLib/src/System/IO/WritableMemoryStream.cs	Doc updates clarifying immediate exposure of existing buffer contents.
src/libraries/System.Private.CoreLib/src/System/IO/StringStream.cs	Adds rationale comment for keeping the stream non-seekable.
src/libraries/System.Private.CoreLib/src/System/IO/ReadOnlyMemoryStream.cs	Doc updates clarifying immediate exposure (and minor wording tweak needed).
src/libraries/System.Memory/tests/ReadOnlyBuffer/ReadOnlySequenceStream.ConformanceTests.cs	Updates conformance configuration to treat the stream as non-seekable; removes seek-specific override.
src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs	Implements non-seekable contract; removes seek/position bookkeeping and logic.
src/libraries/System.Memory/src/Resources/Strings.resx	Adds NotSupported_UnseekableStream resource; removes no-longer-needed seek-related resource strings.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:173

• CopyToAsyncCore only assigns _position = _sequence.End after the full loop completes. If WriteAsync throws/cancels after some segments were written, the stream’s _position stays at the original value and subsequent reads can replay already-transferred bytes.

Advance _position after each successfully completed WriteAsync call.

 {
await destination.WriteAsync(segment, cancellationToken).ConfigureAwait(false);
}
_position = _sequence.End;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:44

• This change makes a previously-seekable public Stream (ReadOnlySequenceStream is in System.Memory ref) report CanSeek == false and throw NotSupportedException from Length/Position/Seek. That’s a behavioral breaking change for existing consumers.

The PR description doesn’t reference a breaking-change tracking issue. Per docs/project/breaking-change-process.md, please link/create an issue marked breaking-change (with before/after behavior and mitigations) and reference it from the PR so reviewers can evaluate compatibility impact.

 // segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:143

• CopyTo writes multiple segments but only updates _position after the loop completes. If destination.Write(...) throws after some segments were successfully written, the stream will keep its old _position even though bytes were already consumed, so subsequent reads / retries can duplicate data.

Update _position after each successfully-written segment so the stream’s internal read position always reflects the bytes already transferred.

This issue also appears on line 169 of the same file.

 {
destination.Write(segment.Span);
}
_position = _sequence.End;

[adamsitnik]

Overall it LGTM, but I wonder if we should improve the XML doc comments before merging. Thank you for fixing it before the .NET 11 release @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:45

• Changing ReadOnlySequenceStream to be non-seekable (CanSeek=false, and Length/Position/Seek throwing NotSupportedException) is a behavioral breaking change for a public type (see ref/System.Memory.cs). Per repo guidelines, this should go through the breaking-change process and include the required documentation/metadata updates (e.g., add the appropriate breaking-change entry per docs/project/breaking-change-process.md).

 /// <summary>Gets a value indicating whether the <see cref="ReadOnlySequenceStream"/> supports seeking.</summary>
// Keep this intentionally non-seekable: backward positioning requires traversing segments
// again from the beginning, making repeated seeks worst-case O(N). ReadOnlySequence<T>
// segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

[adamsitnik]

I left a comment, PTAL @jozkee

[ViveliDuCh]

LGTM. Thanks for the documentation effort!

[jozkee]

Summary

• clarify that ReadOnlyMemoryStream and WritableMemoryStream immediately expose their backing memory contents
• make ReadOnlySequenceStream intentionally non-seekable and remove its seek state and traversal logic
• keep StringStream intentionally non-seekable and document why backward positioning would require rerunning the encoder
• make Length, Position, and Seek consistently throw the standard unseekable-stream NotSupportedException
• update stream conformance and focused unit coverage
• For Writable memory stream, added ctor remarks suggesting users to clear rented or reused memory before constructing the stream if its existing contents should not be exposed.

Rationale

Backward positioning requires replaying work from the beginning. For ReadOnlySequenceStream, that means traversing segments whose boundaries may be indirectly controlled by an untrusted network client through packet framing. Even correct stitching can therefore produce adversarial fragmentation, and consumers must tolerate the worst technically compliant segmentation rather than assuming ASP.NET-like behavior. For StringStream, backward positioning requires rerunning the encoder. Repeated seeks can turn both cases into worst-case O(N) work.

Validation

• checked and Release CoreLib builds
• System.Memory build
• 405 focused ReadOnlySequenceStream conformance tests
• 275 focused StringStream conformance/unit tests

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR aligns several Stream implementations with an intentionally non-seekable contract (notably ReadOnlySequenceStream, and reinforcing StringStream expectations) and updates docs/tests accordingly, including clarifying that the memory-backed streams expose existing buffer contents immediately.

Changes:

• Make ReadOnlySequenceStream intentionally non-seekable by removing seek state/traversal and having Length/Position/Seek throw NotSupportedException consistently.
• Clarify ReadOnlyMemoryStream / WritableMemoryStream docs that backing memory contents are immediately readable.
• Update conformance and focused unit tests to reflect the non-seekable behavior and exception expectations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/libraries/System.Runtime/tests/System.IO.Tests/StringStream/StringStreamTests_String.cs	Extends capability tests to assert unseekable members throw NotSupportedException.
src/libraries/System.Private.CoreLib/src/System/IO/WritableMemoryStream.cs	Doc updates clarifying immediate exposure of existing buffer contents.
src/libraries/System.Private.CoreLib/src/System/IO/StringStream.cs	Adds rationale comment for keeping the stream non-seekable.
src/libraries/System.Private.CoreLib/src/System/IO/ReadOnlyMemoryStream.cs	Doc updates clarifying immediate exposure (and minor wording tweak needed).
src/libraries/System.Memory/tests/ReadOnlyBuffer/ReadOnlySequenceStream.ConformanceTests.cs	Updates conformance configuration to treat the stream as non-seekable; removes seek-specific override.
src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs	Implements non-seekable contract; removes seek/position bookkeeping and logic.
src/libraries/System.Memory/src/Resources/Strings.resx	Adds NotSupported_UnseekableStream resource; removes no-longer-needed seek-related resource strings.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:173

• CopyToAsyncCore only assigns _position = _sequence.End after the full loop completes. If WriteAsync throws/cancels after some segments were written, the stream’s _position stays at the original value and subsequent reads can replay already-transferred bytes.

Advance _position after each successfully completed WriteAsync call.

 {
await destination.WriteAsync(segment, cancellationToken).ConfigureAwait(false);
}
_position = _sequence.End;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:44

• This change makes a previously-seekable public Stream (ReadOnlySequenceStream is in System.Memory ref) report CanSeek == false and throw NotSupportedException from Length/Position/Seek. That’s a behavioral breaking change for existing consumers.

The PR description doesn’t reference a breaking-change tracking issue. Per docs/project/breaking-change-process.md, please link/create an issue marked breaking-change (with before/after behavior and mitigations) and reference it from the PR so reviewers can evaluate compatibility impact.

 // segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:143

• CopyTo writes multiple segments but only updates _position after the loop completes. If destination.Write(...) throws after some segments were successfully written, the stream will keep its old _position even though bytes were already consumed, so subsequent reads / retries can duplicate data.

Update _position after each successfully-written segment so the stream’s internal read position always reflects the bytes already transferred.

This issue also appears on line 169 of the same file.

 {
destination.Write(segment.Span);
}
_position = _sequence.End;

[adamsitnik]

Overall it LGTM, but I wonder if we should improve the XML doc comments before merging. Thank you for fixing it before the .NET 11 release @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:45

• Changing ReadOnlySequenceStream to be non-seekable (CanSeek=false, and Length/Position/Seek throwing NotSupportedException) is a behavioral breaking change for a public type (see ref/System.Memory.cs). Per repo guidelines, this should go through the breaking-change process and include the required documentation/metadata updates (e.g., add the appropriate breaking-change entry per docs/project/breaking-change-process.md).

 /// <summary>Gets a value indicating whether the <see cref="ReadOnlySequenceStream"/> supports seeking.</summary>
// Keep this intentionally non-seekable: backward positioning requires traversing segments
// again from the beginning, making repeated seeks worst-case O(N). ReadOnlySequence<T>
// segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

[adamsitnik]

I left a comment, PTAL @jozkee

[ViveliDuCh]

LGTM. Thanks for the documentation effort!

[jozkee]

Summary

• clarify that ReadOnlyMemoryStream and WritableMemoryStream immediately expose their backing memory contents
• make ReadOnlySequenceStream intentionally non-seekable and remove its seek state and traversal logic
• keep StringStream intentionally non-seekable and document why backward positioning would require rerunning the encoder
• make Length, Position, and Seek consistently throw the standard unseekable-stream NotSupportedException
• update stream conformance and focused unit coverage
• For Writable memory stream, added ctor remarks suggesting users to clear rented or reused memory before constructing the stream if its existing contents should not be exposed.

Rationale

Backward positioning requires replaying work from the beginning. For ReadOnlySequenceStream, that means traversing segments whose boundaries may be indirectly controlled by an untrusted network client through packet framing. Even correct stitching can therefore produce adversarial fragmentation, and consumers must tolerate the worst technically compliant segmentation rather than assuming ASP.NET-like behavior. For StringStream, backward positioning requires rerunning the encoder. Repeated seeks can turn both cases into worst-case O(N) work.

Validation

• checked and Release CoreLib builds
• System.Memory build
• 405 focused ReadOnlySequenceStream conformance tests
• 275 focused StringStream conformance/unit tests

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR aligns several Stream implementations with an intentionally non-seekable contract (notably ReadOnlySequenceStream, and reinforcing StringStream expectations) and updates docs/tests accordingly, including clarifying that the memory-backed streams expose existing buffer contents immediately.

Changes:

• Make ReadOnlySequenceStream intentionally non-seekable by removing seek state/traversal and having Length/Position/Seek throw NotSupportedException consistently.
• Clarify ReadOnlyMemoryStream / WritableMemoryStream docs that backing memory contents are immediately readable.
• Update conformance and focused unit tests to reflect the non-seekable behavior and exception expectations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/libraries/System.Runtime/tests/System.IO.Tests/StringStream/StringStreamTests_String.cs	Extends capability tests to assert unseekable members throw NotSupportedException.
src/libraries/System.Private.CoreLib/src/System/IO/WritableMemoryStream.cs	Doc updates clarifying immediate exposure of existing buffer contents.
src/libraries/System.Private.CoreLib/src/System/IO/StringStream.cs	Adds rationale comment for keeping the stream non-seekable.
src/libraries/System.Private.CoreLib/src/System/IO/ReadOnlyMemoryStream.cs	Doc updates clarifying immediate exposure (and minor wording tweak needed).
src/libraries/System.Memory/tests/ReadOnlyBuffer/ReadOnlySequenceStream.ConformanceTests.cs	Updates conformance configuration to treat the stream as non-seekable; removes seek-specific override.
src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs	Implements non-seekable contract; removes seek/position bookkeeping and logic.
src/libraries/System.Memory/src/Resources/Strings.resx	Adds NotSupported_UnseekableStream resource; removes no-longer-needed seek-related resource strings.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:173

• CopyToAsyncCore only assigns _position = _sequence.End after the full loop completes. If WriteAsync throws/cancels after some segments were written, the stream’s _position stays at the original value and subsequent reads can replay already-transferred bytes.

Advance _position after each successfully completed WriteAsync call.

 {
await destination.WriteAsync(segment, cancellationToken).ConfigureAwait(false);
}
_position = _sequence.End;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:44

• This change makes a previously-seekable public Stream (ReadOnlySequenceStream is in System.Memory ref) report CanSeek == false and throw NotSupportedException from Length/Position/Seek. That’s a behavioral breaking change for existing consumers.

The PR description doesn’t reference a breaking-change tracking issue. Per docs/project/breaking-change-process.md, please link/create an issue marked breaking-change (with before/after behavior and mitigations) and reference it from the PR so reviewers can evaluate compatibility impact.

 // segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:143

• CopyTo writes multiple segments but only updates _position after the loop completes. If destination.Write(...) throws after some segments were successfully written, the stream will keep its old _position even though bytes were already consumed, so subsequent reads / retries can duplicate data.

Update _position after each successfully-written segment so the stream’s internal read position always reflects the bytes already transferred.

This issue also appears on line 169 of the same file.

 {
destination.Write(segment.Span);
}
_position = _sequence.End;

[adamsitnik]

Overall it LGTM, but I wonder if we should improve the XML doc comments before merging. Thank you for fixing it before the .NET 11 release @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:45

• Changing ReadOnlySequenceStream to be non-seekable (CanSeek=false, and Length/Position/Seek throwing NotSupportedException) is a behavioral breaking change for a public type (see ref/System.Memory.cs). Per repo guidelines, this should go through the breaking-change process and include the required documentation/metadata updates (e.g., add the appropriate breaking-change entry per docs/project/breaking-change-process.md).

 /// <summary>Gets a value indicating whether the <see cref="ReadOnlySequenceStream"/> supports seeking.</summary>
// Keep this intentionally non-seekable: backward positioning requires traversing segments
// again from the beginning, making repeated seeks worst-case O(N). ReadOnlySequence<T>
// segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

[adamsitnik]

I left a comment, PTAL @jozkee

[ViveliDuCh]

LGTM. Thanks for the documentation effort!

[jozkee]

Summary

• clarify that ReadOnlyMemoryStream and WritableMemoryStream immediately expose their backing memory contents
• make ReadOnlySequenceStream intentionally non-seekable and remove its seek state and traversal logic
• keep StringStream intentionally non-seekable and document why backward positioning would require rerunning the encoder
• make Length, Position, and Seek consistently throw the standard unseekable-stream NotSupportedException
• update stream conformance and focused unit coverage
• For Writable memory stream, added ctor remarks suggesting users to clear rented or reused memory before constructing the stream if its existing contents should not be exposed.

Rationale

Backward positioning requires replaying work from the beginning. For ReadOnlySequenceStream, that means traversing segments whose boundaries may be indirectly controlled by an untrusted network client through packet framing. Even correct stitching can therefore produce adversarial fragmentation, and consumers must tolerate the worst technically compliant segmentation rather than assuming ASP.NET-like behavior. For StringStream, backward positioning requires rerunning the encoder. Repeated seeks can turn both cases into worst-case O(N) work.

Validation

• checked and Release CoreLib builds
• System.Memory build
• 405 focused ReadOnlySequenceStream conformance tests
• 275 focused StringStream conformance/unit tests

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR aligns several Stream implementations with an intentionally non-seekable contract (notably ReadOnlySequenceStream, and reinforcing StringStream expectations) and updates docs/tests accordingly, including clarifying that the memory-backed streams expose existing buffer contents immediately.

Changes:

• Make ReadOnlySequenceStream intentionally non-seekable by removing seek state/traversal and having Length/Position/Seek throw NotSupportedException consistently.
• Clarify ReadOnlyMemoryStream / WritableMemoryStream docs that backing memory contents are immediately readable.
• Update conformance and focused unit tests to reflect the non-seekable behavior and exception expectations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/libraries/System.Runtime/tests/System.IO.Tests/StringStream/StringStreamTests_String.cs	Extends capability tests to assert unseekable members throw NotSupportedException.
src/libraries/System.Private.CoreLib/src/System/IO/WritableMemoryStream.cs	Doc updates clarifying immediate exposure of existing buffer contents.
src/libraries/System.Private.CoreLib/src/System/IO/StringStream.cs	Adds rationale comment for keeping the stream non-seekable.
src/libraries/System.Private.CoreLib/src/System/IO/ReadOnlyMemoryStream.cs	Doc updates clarifying immediate exposure (and minor wording tweak needed).
src/libraries/System.Memory/tests/ReadOnlyBuffer/ReadOnlySequenceStream.ConformanceTests.cs	Updates conformance configuration to treat the stream as non-seekable; removes seek-specific override.
src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs	Implements non-seekable contract; removes seek/position bookkeeping and logic.
src/libraries/System.Memory/src/Resources/Strings.resx	Adds NotSupported_UnseekableStream resource; removes no-longer-needed seek-related resource strings.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:173

• CopyToAsyncCore only assigns _position = _sequence.End after the full loop completes. If WriteAsync throws/cancels after some segments were written, the stream’s _position stays at the original value and subsequent reads can replay already-transferred bytes.

Advance _position after each successfully completed WriteAsync call.

 {
await destination.WriteAsync(segment, cancellationToken).ConfigureAwait(false);
}
_position = _sequence.End;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:44

• This change makes a previously-seekable public Stream (ReadOnlySequenceStream is in System.Memory ref) report CanSeek == false and throw NotSupportedException from Length/Position/Seek. That’s a behavioral breaking change for existing consumers.

The PR description doesn’t reference a breaking-change tracking issue. Per docs/project/breaking-change-process.md, please link/create an issue marked breaking-change (with before/after behavior and mitigations) and reference it from the PR so reviewers can evaluate compatibility impact.

 // segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:143

• CopyTo writes multiple segments but only updates _position after the loop completes. If destination.Write(...) throws after some segments were successfully written, the stream will keep its old _position even though bytes were already consumed, so subsequent reads / retries can duplicate data.

Update _position after each successfully-written segment so the stream’s internal read position always reflects the bytes already transferred.

This issue also appears on line 169 of the same file.

 {
destination.Write(segment.Span);
}
_position = _sequence.End;

[adamsitnik]

Overall it LGTM, but I wonder if we should improve the XML doc comments before merging. Thank you for fixing it before the .NET 11 release @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:45

• Changing ReadOnlySequenceStream to be non-seekable (CanSeek=false, and Length/Position/Seek throwing NotSupportedException) is a behavioral breaking change for a public type (see ref/System.Memory.cs). Per repo guidelines, this should go through the breaking-change process and include the required documentation/metadata updates (e.g., add the appropriate breaking-change entry per docs/project/breaking-change-process.md).

 /// <summary>Gets a value indicating whether the <see cref="ReadOnlySequenceStream"/> supports seeking.</summary>
// Keep this intentionally non-seekable: backward positioning requires traversing segments
// again from the beginning, making repeated seeks worst-case O(N). ReadOnlySequence<T>
// segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

[adamsitnik]

I left a comment, PTAL @jozkee

[ViveliDuCh]

LGTM. Thanks for the documentation effort!

[jozkee]

Summary

• clarify that ReadOnlyMemoryStream and WritableMemoryStream immediately expose their backing memory contents
• make ReadOnlySequenceStream intentionally non-seekable and remove its seek state and traversal logic
• keep StringStream intentionally non-seekable and document why backward positioning would require rerunning the encoder
• make Length, Position, and Seek consistently throw the standard unseekable-stream NotSupportedException
• update stream conformance and focused unit coverage
• For Writable memory stream, added ctor remarks suggesting users to clear rented or reused memory before constructing the stream if its existing contents should not be exposed.

Rationale

Backward positioning requires replaying work from the beginning. For ReadOnlySequenceStream, that means traversing segments whose boundaries may be indirectly controlled by an untrusted network client through packet framing. Even correct stitching can therefore produce adversarial fragmentation, and consumers must tolerate the worst technically compliant segmentation rather than assuming ASP.NET-like behavior. For StringStream, backward positioning requires rerunning the encoder. Repeated seeks can turn both cases into worst-case O(N) work.

Validation

• checked and Release CoreLib builds
• System.Memory build
• 405 focused ReadOnlySequenceStream conformance tests
• 275 focused StringStream conformance/unit tests

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR aligns several Stream implementations with an intentionally non-seekable contract (notably ReadOnlySequenceStream, and reinforcing StringStream expectations) and updates docs/tests accordingly, including clarifying that the memory-backed streams expose existing buffer contents immediately.

Changes:

• Make ReadOnlySequenceStream intentionally non-seekable by removing seek state/traversal and having Length/Position/Seek throw NotSupportedException consistently.
• Clarify ReadOnlyMemoryStream / WritableMemoryStream docs that backing memory contents are immediately readable.
• Update conformance and focused unit tests to reflect the non-seekable behavior and exception expectations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/libraries/System.Runtime/tests/System.IO.Tests/StringStream/StringStreamTests_String.cs	Extends capability tests to assert unseekable members throw NotSupportedException.
src/libraries/System.Private.CoreLib/src/System/IO/WritableMemoryStream.cs	Doc updates clarifying immediate exposure of existing buffer contents.
src/libraries/System.Private.CoreLib/src/System/IO/StringStream.cs	Adds rationale comment for keeping the stream non-seekable.
src/libraries/System.Private.CoreLib/src/System/IO/ReadOnlyMemoryStream.cs	Doc updates clarifying immediate exposure (and minor wording tweak needed).
src/libraries/System.Memory/tests/ReadOnlyBuffer/ReadOnlySequenceStream.ConformanceTests.cs	Updates conformance configuration to treat the stream as non-seekable; removes seek-specific override.
src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs	Implements non-seekable contract; removes seek/position bookkeeping and logic.
src/libraries/System.Memory/src/Resources/Strings.resx	Adds NotSupported_UnseekableStream resource; removes no-longer-needed seek-related resource strings.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:173

• CopyToAsyncCore only assigns _position = _sequence.End after the full loop completes. If WriteAsync throws/cancels after some segments were written, the stream’s _position stays at the original value and subsequent reads can replay already-transferred bytes.

Advance _position after each successfully completed WriteAsync call.

 {
await destination.WriteAsync(segment, cancellationToken).ConfigureAwait(false);
}
_position = _sequence.End;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:44

• This change makes a previously-seekable public Stream (ReadOnlySequenceStream is in System.Memory ref) report CanSeek == false and throw NotSupportedException from Length/Position/Seek. That’s a behavioral breaking change for existing consumers.

The PR description doesn’t reference a breaking-change tracking issue. Per docs/project/breaking-change-process.md, please link/create an issue marked breaking-change (with before/after behavior and mitigations) and reference it from the PR so reviewers can evaluate compatibility impact.

 // segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:143

• CopyTo writes multiple segments but only updates _position after the loop completes. If destination.Write(...) throws after some segments were successfully written, the stream will keep its old _position even though bytes were already consumed, so subsequent reads / retries can duplicate data.

Update _position after each successfully-written segment so the stream’s internal read position always reflects the bytes already transferred.

This issue also appears on line 169 of the same file.

 {
destination.Write(segment.Span);
}
_position = _sequence.End;

[adamsitnik]

Overall it LGTM, but I wonder if we should improve the XML doc comments before merging. Thank you for fixing it before the .NET 11 release @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:45

• Changing ReadOnlySequenceStream to be non-seekable (CanSeek=false, and Length/Position/Seek throwing NotSupportedException) is a behavioral breaking change for a public type (see ref/System.Memory.cs). Per repo guidelines, this should go through the breaking-change process and include the required documentation/metadata updates (e.g., add the appropriate breaking-change entry per docs/project/breaking-change-process.md).

 /// <summary>Gets a value indicating whether the <see cref="ReadOnlySequenceStream"/> supports seeking.</summary>
// Keep this intentionally non-seekable: backward positioning requires traversing segments
// again from the beginning, making repeated seeks worst-case O(N). ReadOnlySequence<T>
// segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

[adamsitnik]

I left a comment, PTAL @jozkee

[ViveliDuCh]

LGTM. Thanks for the documentation effort!

[jozkee]

Summary

• clarify that ReadOnlyMemoryStream and WritableMemoryStream immediately expose their backing memory contents
• make ReadOnlySequenceStream intentionally non-seekable and remove its seek state and traversal logic
• keep StringStream intentionally non-seekable and document why backward positioning would require rerunning the encoder
• make Length, Position, and Seek consistently throw the standard unseekable-stream NotSupportedException
• update stream conformance and focused unit coverage
• For Writable memory stream, added ctor remarks suggesting users to clear rented or reused memory before constructing the stream if its existing contents should not be exposed.

Rationale

Backward positioning requires replaying work from the beginning. For ReadOnlySequenceStream, that means traversing segments whose boundaries may be indirectly controlled by an untrusted network client through packet framing. Even correct stitching can therefore produce adversarial fragmentation, and consumers must tolerate the worst technically compliant segmentation rather than assuming ASP.NET-like behavior. For StringStream, backward positioning requires rerunning the encoder. Repeated seeks can turn both cases into worst-case O(N) work.

Validation

• checked and Release CoreLib builds
• System.Memory build
• 405 focused ReadOnlySequenceStream conformance tests
• 275 focused StringStream conformance/unit tests

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR aligns several Stream implementations with an intentionally non-seekable contract (notably ReadOnlySequenceStream, and reinforcing StringStream expectations) and updates docs/tests accordingly, including clarifying that the memory-backed streams expose existing buffer contents immediately.

Changes:

• Make ReadOnlySequenceStream intentionally non-seekable by removing seek state/traversal and having Length/Position/Seek throw NotSupportedException consistently.
• Clarify ReadOnlyMemoryStream / WritableMemoryStream docs that backing memory contents are immediately readable.
• Update conformance and focused unit tests to reflect the non-seekable behavior and exception expectations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/libraries/System.Runtime/tests/System.IO.Tests/StringStream/StringStreamTests_String.cs	Extends capability tests to assert unseekable members throw NotSupportedException.
src/libraries/System.Private.CoreLib/src/System/IO/WritableMemoryStream.cs	Doc updates clarifying immediate exposure of existing buffer contents.
src/libraries/System.Private.CoreLib/src/System/IO/StringStream.cs	Adds rationale comment for keeping the stream non-seekable.
src/libraries/System.Private.CoreLib/src/System/IO/ReadOnlyMemoryStream.cs	Doc updates clarifying immediate exposure (and minor wording tweak needed).
src/libraries/System.Memory/tests/ReadOnlyBuffer/ReadOnlySequenceStream.ConformanceTests.cs	Updates conformance configuration to treat the stream as non-seekable; removes seek-specific override.
src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs	Implements non-seekable contract; removes seek/position bookkeeping and logic.
src/libraries/System.Memory/src/Resources/Strings.resx	Adds NotSupported_UnseekableStream resource; removes no-longer-needed seek-related resource strings.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:173

• CopyToAsyncCore only assigns _position = _sequence.End after the full loop completes. If WriteAsync throws/cancels after some segments were written, the stream’s _position stays at the original value and subsequent reads can replay already-transferred bytes.

Advance _position after each successfully completed WriteAsync call.

 {
await destination.WriteAsync(segment, cancellationToken).ConfigureAwait(false);
}
_position = _sequence.End;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:44

• This change makes a previously-seekable public Stream (ReadOnlySequenceStream is in System.Memory ref) report CanSeek == false and throw NotSupportedException from Length/Position/Seek. That’s a behavioral breaking change for existing consumers.

The PR description doesn’t reference a breaking-change tracking issue. Per docs/project/breaking-change-process.md, please link/create an issue marked breaking-change (with before/after behavior and mitigations) and reference it from the PR so reviewers can evaluate compatibility impact.

 // segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:143

• CopyTo writes multiple segments but only updates _position after the loop completes. If destination.Write(...) throws after some segments were successfully written, the stream will keep its old _position even though bytes were already consumed, so subsequent reads / retries can duplicate data.

Update _position after each successfully-written segment so the stream’s internal read position always reflects the bytes already transferred.

This issue also appears on line 169 of the same file.

 {
destination.Write(segment.Span);
}
_position = _sequence.End;

[adamsitnik]

Overall it LGTM, but I wonder if we should improve the XML doc comments before merging. Thank you for fixing it before the .NET 11 release @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Memory/src/System/Buffers/ReadOnlySequenceStream.cs:45

• Changing ReadOnlySequenceStream to be non-seekable (CanSeek=false, and Length/Position/Seek throwing NotSupportedException) is a behavioral breaking change for a public type (see ref/System.Memory.cs). Per repo guidelines, this should go through the breaking-change process and include the required documentation/metadata updates (e.g., add the appropriate breaking-change entry per docs/project/breaking-change-process.md).

 /// <summary>Gets a value indicating whether the <see cref="ReadOnlySequenceStream"/> supports seeking.</summary>
// Keep this intentionally non-seekable: backward positioning requires traversing segments
// again from the beginning, making repeated seeks worst-case O(N). ReadOnlySequence<T>
// segment boundaries may be indirectly controlled by an untrusted network client through
// packet framing, so even correct stitching logic can produce adversarial fragmentation.
// Consumers must remain resilient against the worst technically compliant implementation
// rather than assuming ASP.NET-like segmentation.
public override bool CanSeek => false;

[adamsitnik]

I left a comment, PTAL @jozkee

[ViveliDuCh]

LGTM. Thanks for the documentation effort!