-
Notifications
You must be signed in to change notification settings - Fork 1k
fix: Honor -pr http11 flag by disabling HTTP/2 fallback (#2240) #2387
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
fix: Honor -pr http11 flag by disabling HTTP/2 fallback (#2240) #2387
Conversation
This patch adds the DisableHTTP2 option to retryablehttp when the user specifies -pr http11, preventing automatic fallback to HTTP/2. The patch should be applied to common/httpx/httpx.go after line 79.
Detailed guide on how to implement the fix for issue projectdiscovery#2240
Complete documentation of the fix including problem analysis, solution, and implementation details.
Shell script to automatically apply the fix to common/httpx/httpx.go
WalkthroughThis pull request adds comprehensive documentation and implementation guides for a fix that honors the HTTP/1.1-only flag by preventing automatic HTTP/2 fallback in retryablehttp-go. The changes include a detailed problem analysis, solution overview, implementation instructions, patch file, and an automated script to apply the fix. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apply-fix.sh`:
- Around line 22-28: The current use of sed -i is not portable on macOS; replace
the in-place sed invocation that appends the HTTP/1.1 block with a portable
tempfile pattern: create a temp file (using mktemp), run sed with the same
script (the block that checks httpx.Options.Protocol == "http11" and sets
retryablehttpOptions.DisableHTTP2 = true) redirecting output to the temp file,
then atomically mv the temp file back to FILE and clean up on failure; ensure
you preserve the exact appended lines and variable names (FILE,
httpx.Options.Protocol, retryablehttpOptions.DisableHTTP2) and handle errors
(non-zero exit) before replacing the original file.
🧹 Nitpick comments (5)
apply-fix.sh (2)
6-10: Add error handling for missing file.The script proceeds without checking if the target file exists, which could result in silent failures or confusing error messages.
Proposed fix
FILE="common/httpx/httpx.go" BACKUP="common/httpx/httpx.go.backup" +# Check if file exists +if [[ ! -f "$FILE" ]]; then + echo "Error: $FILE not found. Please run this script from the repository root." + exit 1 +fi + # Create backup cp "$FILE" "$BACKUP"
30-33: Consider verifying the insertion succeeded.The script assumes the sed pattern matched. If the anchor line doesn't exist (e.g., code was refactored), the fix won't be applied but the script will report success.
Proposed verification
echo "Fix applied to $FILE" echo "Backup saved to $BACKUP" echo "" +# Verify the fix was applied +if grep -q "DisableHTTP2 = true" "$FILE"; then + echo "✅ Verification: Fix successfully applied" +else + echo "⚠️ Warning: Could not verify fix was applied. The anchor line may have changed." + exit 1 +fi +echo "" echo "Please review the changes with: git diff $FILE"IMPLEMENTATION_GUIDE.md (1)
26-30: Consider adding a note about determining the correct version.The placeholder
v1.0.XXXis understandable since PR#521hasn't merged yet. Consider adding guidance on how to determine the correct version once available.Suggested clarification
### Step 2: Update go.mod Update the retryablehttp-go dependency to the version that includes the DisableHTTP2 option:-github.com/projectdiscovery/retryablehttp-go v1.0.XXX // version with DisableHTTP2 support
+github.com/projectdiscovery/retryablehttp-go v1.0.XXX // Replace XXX with actual version after PR#521merges+ +To find the correct version, check the [retryablehttp-go releases](https://github.com/projectdiscovery/retryablehttp-go/releases) after PR `#521` is merged.FIX_README.md (2)
19-27: Add language specifier to code block.The fenced code block is missing a language specifier, which affects syntax highlighting and accessibility.
Proposed fix
2. **retryablehttp-go Fallback (Lines 65-68 in retryablehttp-go/do.go):** - ``` + ```go // if err is equal to missing minor protocol version retry with http/2 if err != nil && stringsutil.ContainsAny(err.Error(), "net/http: HTTP/1.x transport connection broken: malformed HTTP version \"HTTP/2\"", "net/http: HTTP/1.x transport connection broken: malformed HTTP response") { resp, err = c.HTTPClient2.Do(req.Request) checkOK, checkErr = c.CheckRetry(req.Context(), resp, err) } ```
114-127: These PR documentation files should be removed once merged to avoid clutter in the main branch.The files (
FIX_README.md,IMPLEMENTATION_GUIDE.md,PATCH_NOTES.md,fix-http11.patch,apply-fix.sh) are implementation guides for a specific fix (issue#2240) and have already served their purpose—the actual code change is already applied incommon/httpx/httpx.go(lines 156-158). Once this PR is merged, these files become historical artifacts that won't be needed by future developers.If the detailed context about why the HTTP/2 fallback prevention exists is valuable, move that explanation into inline code comments or the main README; otherwise, remove these files before merging.
| sed -i '/retryablehttpOptions.Trace = options.Trace/a\ | ||
| \t// Disable HTTP/2 fallback when http11 protocol is explicitly requested\ | ||
| \t// This prevents retryablehttp from automatically retrying with HTTP/2\ | ||
| \t// when HTTP/1.x errors occur, honoring the user'"'"'s -pr http11 flag\ | ||
| \tif httpx.Options.Protocol == "http11" {\ | ||
| \t\tretryablehttpOptions.DisableHTTP2 = true\ | ||
| \t}' "$FILE" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sed -i is not portable across macOS and Linux.
On macOS (BSD sed), sed -i requires an empty string argument (sed -i ''), whereas GNU sed (Linux) does not. This script will fail on macOS with an error about extra characters after the command.
Proposed fix for cross-platform compatibility
-sed -i '/retryablehttpOptions.Trace = options.Trace/a\
+# Detect OS for sed compatibility
+if [[ "$OSTYPE" == "darwin"* ]]; then
+ SED_INPLACE="sed -i ''"
+else
+ SED_INPLACE="sed -i"
+fi
+
+$SED_INPLACE '/retryablehttpOptions.Trace = options.Trace/a\
\t// Disable HTTP/2 fallback when http11 protocol is explicitly requested\
\t// This prevents retryablehttp from automatically retrying with HTTP/2\
\t// when HTTP/1.x errors occur, honoring the user'"'"'s -pr http11 flag\
\tif httpx.Options.Protocol == "http11" {\
\t\tretryablehttpOptions.DisableHTTP2 = true\
\t}' "$FILE"📝 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.
| sed -i '/retryablehttpOptions.Trace = options.Trace/a\ | |
| \t// Disable HTTP/2 fallback when http11 protocol is explicitly requested\ | |
| \t// This prevents retryablehttp from automatically retrying with HTTP/2\ | |
| \t// when HTTP/1.x errors occur, honoring the user'"'"'s -pr http11 flag\ | |
| \tif httpx.Options.Protocol == "http11" {\ | |
| \t\tretryablehttpOptions.DisableHTTP2 = true\ | |
| \t}' "$FILE" | |
| # Detect OS for sed compatibility | |
| if [[ "$OSTYPE" == "darwin"* ]]; then | |
| SED_INPLACE="sed -i ''" | |
| else | |
| SED_INPLACE="sed -i" | |
| fi | |
| $SED_INPLACE '/retryablehttpOptions.Trace = options.Trace/a\ | |
| \t// Disable HTTP/2 fallback when http11 protocol is explicitly requested\ | |
| \t// This prevents retryablehttp from automatically retrying with HTTP/2\ | |
| \t// when HTTP/1.x errors occur, honoring the user'"'"'s -pr http11 flag\ | |
| \tif httpx.Options.Protocol == "http11" {\ | |
| \t\tretryablehttpOptions.DisableHTTP2 = true\ | |
| \t}' "$FILE" |
🤖 Prompt for AI Agents
In `@apply-fix.sh` around lines 22 - 28, The current use of sed -i is not portable
on macOS; replace the in-place sed invocation that appends the HTTP/1.1 block
with a portable tempfile pattern: create a temp file (using mktemp), run sed
with the same script (the block that checks httpx.Options.Protocol == "http11"
and sets retryablehttpOptions.DisableHTTP2 = true) redirecting output to the
temp file, then atomically mv the temp file back to FILE and clean up on
failure; ensure you preserve the exact appended lines and variable names (FILE,
httpx.Options.Protocol, retryablehttpOptions.DisableHTTP2) and handle errors
(non-zero exit) before replacing the original file.
Description
Fixes #2240 ($100 bounty)
This PR ensures that httpx honors the
-pr http11flag by preventing automatic fallback to HTTP/2 in the retryablehttp-go library.Problem
When using httpx with
-pr http11to enforce HTTP/1.1-only communication, the flag was being ignored because retryablehttp-go automatically falls back to HTTP/2 when encountering certain HTTP/1.x protocol errors.Root Cause
httpx correctly disables HTTP/2 in the main HTTP client (lines 156-160):
retryablehttp-go automatically retries with HTTP/2 on errors (do.go lines 65-68):
This bypasses httpx's HTTP/1.1-only configuration.
Solution
This PR adds support for the new
DisableHTTP2option in retryablehttp-go (see PR projectdiscovery/retryablehttp-go#521).Code Change
File:
common/httpx/httpx.goLocation: After line 79 (after
retryablehttpOptions.Trace = options.Trace)Add:
Implementation Notes
Due to file size limitations, this PR includes:
FIX_README.mdIMPLEMENTATION_GUIDE.mdfix-http11.patchapply-fix.shcommon/httpx/httpx.go(see documentation)The actual code modification is simple (5 lines) but needs to be applied manually or using the provided script.
Dependencies
go.modto use retryablehttp-go version withDisableHTTP2supportTesting
Before Fix:
$ httpx -u https://example.com -pr http11 -verbose # HTTP/2 is used despite -pr http11 flag ❌After Fix:
$ httpx -u https://example.com -pr http11 -verbose # Only HTTP/1.1 is used ✅Backward Compatibility
✅ Fully backward compatible:
-pr http11is explicitly specifiedBenefits
-pr http11is specifiedFiles Added
FIX_README.md- Complete problem analysis and solutionIMPLEMENTATION_GUIDE.md- Step-by-step implementation guidePATCH_NOTES.md- Quick reference for the changefix-http11.patch- Patch file showing the diffapply-fix.sh- Shell script to apply the fix automaticallyRelated Issues
Checklist
Next Steps
common/httpx/httpx.go(documented in this PR)go.moddependencySummary by CodeRabbit