[Mzack9999]

Proposed changes

Close #5887

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

• New Features

  • Added protocol-redirects option for protocol-level redirect control in HTTP requests, requiring pairing with existing redirect options.
  • Introduced same-scheme redirect policy allowing redirects only when the protocol (HTTP/HTTPS) remains unchanged.

• Tests

  • Comprehensive new tests validating request clustering eligibility, template clustering key behavior, and redirect policy enforcement across multiple scenarios.

[neo-by-projectdiscovery-dev]

Neo - PR Security Review

No security issues found

Highlights

• Adds protocol-redirects feature to follow cross-protocol redirects (e.g., HTTP to HTTPS)
• Includes validation requiring redirects or host-redirects to be enabled first
• Extends existing redirect functionality in HTTP protocol implementation

Hardening Notes

• Sandbox environment experienced multiple timeouts preventing full diff analysis and automated scanner execution
• Manual verification of protocol validation logic and redirect URL handling in httpclientpool could not be completed
• Recommend follow-up review once sandbox infrastructure is stable to verify: (1) Protocol whitelist implementation, (2) SSRF protections in cross-protocol redirects, (3) URL parsing security for protocol transitions

_{Comment @pdneo help for available commands. · Open in Neo}

[coderabbitai]

Walkthrough

This change implements a new ProtocolRedirects feature that restricts HTTP redirects to remain within the same protocol scheme. A new ProtocolRedirects boolean field is added to the Request struct, integrated into redirect-flow logic, validation rules, and clustering behavior to control whether redirects can cross between HTTP and HTTPS.

Changes

Cohort / File(s)	Summary
Core Feature Implementation pkg/protocols/http/http.go, pkg/protocols/http/httpclientpool/clientpool.go	Added ProtocolRedirects bool field to Request struct with YAML/JSON marshaling. Implemented FollowSameSchemeRedirect redirect mode that validates the target URL scheme matches the last request scheme in the redirect chain before proceeding.
Clustering & Caching pkg/protocols/http/cluster.go	Updated TmplClusterKey() hash computation to include request.ProtocolRedirects value, ensuring distinct cluster keys when this field differs.
Validation pkg/protocols/http/validate.go	Added validation rule requiring either Redirects or HostRedirects to be enabled when ProtocolRedirects is true.
Test Coverage pkg/protocols/http/cluster_test.go, pkg/protocols/http/httpclientpool/clientpool_test.go, pkg/protocols/http/validate_test.go	Added comprehensive test suites covering clustering eligibility, clustering key determinism and equality, same-scheme redirect policy enforcement across single and chained redirects, redirect count limits, and redirect-option validation combinations.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Request Handler
    participant Redirect Check
    participant FollowSameSchemeRedirect
    participant Target Server

    User->>Request Handler: Execute request with ProtocolRedirects=true
    Request Handler->>Request Handler: Set RedirectFlow to FollowSameSchemeRedirect
    Request Handler->>Target Server: Send initial HTTP request
    Target Server-->>Request Handler: 3xx redirect response
    Request Handler->>Redirect Check: Check if redirect allowed
    Redirect Check->>FollowSameSchemeRedirect: Validate scheme consistency
    FollowSameSchemeRedirect->>FollowSameSchemeRedirect: Compare URL.Scheme (current vs last)
    
    alt Scheme matches (same protocol)
        FollowSameSchemeRedirect->>Redirect Check: Check max redirects
        Redirect Check-->>Request Handler: Redirect allowed
        Request Handler->>Target Server: Follow redirect
    else Scheme differs (protocol changed)
        FollowSameSchemeRedirect-->>Redirect Check: http.ErrUseLastResponse
        Redirect Check-->>Request Handler: Redirect blocked
        Request Handler-->>User: Return last response
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A protocol-guard in the redirect flow,
Keeps schemes aligned where'er we go,
No https-to-http surprise,
Just same-scheme hops under cautious eyes. 🔒

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'adding protocols redirect' is vague and generic, using non-descriptive phrasing that does not clearly convey the specific purpose of this change.	Revise the title to be more specific, such as 'Add protocol-redirects option to restrict redirects within the same scheme' or similar.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	The PR successfully implements all coding requirements from issue #5887: adds ProtocolRedirects field with protocol-level redirect control, includes validation requiring ProtocolRedirects to be paired with existing redirect options, and provides comprehensive test coverage.
Out of Scope Changes check	✅ Passed	All changes are directly related to implementing the protocol-redirects feature from issue #5887; no unrelated or out-of-scope modifications were introduced.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 5887-protocol-redirects

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/protocols/http/http.go`:
- Around line 345-347: The current logic sets
connectionConfiguration.RedirectFlow to FollowSameSchemeRedirect when
request.ProtocolRedirects is true, which unconditionally overrides a prior
FollowSameHostRedirect set when request.HostRedirects is true; update the
handling in the RedirectFlow selection so both restrictions are preserved:
either add and use a new enum/value like FollowSameHostAndSchemeRedirect and set
RedirectFlow to that when both request.HostRedirects and
request.ProtocolRedirects are true, or change the branching to an else-if chain
that prevents ProtocolRedirects from overwriting an already-set
FollowSameHostRedirect. Ensure you update any switch/assignment logic that
references RedirectFlow, and use the symbols ProtocolRedirects, HostRedirects,
RedirectFlow, FollowSameSchemeRedirect and FollowSameHostRedirect to locate and
modify the code.

In `@pkg/protocols/http/httpclientpool/clientpool.go`:
- Around line 402-407: The FollowSameSchemeRedirect branch currently compares
req.URL.Scheme against via[len(via)-1].URL.Scheme which is inconsistent with
FollowSameHostRedirect (which uses via[0]) and can allow scheme changes
mid-chain; change the comparison to use the original request's scheme
via[0].URL.Scheme and then call checkMaxRedirects as before (identify the code
in the FollowSameSchemeRedirect case that references via and checkMaxRedirects
and replace len(via)-1 with 0).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4b999dee-d8ec-4bfe-a7fb-fc45f3a989a0

📥 Commits

Reviewing files that changed from the base of the PR and between c8a9853 and d7ceec9.

📒 Files selected for processing (7)

• pkg/protocols/http/cluster.go
• pkg/protocols/http/cluster_test.go
• pkg/protocols/http/http.go
• pkg/protocols/http/httpclientpool/clientpool.go
• pkg/protocols/http/httpclientpool/clientpool_test.go
• pkg/protocols/http/validate.go
• pkg/protocols/http/validate_test.go

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there's any existing combined redirect mode or if this is the intended design
rg -n "FollowSame" pkg/protocols/http/httpclientpool/

Repository: projectdiscovery/nuclei

Length of output: 1647

---

🏁 Script executed:

# Check the redirect logic implementations in clientpool.go
head -n 430 pkg/protocols/http/httpclientpool/clientpool.go | tail -n 70

Repository: projectdiscovery/nuclei

Length of output: 2018

---

🏁 Script executed:

# Check the validation logic in http.go to understand allowed combinations
rg -n "ProtocolRedirects" pkg/protocols/http/ -A 3 -B 1

Repository: projectdiscovery/nuclei

Length of output: 4262

---

🏁 Script executed:

# Check lines around 345 in http.go to see the full Compile logic for redirects
sed -n '310,360p' pkg/protocols/http/http.go

Repository: projectdiscovery/nuclei

Length of output: 1776

---

🏁 Script executed:

rg -n "HostRedirects.*ProtocolRedirects|ProtocolRedirects.*HostRedirects" pkg/protocols/http/ -B 2 -A 5

Repository: projectdiscovery/nuclei

Length of output: 1113

---

🏁 Script executed:

# Also check if there are any integration tests or behavioral tests for redirect combinations
find pkg/protocols/http -name "*test.go" -exec grep -l "HostRedirects\|ProtocolRedirects" {} \;

Repository: projectdiscovery/nuclei

Length of output: 137

---

🏁 Script executed:

# Look for any tests or documentation about expected redirect behavior with both flags
rg -n "HostRedirects" pkg/protocols/http/http.go -B 5 -A 5

Repository: projectdiscovery/nuclei

Length of output: 1696

---

🏁 Script executed:

# Check the description of HostRedirects to understand intent
sed -n '175,210p' pkg/protocols/http/http.go

Repository: projectdiscovery/nuclei

Length of output: 3093

---

ProtocolRedirects unconditionally overrides HostRedirects, losing same-host restriction.

When both host-redirects: true and protocol-redirects: true are set, the current implementation at lines 345–346 unconditionally sets RedirectFlow = FollowSameSchemeRedirect, which only checks the scheme. This overwrites the previous FollowSameHostRedirect assignment at line 343, causing the host restriction to be lost.

The code validates this combination as valid (validate.go line 14), but there is no combined redirect mode available to enforce both same-host and same-scheme restrictions simultaneously. Either implement a new combined redirect mode (e.g., FollowSameHostAndSchemeRedirect) or use an else-if chain to prevent unintended overrides.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/protocols/http/http.go` around lines 345 - 347, The current logic sets
connectionConfiguration.RedirectFlow to FollowSameSchemeRedirect when
request.ProtocolRedirects is true, which unconditionally overrides a prior
FollowSameHostRedirect set when request.HostRedirects is true; update the
handling in the RedirectFlow selection so both restrictions are preserved:
either add and use a new enum/value like FollowSameHostAndSchemeRedirect and set
RedirectFlow to that when both request.HostRedirects and
request.ProtocolRedirects are true, or change the branching to an else-if chain
that prevents ProtocolRedirects from overwriting an already-set
FollowSameHostRedirect. Ensure you update any switch/assignment logic that
references RedirectFlow, and use the symbols ProtocolRedirects, HostRedirects,
RedirectFlow, FollowSameSchemeRedirect and FollowSameHostRedirect to locate and
modify the code.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Inconsistent base for scheme comparison - consider using via[0] instead of via[len(via)-1].

