Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5ba7ef4 to
f9204da
Compare
|
@hamiddd1980 can you please validate if this fixes the issue you've reported? |
f9204da to
bd6c067
Compare
There was a problem hiding this comment.
Pull request overview
This PR changes how System.Net.Http.Headers.HttpHeaders stores headers so that callers can add headers that were previously blocked by WebHeaderCollection’s restricted-header validation logic.
Changes:
- Initialize
HttpHeaders._headerStorewithnew WebHeaderCollection(false)instead oftrue, disabling restricted-header checks for these collections.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public abstract class HttpHeaders | ||
| { | ||
| internal WebHeaderCollection _headerStore = new WebHeaderCollection(true); | ||
| internal WebHeaderCollection _headerStore = new WebHeaderCollection(false); |
There was a problem hiding this comment.
Switching the backing store to new WebHeaderCollection(false) disables the restricted-header checks (see WebHeaderCollection.ThrowOnRestrictedHeader()), which changes behavior for known headers like Accept, Connection, Host, Content-Length, etc., not just custom X-* headers. Given HeaderInfoTable treats unknown headers as non-restricted, custom headers were already allowed before this change; please either update the PR title/description to reflect the broader behavior change, or narrow the change to only what’s needed for the reported issue.
| public abstract class HttpHeaders | ||
| { | ||
| internal WebHeaderCollection _headerStore = new WebHeaderCollection(true); | ||
| internal WebHeaderCollection _headerStore = new WebHeaderCollection(false); |
There was a problem hiding this comment.
With restricted-header validation disabled, callers can now set headers that other parts of this library treat as managed/internal. For example, HttpContentHeaders.ContentLength reads/parses the Content-Length header from _headerStore and will throw if a user sets a non-numeric value via content.Headers.Add("Content-Length", ...). Consider keeping restrictions for headers that must remain controlled (e.g., Content-Length, Host, Transfer-Encoding) or adding explicit validation/guardrails in HttpHeaders.Add to prevent inconsistent state.
| internal WebHeaderCollection _headerStore = new WebHeaderCollection(false); | |
| internal WebHeaderCollection _headerStore = new WebHeaderCollection(true); |
| public abstract class HttpHeaders | ||
| { | ||
| internal WebHeaderCollection _headerStore = new WebHeaderCollection(true); | ||
| internal WebHeaderCollection _headerStore = new WebHeaderCollection(false); |
There was a problem hiding this comment.
This line changes header-validation behavior globally for HttpRequestHeaders, HttpResponseHeaders, and HttpContentHeaders, but there are no unit tests asserting the new expected behavior. Please add tests that (1) verify previously-blocked standard headers (e.g., Accept) can now be added, and (2) define the intended behavior for managed headers like Content-Length (either still rejected or validated).
Description
Allow custom HTTP headers
Motivation and Context
How Has This Been Tested?
Screenshots
Types of changes
Checklist: