Skip to content

Conversation

@SolariSystems
Copy link

@SolariSystems SolariSystems commented Feb 9, 2026

Proposed changes

Closes #2240

When a user passes -pr http11, httpx correctly configures the primary transport to disable HTTP/2 (TLSNextProto = {}, GODEBUG=http2client=0). However, retryablehttp-go maintains a secondary HTTPClient2 that is always configured with native HTTP/2 support. When the primary HTTP/1.1 request encounters certain protocol-mismatch errors (e.g. malformed HTTP version "HTTP/2"), retryablehttp silently retries the request using HTTPClient2, which speaks HTTP/2 -- completely bypassing the user's explicit protocol selection.

Fix: After constructing the retryablehttp client, when Protocol == HTTP11, set HTTPClient2 = HTTPClient. This makes the fallback path use the same HTTP/1.1-only client, so retryablehttp can never silently upgrade the protocol.

Before: -pr http11 is ignored on servers that trigger retryablehttp's HTTP/2 fallback path (do.go:62-66). The request silently completes over HTTP/2 with a different transport that does not share the primary client's proxy, dialer, redirect, or TLS configuration.

After: -pr http11 is strictly honored. The fallback client is the same object as the primary client, so all protocol-mismatch retries still use HTTP/1.1 with the user's configured transport settings.

Proof

$ go test ./common/httpx/ -count=1 -v -run "TestHTTP11|TestDo"
=== RUN   TestDo
=== RUN   TestDo/content-length_in_header
=== RUN   TestDo/content-length_with_binary_body
--- PASS: TestDo (0.04s)
    --- PASS: TestDo/content-length_in_header (0.00s)
    --- PASS: TestDo/content-length_with_binary_body (0.00s)
=== RUN   TestHTTP11DisablesHTTP2Fallback
=== RUN   TestHTTP11DisablesHTTP2Fallback/http11_protocol_pins_fallback_client
=== RUN   TestHTTP11DisablesHTTP2Fallback/default_protocol_keeps_separate_fallback_client
--- PASS: TestHTTP11DisablesHTTP2Fallback (0.07s)
    --- PASS: TestHTTP11DisablesHTTP2Fallback/http11_protocol_pins_fallback_client (0.04s)
    --- PASS: TestHTTP11DisablesHTTP2Fallback/default_protocol_keeps_separate_fallback_client (0.03s)
PASS
ok      github.com/projectdiscovery/httpx/common/httpx  0.158s

Full suite:

$ go test ./... -count=1
ok      github.com/projectdiscovery/httpx/common/authprovider       0.010s
ok      github.com/projectdiscovery/httpx/common/authprovider/authx 0.012s
ok      github.com/projectdiscovery/httpx/common/httpx              0.224s
ok      github.com/projectdiscovery/httpx/common/inputformats       0.003s
ok      github.com/projectdiscovery/httpx/common/pagetypeclassifier 0.133s
ok      github.com/projectdiscovery/httpx/runner                    0.970s

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Summary by CodeRabbit

  • Bug Fixes

    • Respects explicit HTTP/1.1 requests and prevents unintended HTTP/2 fallback.
  • Tests

    • Improved test coverage using local test servers to validate content-length handling and protocol fallback behavior.

@auto-assign auto-assign bot requested a review from dogancanbakir February 9, 2026 17:04
@coderabbitai
Copy link

coderabbitai bot commented Feb 9, 2026

Walkthrough

Replaces string protocol check with a constant and, when HTTP/1.1 is explicitly selected, aliases the retryablehttp fallback client (HTTPClient2) to the HTTP/1.1 client to prevent automatic HTTP/2 fallback. Tests moved to local httptest servers and add coverage for Content-Length and fallback behavior.

Changes

Cohort / File(s) Summary
HTTP/1.1 Protocol Guard
common/httpx/httpx.go
Use HTTP11 constant for protocol comparison; after building the HTTP client, when protocol is HTTP/1.1 set HTTPClient2 = HTTPClient to disable retryablehttp's HTTP/2 fallback.
Test Coverage Expansion
common/httpx/httpx_test.go
Switch tests to httptest servers, add Content-Length/body cases, and add TestHTTP11DisablesHTTP2Fallback asserting HTTPClient2 is pinned to HTTPClient for HTTP/1.1 and remains independent by default.

Sequence Diagram(s)

sequenceDiagram
    participant Test as Test
    participant HTTPX as HTTPX (build client)
    participant Retryable as retryablehttp (Do)
    participant Server as Origin Server

    rect rgba(200,200,255,0.5)
    Test->>HTTPX: configure Protocol=HTTP11
    HTTPX->>Retryable: create HTTPClient (http1-only)
    HTTPX-->>Retryable: set HTTPClient2 = HTTPClient
    end

    Test->>Retryable: Do(Request)
    Retryable->>Server: send request (HTTP/1.1)
    Server-->>Retryable: respond (OK)
    Retryable-->>Test: return response (no HTTP/2 fallback)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I’m a rabbit in a curious patch of code,
I hop where protocols once freely rode,
I pin HTTP/1.1 so it won’t stray,
Tests stand guard along the way,
Hooray for steady hopping mode! 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: disabling retryablehttp's HTTP/2 fallback when HTTP/1.1 is explicitly requested via the -pr flag.
Linked Issues check ✅ Passed The code changes directly address the core issue #2240 by setting HTTPClient2 = HTTPClient when Protocol == HTTP11, preventing silent fallback to HTTP/2 and respecting the -pr http11 flag.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the HTTP/1.1 protocol enforcement: constant replacement, fallback client aliasing, and corresponding test coverage with no extraneous modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

-pr http11 flag is ignored on retryablehttp-go due to HTTP/2 fallback

1 participant