The FollowSameSchemeRedirect case compares the scheme against the last request in the chain (via[len(via)-1]), whereas FollowSameHostRedirect (lines 391-394) compares the host against the original request (via[0]).

This inconsistency could lead to unexpected behavior. If the goal is to restrict redirects to the original protocol used to initiate the request (per issue #5887), the comparison should use via[0].URL.Scheme:

🐛 Proposed fix to use original request scheme

 case FollowSameSchemeRedirect:
-	previousScheme := via[len(via)-1].URL.Scheme
+	originalScheme := via[0].URL.Scheme
-	if req.URL.Scheme != previousScheme {
+	if req.URL.Scheme != originalScheme {
 		return http.ErrUseLastResponse
 	}
 	return checkMaxRedirects(req, via, maxRedirects)

📝 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.

Suggested change

	case FollowSameSchemeRedirect:
	previousScheme := via[len(via)-1].URL.Scheme
	if req.URL.Scheme != previousScheme {
	return http.ErrUseLastResponse
	}
	return checkMaxRedirects(req, via, maxRedirects)
	case FollowSameSchemeRedirect:
	originalScheme := via[0].URL.Scheme
	if req.URL.Scheme != originalScheme {
	return http.ErrUseLastResponse
	}
	return checkMaxRedirects(req, via, maxRedirects)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/protocols/http/httpclientpool/clientpool.go` around lines 402 - 407, The
FollowSameSchemeRedirect branch currently compares req.URL.Scheme against
via[len(via)-1].URL.Scheme which is inconsistent with FollowSameHostRedirect
(which uses via[0]) and can allow scheme changes mid-chain; change the
comparison to use the original request's scheme via[0].URL.Scheme and then call
checkMaxRedirects as before (identify the code in the FollowSameSchemeRedirect
case that references via and checkMaxRedirects and replace len(via)-1 with 0).