Skip to content

Stop modifying HttpClient.DefaultRequestHeaders#2363

Merged
alexeyzimarev merged 2 commits intodevfrom
2156-do-not-modify-default-headers
Mar 1, 2026
Merged

Stop modifying HttpClient.DefaultRequestHeaders#2363
alexeyzimarev merged 2 commits intodevfrom
2156-do-not-modify-default-headers

Conversation

@alexeyzimarev
Copy link
Member

Summary

Closes #2156

  • Removed ConfigureHttpClient method that was setting ExpectContinue on HttpClient.DefaultRequestHeaders, which mutated shared HttpClient instances
  • Moved Expect100Continue to per-request HttpRequestMessage.Headers.ExpectContinue in ExecuteRequestAsync
  • Removed the now-unnecessary .ConfigureHttpClient(...) delegate from the DI extension

This completes the work started when UserAgent was moved to DefaultParameters. RestSharp no longer modifies HttpClient.DefaultRequestHeaders at all.

Test plan

  • Added test: shared HttpClient default headers are not modified when Expect100Continue is set
  • Added test: new HttpClient default headers are not modified when Expect100Continue is set
  • All 542 existing tests pass (unit + integration + serializer + DI)

🤖 Generated with Claude Code

Move Expect100Continue from HttpClient.DefaultRequestHeaders to
per-request HttpRequestMessage.Headers so that shared HttpClient
instances are not mutated by RestClient configuration.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@qodo-free-for-open-source-projects
Copy link
Contributor

Review Summary by Qodo

Stop modifying HttpClient.DefaultRequestHeaders

✨ Enhancement 🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Remove ConfigureHttpClient method that mutated shared HttpClient.DefaultRequestHeaders
• Move Expect100Continue setting from HttpClient to per-request HttpRequestMessage.Headers
• Prevent modification of shared HttpClient instances across multiple RestClient instances
• Add tests verifying HttpClient.DefaultRequestHeaders remain unmodified
Diagram
flowchart LR
  A["RestClient Configuration"] -->|Previously| B["Modified HttpClient.DefaultRequestHeaders"]
  A -->|Now| C["Per-Request HttpRequestMessage.Headers"]
  B -->|Issue| D["Shared HttpClient Mutation"]
  C -->|Solution| E["Isolated Request Configuration"]
Loading

Grey Divider

File Changes

1. src/RestSharp.Extensions.DependencyInjection/ServiceCollectionExtensions.cs ✨ Enhancement +0/-1

Remove ConfigureHttpClient call from DI extension

• Removed .ConfigureHttpClient(client => RestClient.ConfigureHttpClient(client, options)) call
 from DI extension
• Simplified AddRestClient method by eliminating the now-unnecessary ConfigureHttpClient
 delegate

src/RestSharp.Extensions.DependencyInjection/ServiceCollectionExtensions.cs


2. src/RestSharp/RestClient.Async.cs ✨ Enhancement +5/-4

Move ExpectContinue to per-request headers

• Added message.Headers.ExpectContinue = Options.Expect100Continue to set header on per-request
 basis
• Aligned formatting of message header assignments for consistency

src/RestSharp/RestClient.Async.cs


3. src/RestSharp/RestClient.cs ✨ Enhancement +0/-6

Remove ConfigureHttpClient method and calls

• Removed ConfigureHttpClient(httpClient, options) call from GetClient() method
• Removed ConfigureHttpClient(httpClient, options) call from constructor with HttpClient
 parameter
• Deleted internal ConfigureHttpClient static method entirely

src/RestSharp/RestClient.cs


View more (1)
4. test/RestSharp.Tests/RestClientTests.cs 🧪 Tests +25/-0

Add tests for HttpClient header isolation

• Added test Should_not_set_expect_continue_on_shared_http_client_default_headers verifying shared
 HttpClient is not modified
• Added test Should_not_set_expect_continue_on_new_http_client_default_headers verifying new
 HttpClient is not modified
• Both tests assert HttpClient.DefaultRequestHeaders.ExpectContinue remains null

test/RestSharp.Tests/RestClientTests.cs


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Contributor

qodo-free-for-open-source-projects bot commented Mar 1, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. ExpectContinue missing on redirects🐞 Bug ✓ Correctness
Description
RestClientOptions.Expect100Continue is applied to the initial HttpRequestMessage but not to redirect
follow-up HttpRequestMessages created during manual redirect handling. As a result, redirected
requests may ignore the Expect100Continue option (behavior regression vs when it was applied via
HttpClient default headers).
Code

src/RestSharp/RestClient.Async.cs[141]

+        message.Headers.ExpectContinue = Options.Expect100Continue;
Evidence
The client option exists to control the request Expect/100-continue behavior. The initial request
explicitly sets HttpRequestMessage.Headers.ExpectContinue, but redirects are performed manually
(AllowAutoRedirect=false) by creating a brand-new HttpRequestMessage in CreateRedirectMessage, which
does not set ExpectContinue; therefore, the option is not consistently applied across all requests
for a single execution when redirects occur.

src/RestSharp/Options/RestClientOptions.cs[164-167]
src/RestSharp/RestClient.Async.cs[137-143]
src/RestSharp/RestClient.cs[225-247]
src/RestSharp/RestClient.Async.cs[180-209]
src/RestSharp/RestClient.Async.cs[275-289]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RestClientOptions.Expect100Continue` is only applied to the initial `HttpRequestMessage` in `ExecuteRequestAsync`. When redirects are followed, RestSharp creates a new `HttpRequestMessage` in `CreateRedirectMessage`, but it does not set `Headers.ExpectContinue`, so redirected requests can ignore the option.
### Issue Context
RestSharp performs redirects manually (`AllowAutoRedirect = false`) and uses `CreateRedirectMessage` for each hop.
### Fix Focus Areas
- src/RestSharp/RestClient.Async.cs[265-290]
- src/RestSharp/RestClient.Async.cs[137-143]
- test/RestSharp.Tests.Integrated/CookieRedirectTests.cs[194-220] (or add a new integrated redirect test using `/echo-request` to assert the `Expect` header on the final hop)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Mar 1, 2026

Deploying restsharp with  Cloudflare Pages  Cloudflare Pages

Latest commit: d310886
Status: ✅  Deploy successful!
Preview URL: https://3dc9e9bf.restsharp.pages.dev
Branch Preview URL: https://2156-do-not-modify-default-h.restsharp.pages.dev

View logs

@github-actions
Copy link

github-actions bot commented Mar 1, 2026

Test Results

   42 files     42 suites   17m 42s ⏱️
  543 tests   543 ✅ 0 💤 0 ❌
3 789 runs  3 789 ✅ 0 💤 0 ❌

Results for commit d310886.

♻️ This comment has been updated with latest results.

Apply Options.Expect100Continue to HttpRequestMessages created during
redirect handling, matching the behavior of the initial request.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 1, 2026

@alexeyzimarev alexeyzimarev merged commit 8cafdd1 into dev Mar 1, 2026
12 checks passed
@alexeyzimarev alexeyzimarev deleted the 2156-do-not-modify-default-headers branch March 1, 2026 16:26
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.

Remove modification of HttpClient DefaultRequestHeaders

1 participant