fix: respect explicit HTTP/1.1 configuration by disabling HTTP/2 fallback#2433
fix: respect explicit HTTP/1.1 configuration by disabling HTTP/2 fallback#2433hetbhatt2807 wants to merge 11 commits intoprojectdiscovery:devfrom
Conversation
Add custom CheckRetry function for HTTP/1.1 with fallback control.
Added instructions for disabling HTTP/2 fallback with the -dhf flag.
WalkthroughThese changes introduce a DisableHTTPFallback feature across the httpx client, runner, and options modules. When enabled with the --dhf flag and HTTP/1.1 protocol, the client prevents automatic HTTP/2 fallback retries. A custom retry policy, helper function for error detection, comprehensive tests, and documentation support this functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RetryHandler as Retry Handler
participant ErrorDetector as Error Detector
Client->>RetryHandler: Request fails with error
RetryHandler->>ErrorDetector: isHTTP2FallbackError(err)?
ErrorDetector-->>RetryHandler: true/false
alt DisableHTTPFallback=true AND Protocol=http11 AND HTTP/2 Fallback Error
RetryHandler-->>Client: Retry=false, return error
else DisableHTTPFallback=false OR Non-Fallback Error
RetryHandler->>RetryHandler: Apply DefaultRetryPolicy
RetryHandler-->>Client: Retry decision from default policy
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
common/httpx/httpx_disablehttpfallback_test.go (1)
18-24: Add one constructor-path test forHTTPX.New(...)wiring.These tests validate the retry helper path, but they don’t assert that
common/httpx/httpx.goactually wireshttpx.client.CheckRetrycorrectly whenProtocol=http11andDisableHTTPFallback=true. Add one integration-style test throughNew(...)to prevent regressions in the real runtime path.Also applies to: 45-51, 69-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/httpx/httpx_disablehttpfallback_test.go` around lines 18 - 24, Add an integration-style test that constructs the HTTPX client via HTTPX.New(...) (not the lower-level constructor) with options Protocol=http11 and DisableHTTPFallback=true, then obtain the retry helper via the exported NewCheckRetryFunc (or inspect httpx.client.CheckRetry) and assert it behaves as expected (e.g., non-nil and enforces no-fallback semantics); this ensures common/httpx/httpx.go actually wires the client's CheckRetry when using the real New(...) path and prevents regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/httpx/httpx.go`:
- Around line 475-486: The getCustomCheckRetry function is currently nested
inside the Sanitize function which causes a compile error; move the entire
getCustomCheckRetry declaration so it is a top-level (file-level) function (not
inside Sanitize), keeping its signature getCustomCheckRetry(opts *Options)
retryablehttp.CheckRetryFunc and its use of isHTTP2FallbackError and
retryablehttp.DefaultRetryPolicy intact; ensure Sanitize returns before the
function and update any references to getCustomCheckRetry to use the now
top-level function.
In `@common/httpx/retry.go`:
- Around line 8-9: The code imports two different retryablehttp packages causing
incompatible types and duplicated helpers; unify to the same import (use
github.com/projectdiscovery/retryablehttp-go) across files and update
signatures/usages accordingly: change the import in retry.go to
github.com/projectdiscovery/retryablehttp-go, ensure NewCheckRetryFunc returns
projectdiscovery's retryablehttp.CheckRetryFunc, update any callers (e.g.,
getCustomCheckRetry in httpx.go) to accept that type, and consolidate
isHTTP2FallbackError so it is referenced from the same package (or moved to a
shared/internal helper) to eliminate cross-package type mismatches.
In `@README.md`:
- Around line 292-299: The "### Disable HTTP/2 Fallback" heading breaks
list/heading nesting in README.md; replace that heading with inline explanatory
text (or a proper section header outside any list) and format the example as a
fenced bash block; specifically update the "Disable HTTP/2 Fallback" block to
read like a bullet or sentence (e.g., "Disable HTTP/2 fallback: Use `-dhf` to
prevent automatic fallback when running in HTTP/1.1 mode.") and keep the usage
example `httpx -u http://target.com -pr http11 -dhf` inside a ```bash fenced
code block so the README no longer has the invalid heading jump.
---
Nitpick comments:
In `@common/httpx/httpx_disablehttpfallback_test.go`:
- Around line 18-24: Add an integration-style test that constructs the HTTPX
client via HTTPX.New(...) (not the lower-level constructor) with options
Protocol=http11 and DisableHTTPFallback=true, then obtain the retry helper via
the exported NewCheckRetryFunc (or inspect httpx.client.CheckRetry) and assert
it behaves as expected (e.g., non-nil and enforces no-fallback semantics); this
ensures common/httpx/httpx.go actually wires the client's CheckRetry when using
the real New(...) path and prevents regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 89bf6dbe-27b1-4a6f-beb4-3739b3105bca
📒 Files selected for processing (6)
README.mdcommon/httpx/httpx.gocommon/httpx/httpx_disablehttpfallback_test.gocommon/httpx/option.gocommon/httpx/retry.gorunner/options.go
| // getCustomCheckRetry returns a custom CheckRetry function that respects DisableHTTPFallback | ||
| func getCustomCheckRetry(opts *Options) retryablehttp.CheckRetryFunc { | ||
| return func(ctx context.Context, resp *http.Response, err error) (bool, error) { | ||
| if opts.Protocol == "http11" && opts.DisableHTTPFallback { | ||
| if isHTTP2FallbackError(err) { | ||
| return false, err | ||
| } | ||
| } | ||
| return retryablehttp.DefaultRetryPolicy(ctx, resp, err) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if file exists and get its size
if [ -f "common/httpx/httpx.go" ]; then
wc -l common/httpx/httpx.go
# Show lines 455-510 with context around the cited lines 475-486
nl -ba common/httpx/httpx.go | sed -n '455,510p'
else
echo "File not found at common/httpx/httpx.go"
find . -name "httpx.go" -type f 2>/dev/null || echo "No httpx.go found"
fiRepository: projectdiscovery/httpx
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# View lines 455-486 using sed for line numbers
sed -n '455,486p' common/httpx/httpx.go | awk '{printf "%4d %s\n", NR+454, $0}'Repository: projectdiscovery/httpx
Length of output: 1178
Move getCustomCheckRetry out of Sanitize (compile blocker).
Go does not support named function declarations inside other functions. The getCustomCheckRetry function at lines 476-485 is incorrectly nested within the Sanitize method (which closes at line 486), appearing after the return statement at line 474. This is a syntax error and will not compile.
🐛 Proposed fix
func (httpx *HTTPX) Sanitize(respStr string, trimLine, normalizeSpaces bool) string {
respStr = httpx.htmlPolicy.Sanitize(respStr)
if trimLine {
respStr = strings.ReplaceAll(respStr, "\n", "")
}
if normalizeSpaces {
respStr = httputilz.NormalizeSpaces(respStr)
}
return respStr
- // getCustomCheckRetry returns a custom CheckRetry function that respects DisableHTTPFallback
- func getCustomCheckRetry(opts *Options) retryablehttp.CheckRetryFunc {
- return func(ctx context.Context, resp *http.Response, err error) (bool, error) {
- if opts.Protocol == "http11" && opts.DisableHTTPFallback {
- if isHTTP2FallbackError(err) {
- return false, err
- }
- }
- return retryablehttp.DefaultRetryPolicy(ctx, resp, err)
- }
-}
}
+
+// getCustomCheckRetry returns a custom CheckRetry function that respects DisableHTTPFallback.
+func getCustomCheckRetry(opts *Options) retryablehttp.CheckRetryFunc {
+ return func(ctx context.Context, resp *http.Response, err error) (bool, error) {
+ if opts.Protocol == "http11" && opts.DisableHTTPFallback && isHTTP2FallbackError(err) {
+ return false, err
+ }
+ return retryablehttp.DefaultRetryPolicy(ctx, resp, err)
+ }
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@common/httpx/httpx.go` around lines 475 - 486, The getCustomCheckRetry
function is currently nested inside the Sanitize function which causes a compile
error; move the entire getCustomCheckRetry declaration so it is a top-level
(file-level) function (not inside Sanitize), keeping its signature
getCustomCheckRetry(opts *Options) retryablehttp.CheckRetryFunc and its use of
isHTTP2FallbackError and retryablehttp.DefaultRetryPolicy intact; ensure
Sanitize returns before the function and update any references to
getCustomCheckRetry to use the now top-level function.
| "github.com/hashicorp/go-retryablehttp" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Retry-related imports in common/httpx ==="
rg -n 'go-retryablehttp|retryablehttp-go' common/httpx
echo
echo "=== CheckRetry assignments/usages ==="
rg -n '\bCheckRetry\b|NewCheckRetryFunc|getCustomCheckRetry' common/httpx
echo
echo "=== Client/NewClient symbols in common/httpx ==="
rg -nP 'type\s+Client\s+struct|func\s+NewClient\s*\(' common/httpxRepository: projectdiscovery/httpx
Length of output: 1275
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== common/httpx/retry.go (lines 1-35) ==="
cat -n common/httpx/retry.go | head -35
echo
echo "=== common/httpx/httpx.go (lines 180-195) ==="
sed -n '180,195p' common/httpx/httpx.go | cat -nRepository: projectdiscovery/httpx
Length of output: 1656
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking where NewCheckRetryFunc is used ==="
rg -n 'NewCheckRetryFunc' --type go
echo
echo "=== getCustomCheckRetry implementation (lines 475-500) ==="
sed -n '475,500p' common/httpx/httpx.go | cat -n
echo
echo "=== Checking imports in httpx.go (first 30 lines) ==="
head -30 common/httpx/httpx.go | cat -nRepository: projectdiscovery/httpx
Length of output: 2134
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking if isHTTP2FallbackError is in retry.go ==="
rg -n 'isHTTP2FallbackError' common/httpx
echo
echo "=== Checking DefaultRetryPolicy calls ==="
rg -n 'DefaultRetryPolicy' common/httpxRepository: projectdiscovery/httpx
Length of output: 609
Unify retryablehttp import path to avoid type incompatibility.
retry.go imports github.com/hashicorp/go-retryablehttp while httpx.go and other files use github.com/projectdiscovery/retryablehttp-go. This creates type drift: NewCheckRetryFunc() returns CheckRetryFunc from the hashicorp package, but getCustomCheckRetry() (in httpx.go) returns CheckRetryFunc from the projectdiscovery package—these are incompatible types even though they share the same signature. Additionally, isHTTP2FallbackError() is defined in retry.go but also used in httpx.go, creating inconsistent package references.
🔧 Proposed fix
import (
"context"
"net/http"
"strings"
- "github.com/hashicorp/go-retryablehttp"
+ retryablehttp "github.com/projectdiscovery/retryablehttp-go"
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "github.com/hashicorp/go-retryablehttp" | |
| ) | |
| import ( | |
| "context" | |
| "net/http" | |
| "strings" | |
| retryablehttp "github.com/projectdiscovery/retryablehttp-go" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@common/httpx/retry.go` around lines 8 - 9, The code imports two different
retryablehttp packages causing incompatible types and duplicated helpers; unify
to the same import (use github.com/projectdiscovery/retryablehttp-go) across
files and update signatures/usages accordingly: change the import in retry.go to
github.com/projectdiscovery/retryablehttp-go, ensure NewCheckRetryFunc returns
projectdiscovery's retryablehttp.CheckRetryFunc, update any callers (e.g.,
getCustomCheckRetry in httpx.go) to accept that type, and consolidate
isHTTP2FallbackError so it is referenced from the same package (or moved to a
shared/internal helper) to eliminate cross-package type mismatches.
| - ### Disable HTTP/2 Fallback | ||
|
|
||
| Use the `-dhf` flag to prevent automatic fallback to HTTP/2 when using HTTP/1.1 mode: | ||
| ```bash | ||
| httpx -u http://target.com -pr http11 -dhf | ||
| ``` | ||
|
|
||
| This ensures strict HTTP/1.1 protocol compliance and prevents automatic protocol switching during retries. |
There was a problem hiding this comment.
Fix heading nesting in Notes.
The - ### Disable HTTP/2 Fallback block introduces an invalid heading jump and odd list/heading structure. Convert it to a normal bullet (or promote to a proper section heading outside the list).
📝 Proposed markdown fix
-- ### Disable HTTP/2 Fallback
-
-Use the `-dhf` flag to prevent automatic fallback to HTTP/2 when using HTTP/1.1 mode:
-```bash
-httpx -u http://target.com -pr http11 -dhf
-```
-
-This ensures strict HTTP/1.1 protocol compliance and prevents automatic protocol switching during retries.
+- **Disable HTTP/2 fallback:** Use `-dhf` to prevent automatic fallback to HTTP/2 when running in HTTP/1.1 mode.
+ ```bash
+ httpx -u http://target.com -pr http11 -dhf
+ ```
+ This enforces strict HTTP/1.1 behavior during retries.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - ### Disable HTTP/2 Fallback | |
| Use the `-dhf` flag to prevent automatic fallback to HTTP/2 when using HTTP/1.1 mode: | |
| ```bash | |
| httpx -u http://target.com -pr http11 -dhf | |
| ``` | |
| This ensures strict HTTP/1.1 protocol compliance and prevents automatic protocol switching during retries. | |
| - **Disable HTTP/2 fallback:** Use `-dhf` to prevent automatic fallback to HTTP/2 when running in HTTP/1.1 mode. |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 292-292: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 292 - 299, The "### Disable HTTP/2 Fallback" heading
breaks list/heading nesting in README.md; replace that heading with inline
explanatory text (or a proper section header outside any list) and format the
example as a fenced bash block; specifically update the "Disable HTTP/2
Fallback" block to read like a bullet or sentence (e.g., "Disable HTTP/2
fallback: Use `-dhf` to prevent automatic fallback when running in HTTP/1.1
mode.") and keep the usage example `httpx -u http://target.com -pr http11 -dhf`
inside a ```bash fenced code block so the README no longer has the invalid
heading jump.
Mzack9999
left a comment
Mzack9999
left a comment
There was a problem hiding this comment.
Requested changes in the associated PR in projectdiscovery/retryablehttp-go#529
2 participants
This PR ensures that when -pr http11 is explicitly set,
httpx does not fallback to HTTP/2 via retryablehttp-go.
Problem:
retryablehttp-go falls back to HTTPClient2 on malformed HTTP/2 errors.
This overrides explicit HTTP/1.1-only configurations in httpx,
causing unintended protocol switches.
Solution:
Changes:
✓ common/httpx/options.go – added fallback control field
✓ common/httpx/retry.go – introduced fallback logic handling
✓ common/httpx/httpx.go – wired DisableHTTPFallback
✓ common/httpx/*_test.go – added unit tests
✓ README.md – documented behavior
✓ CHANGELOG.md – added entry
Testing:
This PR depends on projectdiscovery/retryablehttp-go#529.
Summary by CodeRabbit
Release Notes
New Features
--dhfflag to disable automatic HTTP/2 fallback, enforcing strict HTTP/1.1 protocol compliance.Documentation
Tests