feat: Add auto-referer (HTTP header) option - #2245
Conversation
The option -auto-referer, sets the referer HTTP header of the current request to it's URL.
WalkthroughAdds an AutoReferer option (CLI and internal) and logic to auto-populate Referer from the current request string when enabled; also makes widespread error-ignoring defer changes, replaces some string/URL accessors with request.String()/req.Path/req.Params, swaps errorutil→errkit, and small string API modernizations. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI
participant Runner as Runner.New
participant HTTPX as HTTPX Layer
participant Request as Request Builder
participant Server as Remote
CLI->>Runner: parse flags (--auto-referer)
Runner->>HTTPX: httpxOptions.AutoReferer = options.AutoReferer
alt CustomHeaders contains "Referer:"
Runner->>HTTPX: httpxOptions.AutoReferer = false
note right of HTTPX #D3E4CD: explicit Referer disables auto
end
Request->>HTTPX: build request (headers)
alt httpxOptions.AutoReferer == true AND header "Referer" empty
HTTPX->>Request: set "Referer" = req.String()
note right of Request #FCE1D6: uses req.String() (full request)
else
note right of Request #E8F0FE: keep explicit Referer or none
end
Request->>Server: send HTTP request
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
common/httpx/option.go (1)
14-16: Add field doc and confirm default behaviorAutoReferer addition looks fine and defaults to false via DefaultOptions. Consider adding a short doc comment (e.g., “AutoReferer sets the Referer header to the current request URL when not explicitly provided”) to keep exported API self-explanatory.
runner/options.go (1)
488-489: Clarify flag behavior in help textMinor: consider clarifying that an explicitly provided Referer header disables/overrides auto behavior, e.g., “set the Referer header to the current URL (ignored if Referer is explicitly provided)”.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
common/httpx/httpx.go(1 hunks)common/httpx/option.go(1 hunks)runner/options.go(2 hunks)runner/runner.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
common/httpx/httpx.go (1)
common/httpx/option.go (1)
Options(13-57)
runner/runner.go (1)
common/customheader/customheader.go (1)
CustomHeaders(10-10)
🔇 Additional comments (2)
runner/options.go (1)
264-265: Struct surface looks good; keep CLI/API parityExposing AutoReferer on runner.Options mirrors common/httpx.Options. No concerns.
runner/runner.go (1)
171-175: Ensure any explicit Referer header disables AutoRefererThe current check in runner/runner.go only looks for CLI‐provided headers via options.CustomHeaders.Has("Referer:"), so headers injected through the raw-request flag (-rr) won’t be detected here. In that case, httpxOptions.AutoReferer remains set to options.AutoReferer and the default auto-referer logic in common/httpx/httpx.go will still overwrite the user’s Referer (via r.Header.Set("Referer", …)).
Instead of a one-off guard in the runner, delegate this entirely to the centralized hook in common/httpx/httpx.go (see the SetCustomHeaders logic) so that any Referer—no matter the source—is honored. This avoids scattered special-cases and ensures the auto-referer behavior is consistently disabled whenever a Referer header is already present.
Additionally, confirm that CustomHeaders.Has(...) does a case-insensitive match on the header name (e.g. “referer:”) so “referer:” isn’t missed.
– runner/runner.go, lines 171–175
– common/httpx/httpx.go: use SetCustomHeaders guard to disable auto-referer centrally
– Verify CustomHeaders.Has is case-insensitive for header names
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
common/customports/customport.go (2)
63-69: Protocol merge bugs: narrows existing HTTP+HTTPS and leaks across range loop.
- If a port is already HTTP+HTTPS, new single-protocol input overwrites it to narrower.
- In ranges, mutating
protocolmakes all ports after the first conflict become HTTP+HTTPS even when not needed.- if existingProtocol, ok := Ports[p]; ok { - if existingProtocol == httpx.HTTP && protocol == httpx.HTTPS || existingProtocol == httpx.HTTPS && protocol == httpx.HTTP { - protocol = httpx.HTTPandHTTPS - } - } - Ports[p] = protocol + if existingProtocol, ok := Ports[p]; ok { + if existingProtocol == httpx.HTTPandHTTPS || protocol == httpx.HTTPandHTTPS || + (existingProtocol == httpx.HTTP && protocol == httpx.HTTPS) || + (existingProtocol == httpx.HTTPS && protocol == httpx.HTTP) { + Ports[p] = httpx.HTTPandHTTPS + } else { + // same protocol: preserve existing to avoid churn + Ports[p] = existingProtocol + } + } else { + Ports[p] = protocol + }- for i := lowP; i <= highP; i++ { - if existingProtocol, ok := Ports[i]; ok { - if existingProtocol == httpx.HTTP && protocol == httpx.HTTPS || existingProtocol == httpx.HTTPS && protocol == httpx.HTTP { - protocol = httpx.HTTPandHTTPS - } - } - Ports[i] = protocol - } + for i := lowP; i <= highP; i++ { + finalProtocol := protocol + if existingProtocol, ok := Ports[i]; ok { + if existingProtocol == httpx.HTTPandHTTPS || finalProtocol == httpx.HTTPandHTTPS || + (existingProtocol == httpx.HTTP && finalProtocol == httpx.HTTPS) || + (existingProtocol == httpx.HTTPS && finalProtocol == httpx.HTTP) { + finalProtocol = httpx.HTTPandHTTPS + } else { + // same protocol: keep existing + finalProtocol = existingProtocol + } + } + Ports[i] = finalProtocol + }Also applies to: 90-97
105-110: Reject invalid ports <= 0.Acceptable ports are 1..65535; current check lets 0/-1 through.
func checkPortValue(port int) error { - if port > 65535 { - return errors.New("port value is bigger than 65535") + if port < 1 || port > 65535 { + return errors.New("port value must be between 1 and 65535") } return nil }internal/pdcp/writer.go (1)
173-181: Bug: record dropped when buffer boundary is crossed.When buff.Len()+len(line) > MaxChunkSize, the current line is not written after flushing.
Apply this diff:
- if buff.Len()+len(line) > MaxChunkSize { - // flush existing buffer - if err := u.uploadChunk(buff); err != nil { - gologger.Error().Msgf("Failed to upload asset results on cloud: %v", err) - } - } else { - buff.WriteString(line) - buff.WriteString("\n") - } + if buff.Len()+len(line) > MaxChunkSize { + // flush existing buffer + if err := u.uploadChunk(buff); err != nil { + gologger.Error().Msgf("Failed to upload asset results on cloud: %v", err) + } + } + // write current line (either to emptied buffer or existing one) + buff.WriteString(line) + buff.WriteString("\n")
🧹 Nitpick comments (12)
common/customports/customport.go (3)
86-88: Clarify the constraint to reflect the actual check (<= allowed).The code allows equal endpoints; update message accordingly.
- return fmt.Errorf("first value of port range should be lower than the last port from your range: [%d, %d]", lowP, highP) + return fmt.Errorf("first port must be <= last port: [%d, %d]", lowP, highP)
53-56: Guard against malformed inputs with more than one dash.Limit split to two parts.
- potentialRange := strings.Split(potentialPort, "-") + potentialRange := strings.SplitN(potentialPort, "-", portRangeParts)
56-59: Standardize error casing and remove stray newline.Align with the lowercase style adopted elsewhere.
- if err != nil { - return errors.Wrap(err, fmt.Sprintf("Could not cast port to integer from your value: %s\n", potentialPort)) - } + if err != nil { + return errors.Wrap(err, fmt.Sprintf("could not cast port to integer from your value: %s", potentialPort)) + }- if err != nil { - return errors.Wrap(err, fmt.Sprintf("Could not cast first port of your range(%s) to integer from your value: %s", potentialPort, potentialRange[0])) - } + if err != nil { + return errors.Wrap(err, fmt.Sprintf("could not cast first port of your range(%s) to integer from your value: %s", potentialPort, potentialRange[0])) + }Also applies to: 71-76
cmd/functional-test/main.go (1)
44-46: Explicitly ignoring Close error is fine here; consider centralizing the pattern.LGTM for tests. If this pattern repeats across the repo, a tiny helper (e.g., closeSilently(io.Closer)) can reduce repetition.
runner/headless.go (1)
147-149: Silent cleanup is acceptable; optional: debug-log failures.If troubleshooting flaky headless runs, logging Close/RemoveAll errors at debug level could help.
internal/pdcp/writer.go (2)
233-241: Mutable shared URL can be a future data race; copy before mutation.getRequest mutates u.uploadURL.Path. It’s fine with the current single goroutine, but future concurrent use would race. Prefer copying to a local url.URL before setting Path and building the request URL.
194-195: Log level: consider Info instead of Warning."Uploaded results chunk" is not a warning condition; Info/Verbose would be more appropriate.
runner/runner.go (5)
42-42: Avoid blanket nolint on import; scope it or drop it.
//nolintat import level hides all linter classes. If needed, target the specific linter(s) and add a short rationale; otherwise remove it.
127-130: Don’t silently ignore RemoveAll errors.Failures here (permission, ENOENT races, RO FS) are useful for diagnostics. Log at debug and continue.
- _ = os.RemoveAll(filepath.Join(options.StoreResponseDir, "response", "index.txt")) - _ = os.RemoveAll(filepath.Join(options.StoreResponseDir, "screenshot", "index_screenshot.txt")) + if err := os.RemoveAll(filepath.Join(options.StoreResponseDir, "response", "index.txt")); err != nil { + gologger.Debug().Msgf("cleanup response index failed: %v", err) + } + if err := os.RemoveAll(filepath.Join(options.StoreResponseDir, "screenshot", "index_screenshot.txt")); err != nil { + gologger.Debug().Msgf("cleanup screenshot index failed: %v", err) + }
1114-1115: Body stripping by substring risks accidental header mutation.
strings.ReplaceAll(resp.Raw, resp.ResponseBody, "")may remove identical byte sequences outside the body. Prefer slicing raw at the known header/body boundary (e.g., split on\r\n\r\nonce) or use the library’s structured fields.
2087-2087: Remove stray //nolint on local variable.
outputHashesis used below; the suppression isn’t needed and hides real issues.- outputHashes := !(r.options.JSONOutput || r.options.OutputAll) //nolint + outputHashes := !(r.options.JSONOutput || r.options.OutputAll)
2397-2406: Base URL derivation: prefer urlutil.ParseURL for consistency with unsafe parsing.Minor, but using the project’s parser avoids divergence with
Unsafemode and IDNA handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
cmd/functional-test/main.go(1 hunks)cmd/integration-test/http.go(12 hunks)cmd/integration-test/library.go(2 hunks)common/customports/customport.go(1 hunks)common/httpx/httpx.go(4 hunks)examples/speed_control/main.go(1 hunks)internal/pdcp/writer.go(5 hunks)runner/headless.go(2 hunks)runner/healthcheck.go(2 hunks)runner/runner.go(25 hunks)
✅ Files skipped from review due to trivial changes (3)
- examples/speed_control/main.go
- runner/healthcheck.go
- cmd/integration-test/library.go
🔇 Additional comments (14)
common/customports/customport.go (1)
80-81: Lowercased error string — LGTM.Matches Go style for error messages.
runner/headless.go (1)
118-120: OK to discard page.Close error on deferred cleanup.Matches the repo-wide convention introduced in this PR.
cmd/integration-test/http.go (1)
52-52: OK to discard write errors in test handlers.These are test-only responses; explicit ignores keep handlers concise without affecting assertions.
Also applies to: 104-104, 167-167, 195-195, 218-218, 242-242, 269-269, 294-294, 318-318, 345-345, 369-369, 394-394
internal/pdcp/writer.go (3)
21-21: errkit migration acknowledged.Import switch to errkit is consistent with the PR-wide error-handling refactor.
74-75: Good: wrap parse errors with context.The error now carries actionable context.
189-190: Consistent error wrapping and cleanup behavior.Wrapping improves diagnosability; ignoring resp.Body.Close error is appropriate in this path.
Also applies to: 201-202, 205-206, 212-213, 219-220
common/httpx/httpx.go (3)
158-160: Ignoring Setenv error is acceptable.Matches the intended behavior of forcing http/1.1 without surfacing env errors.
363-366: Using req.String() for the unsafe target is correct.Ensures path/params mutations are reflected in raw execution.
454-455: ReplaceAll is fine.Minor readability improvement; equivalent behavior.
runner/runner.go (5)
1665-1669: Verify DumpRequestRaw URL argument change.Switching to
req.String()(vsreq.URL.String()) may pass an absolute URL or include host overrides differently than expected by rawhttp. Confirm it still emits a valid start-line and host semantics in unsafe mode.
1688-1699: LGTM: Using req.String() to compute fullURL.This correctly reflects custom Host/port tweaks before presentation.
1874-1875: LGTM: fmt.Fprintf to builder reduces allocs.Good micro-optimization across status/headers/metrics blocks.
Also applies to: 1988-1989, 2014-2015, 2019-2019, 2023-2023, 2140-2141, 2151-2151, 2160-2160
2486-2486: LGTM: clone.String() for FaviconURL.Accurately preserves any per-request header/URL tweaks.
171-175: I’ve searched for theCustomHeaderstype and itsHasmethod to confirm how header keys (including casing and colon) are handled. Let me know if you’d like me to inspect the raw‐request merge logic next.
| if err := checkPortValue(highP); err != nil { | ||
| return errors.Wrap(err, fmt.Sprintf("last port of your range(%d)", lowP)) | ||
| } |
There was a problem hiding this comment.
Wrong variable in “last port” error context.
Message prints lowP instead of highP.
- return errors.Wrap(err, fmt.Sprintf("last port of your range(%d)", lowP))
+ return errors.Wrap(err, fmt.Sprintf("last port of your range(%d)", highP))📝 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.
| if err := checkPortValue(highP); err != nil { | |
| return errors.Wrap(err, fmt.Sprintf("last port of your range(%d)", lowP)) | |
| } | |
| if err := checkPortValue(highP); err != nil { | |
| return errors.Wrap(err, fmt.Sprintf("last port of your range(%d)", highP)) | |
| } |
🤖 Prompt for AI Agents
In common/customports/customport.go around lines 82 to 84, the error wrap for
validating the upper bound prints the wrong variable (it uses lowP instead of
highP); update the fmt.Sprintf call to include highP so the error context
correctly identifies the last port value (e.g., change the wrapped message to
reference highP).
| if h.Options.AutoReferer && r.Header.Get("Referer") == "" { | ||
| r.Header.Set("Referer", r.String()) | ||
| } |
There was a problem hiding this comment.
Sanitize Referer to avoid credential leakage.
If the URL contains userinfo (user:pass@host), setting it as Referer leaks credentials. Strip userinfo before setting.
Apply this diff:
- if h.Options.AutoReferer && r.Header.Get("Referer") == "" {
- r.Header.Set("Referer", r.String())
- }
+ if h.Options.AutoReferer && r.Header.Get("Referer") == "" {
+ if u, perr := url.Parse(r.String()); perr == nil {
+ u.User = nil // drop potential credentials
+ r.Header.Set("Referer", u.String())
+ } else {
+ r.Header.Set("Referer", r.String())
+ }
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In common/httpx/httpx.go around lines 438 to 440, the current code sets Referer
to r.String() which can leak credentials if the URL contains userinfo; change
the logic to construct a sanitized referer by cloning r.URL (or using a safe
representation), removing any userinfo (set User to nil), and then using the
sanitized URL string when calling r.Header.Set("Referer", ...). Keep the
existing AutoReferer and header-empty checks, and ensure the sanitized URL is
used so no user:pass@host is ever sent in the Referer header.
| if resp.StatusCode != http.StatusOK { | ||
| return fmt.Errorf("could not upload results got status code %v on %v", resp.StatusCode, resp.Request.URL.String()) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Accept 201/202 for create/append responses.
Some endpoints return 201 Created (POST) or 202 Accepted. Current strict 200 check may misclassify success as error.
Apply this diff:
- if resp.StatusCode != http.StatusOK {
+ if resp.StatusCode != http.StatusOK &&
+ resp.StatusCode != http.StatusCreated && // 201
+ resp.StatusCode != http.StatusAccepted { // 202 (async)
return fmt.Errorf("could not upload results got status code %v on %v", resp.StatusCode, resp.Request.URL.String())
}📝 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.
| if resp.StatusCode != http.StatusOK { | |
| return fmt.Errorf("could not upload results got status code %v on %v", resp.StatusCode, resp.Request.URL.String()) | |
| } | |
| if resp.StatusCode != http.StatusOK && | |
| resp.StatusCode != http.StatusCreated && // 201 | |
| resp.StatusCode != http.StatusAccepted { // 202 (async) | |
| return fmt.Errorf( | |
| "could not upload results got status code %v on %v", | |
| resp.StatusCode, | |
| resp.Request.URL.String(), | |
| ) | |
| } |
🤖 Prompt for AI Agents
In internal/pdcp/writer.go around lines 214 to 216, the status-code check
currently only accepts 200; update it to treat 200, 201 and 202 as success (or
better, accept any 2xx by checking resp.StatusCode >= 200 && resp.StatusCode <
300) and only return an error for non-2xx codes, preserving the existing error
message but ensuring it uses resp.StatusCode and resp.Request.URL.String() as
before.
| _ = plainFile.Close() | ||
| }() | ||
| jsonFile = openOrCreateFile(r.options.Resume, r.options.Output+".json") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Capture and log Close() errors instead of discarding them.
Close can surface buffered-write failures. Prefer a helper that logs and continues.
- defer func() {
- _ = plainFile.Close()
- }()
+ defer closeFile(plainFile, r.options.Output)Add once in this file:
func closeFile(f *os.File, name string) {
if f == nil {
return
}
if err := f.Close(); err != nil {
gologger.Error().Msgf("Could not close file '%s': %s\n", name, err)
}
}Apply the helper to all similar defers in this function.
Also applies to: 815-817, 819-821, 828-830, 839-841, 850-852, 1243-1245, 1337-1339, 1362-1364
🤖 Prompt for AI Agents
In runner/runner.go around lines 811-813 (and also update the similar defers at
815-817, 819-821, 828-830, 839-841, 850-852, 1243-1245, 1337-1339, 1362-1364),
the current defers call f.Close() and discard any error; add a helper function
closeFile(f *os.File, name string) that returns early on nil and logs Close()
errors via gologger.Error().Msgf("Could not close file '%s': %s\n", name, err),
then replace each deferred anonymous Close() with defer closeFile(<fileVar>,
"<filename or variable>") so buffered-write failures are captured and logged
while allowing the function to continue.
| if path.Ext(req.Path) == ".ico" { | ||
| mmh3, md5h, err := r.calculateFaviconHashWithRaw(currentResp) | ||
| return mmh3, md5h, req.URL.Path, currentResp, req.URL.String(), err | ||
| return mmh3, md5h, req.Path, currentResp, req.String(), err | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
req.Path may be empty; add fallback to URL.Path.
Some request types won’t populate Path. Guard and return the actual path used.
- if path.Ext(req.Path) == ".ico" {
- mmh3, md5h, err := r.calculateFaviconHashWithRaw(currentResp)
- return mmh3, md5h, req.Path, currentResp, req.String(), err
- }
+ p := req.Path
+ if p == "" && req.URL != nil {
+ p = req.URL.Path
+ }
+ if path.Ext(p) == ".ico" {
+ mmh3, md5h, err := r.calculateFaviconHashWithRaw(currentResp)
+ return mmh3, md5h, p, currentResp, req.String(), err
+ }📝 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.
| if path.Ext(req.Path) == ".ico" { | |
| mmh3, md5h, err := r.calculateFaviconHashWithRaw(currentResp) | |
| return mmh3, md5h, req.URL.Path, currentResp, req.URL.String(), err | |
| return mmh3, md5h, req.Path, currentResp, req.String(), err | |
| } | |
| // Determine the path, falling back to URL.Path if req.Path is empty | |
| p := req.Path | |
| if p == "" && req.URL != nil { | |
| p = req.URL.Path | |
| } | |
| if path.Ext(p) == ".ico" { | |
| mmh3, md5h, err := r.calculateFaviconHashWithRaw(currentResp) | |
| return mmh3, md5h, p, currentResp, req.String(), err | |
| } |
🤖 Prompt for AI Agents
In runner/runner.go around lines 2380 to 2383, the code checks if
path.Ext(req.Path) == ".ico" but req.Path can be empty for some request types;
change the logic to use a safe path variable: set p := req.Path; if p == "" &&
req.URL != nil { p = req.URL.Path } then use path.Ext(p) for the ico check and
return p (not req.Path) as the path value; this ensures a fallback to
req.URL.Path and avoids nil-pointer or empty-path returns.
2 participants
httpxproduces better results when theRefererheader is set. However, when using stdin, there is currently no built-in way to set theRefererheader based on the current URL.Normally, we achieve this by using a shell while loop:
cat links | while read url; do httpx -H "Referer: $url"; doneThis approach is inefficient, as it requires running
httpxmany times, and also using-threadsis not possible.This feature introduces the
-auto-refereroption, which eliminates the need for a while loop;Users can now simply execute:
cat links | httpx -auto-referer.Summary by CodeRabbit
New Features
Documentation
Refactor
Chores