Skip to content

fix: preserve -pr http11 retry behavior (#2240)#2441

Open
ck3960250 wants to merge 1 commit intoprojectdiscovery:devfrom
ck3960250:fix/2240-http11-fallback
Open

fix: preserve -pr http11 retry behavior (#2240)#2441
ck3960250 wants to merge 1 commit intoprojectdiscovery:devfrom
ck3960250:fix/2240-http11-fallback

Conversation

@ck3960250
Copy link

@ck3960250 ck3960250 commented Mar 7, 2026

/claim #2240

Proposed Changes

  • preserve explicit -pr http11 by preventing retry fallback from switching to a separate HTTP/2-capable client
  • add a focused regression test for HTTP/1.1 mode

Proof

  • go test ./common/httpx -run TestHTTP11ProtocolDisablesHTTP2FallbackClient -v (pass)

Checklist

  • narrow scope
  • tests included

Summary by CodeRabbit

  • Bug Fixes
    • Fixed HTTP/1.1 protocol mode to properly disable HTTP/2 fallback, ensuring connections consistently use the explicitly configured HTTP/1.1 protocol.

@neo-by-projectdiscovery-dev
Copy link

neo-by-projectdiscovery-dev bot commented Mar 7, 2026

Neo - PR Security Review

No security issues found

Highlights

  • Fixes HTTP/1.1 protocol preservation during retry fallback by setting HTTPClient2 to the same HTTP/1.1-configured client
  • Adds regression test to verify HTTP/1.1 mode is maintained across retry attempts
  • Ensures explicit -pr http11 flag is respected throughout the request lifecycle

Comment @neo help for available commands. · Open in Neo

@coderabbitai
Copy link

coderabbitai bot commented Mar 7, 2026

Walkthrough

Adds HTTP/1.1 protocol support by conditionally disabling HTTP/2 fallback when the protocol option is set to HTTP11. This is achieved by assigning the non-retryable HTTP client to the HTTP/2 fallback client. Includes a corresponding test.

Changes

Cohort / File(s) Summary
HTTP/1.1 Protocol Fallback Handling
common/httpx/httpx.go, common/httpx/httpx_test.go
Adds conditional logic to disable HTTP/2 fallback when HTTP/1.1 protocol is explicitly selected, ensuring the same HTTP client is used for both retryable and non-retryable clients. Includes test verification that confirms HTTPClient and HTTPClient2 are identical instances when protocol is HTTP11.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 When HTTP/1.1 hops into town,
We disable the fallback with a simple renown,
One client for both, no switching about,
The protocol stays true, there's simply no doubt!
Hops away proudly 🌟

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: preserve -pr http11 retry behavior' directly aligns with the main change: preserving HTTP/1.1 behavior by preventing HTTP/2 fallback in retry scenarios.

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

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

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
common/httpx/httpx.go (1)

186-189: Use the HTTP11 constant instead of string literal at lines 156 and 186.

The HTTP11 constant is defined in common/httpx/proto.go and is correctly used in the test file, but lines 156 and 186 use the string literal "http11" directly. For consistency and to avoid potential bugs if the constant value changes, both locations should use the constant.

♻️ Suggested fixes

Line 156:

-	if httpx.Options.Protocol == "http11" {
+	if httpx.Options.Protocol == HTTP11 {
 		// disable http2

Line 186:

-	if httpx.Options.Protocol == "http11" {
+	if httpx.Options.Protocol == HTTP11 {
 		// keep retry fallback on the same client so explicit HTTP/1.1 is preserved
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@common/httpx/httpx.go` around lines 186 - 189, Replace the hard-coded
protocol string "http11" with the HTTP11 constant from common/httpx/proto.go to
ensure consistency; update the checks that inspect httpx.Options.Protocol (and
the branch that sets httpx.client.HTTPClient2 when protocol is http11) to
compare against HTTP11 instead of the literal, keeping all behavior identical
but using the defined HTTP11 symbol.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@common/httpx/httpx.go`:
- Around line 186-189: Replace the hard-coded protocol string "http11" with the
HTTP11 constant from common/httpx/proto.go to ensure consistency; update the
checks that inspect httpx.Options.Protocol (and the branch that sets
httpx.client.HTTPClient2 when protocol is http11) to compare against HTTP11
instead of the literal, keeping all behavior identical but using the defined
HTTP11 symbol.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9becbf8d-9270-4711-8c33-9a4fbc174a9d

📥 Commits

Reviewing files that changed from the base of the PR and between ed0f6af and f9d0caa.

📒 Files selected for processing (2)
  • common/httpx/httpx.go
  • common/httpx/httpx_test.go

@a638011
Copy link

a638011 commented Mar 9, 2026

Friendly ping! This PR is ready for review. All checks have passed (Neo ✓, CodeRabbit ✓). Happy to address any feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants