Skip to content

Validate stream receive buffer size - #6302

Open
Gaurav Singh (gaurav2699) wants to merge 1 commit into
mainfrom
user/gaurasingh/fix-stream-recv-buffer-validation
Open

Validate stream receive buffer size#6302
Gaurav Singh (gaurav2699) wants to merge 1 commit into
mainfrom
user/gaurasingh/fix-stream-recv-buffer-validation

Conversation

@gaurav2699

Copy link
Copy Markdown
Contributor

Description

Reject StreamRecvBufferDefault values that are not powers of two before they reach receive-buffer initialization. This aligns public settings application with persisted-settings validation and prevents the assertion exposed by the randomized settings coverage in #6204.

Failure: https://github.com/microsoft/msquic/actions/runs/33798384900/job/100793494460?pr=6204

Testing

  • msquiccoretest.exe --gtest_filter='SettingsTest.StreamRecv*'
  • msquictest.exe --gtest_filter='ParameterValidation.ValidateConfigurationParam:ParameterValidation.ValidateConnectionParam'

Added core coverage that verifies invalid input leaves destination settings unchanged, plus public configuration and connection API coverage for invalid and valid buffer sizes.

Documentation

No documentation impact.

Reject non-power-of-two stream receive buffer defaults before they reach receive buffer initialization, and cover core and public API validation paths.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gaurav2699
Gaurav Singh (gaurav2699) requested a review from a team as a code owner September 4, 2026 18:40
@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.08%. Comparing base (ceba4ec) to head (5a9e0a9).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6302      +/-   ##
==========================================
- Coverage   85.96%   85.08%   -0.89%     
==========================================
  Files          60       60              
  Lines       18976    18976              
==========================================
- Hits        16313    16145     -168     
- Misses       2663     2831     +168     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This changes which values the API accepts.
Was it documented only power of two are accepted? Is the requirement for power of two a consequence of a recent fix on the buffer growth logic or was it there before?

We need to be careful this won't regress a functioning scenario for apps.

@gaurav2699

Copy link
Copy Markdown
Contributor Author

This changes which values the API accepts. Was it documented only power of two are accepted? Is the requirement for power of two a consequence of a recent fix on the buffer growth logic or was it there before?

We need to be careful this won't regress a functioning scenario for apps.

Yes, the power-of-two assertion has existed internally since the original receive-buffer implementation, but the requirement was never documented for  StreamRecvBufferDefault . The public settings path has accepted all values >=4096 since 2020, while registry loading separately enforced power-of-two values.
This was exposed because #6204 started randomizing the previously-disabled settings. I agree rejecting previously accepted API values could regress applications. Debug builds assert these values and release builds accept them. We should in that case remove the debug assert?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants