Fix headless JS loading with -tlsi and addheader/setheader#7325
Fix headless JS loading with -tlsi and addheader/setheader#7325dogancanbakir wants to merge 1 commit intodevfrom
Conversation
Neo - PR Security ReviewNo security issues found Hardening Notes
Comment |
WalkthroughTwo files modified to enhance headless browsing with TLS impersonation support and improve error handling during HTTP response loading. The primary change introduces conditional HTTP/2 transport setup with FastDialer when TLS impersonation is enabled, while the secondary change adds error capture and logging for response loading failures. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/protocols/headless/engine/rules.go (1)
52-54: Consider returning early on error to avoid processing incomplete response data.After logging the error, execution continues with cookie handling, response rule processing, and history storage. If
LoadResponsefails, the response object may be in an undefined state, potentially causing misleading behavior or additional errors downstream.♻️ Proposed fix to return early on error
// perform the request if err := ctx.LoadResponse(httpClient, true); err != nil { gologger.Verbose().Msgf("headless: failed to load response for %s: %s", ctx.Request.URL(), err) + return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/headless/engine/rules.go` around lines 52 - 54, The code logs an error from ctx.LoadResponse(httpClient, true) but continues processing; change the logic in rules.go to return early when ctx.LoadResponse(...) returns an error—i.e., after the gologger.Verbose() call, exit the current function or loop (depending on surrounding context) to prevent running cookie handling, response rule processing, and history storage on an incomplete/invalid response; reference ctx.LoadResponse and the subsequent cookie/response rule/history handling blocks when making the early-return change.
🤖 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/headless/engine/http_client.go`:
- Around line 84-100: The new http2.Transport assigned to roundTripper when
options.TlsImpersonate is true ignores any proxy settings
(AliveHttpProxy/AliveSocksProxy) set earlier; update the DialTLSContext used in
that http2.Transport to use the same proxy-aware dialing logic as the original
transport (i.e., route through the configured HTTP/SOCKS proxy) instead of
calling dialers.Fastdialer.DialTLSWithConfigImpersonate directly. Concretely,
modify the DialTLSContext implementation in the http2.Transport block to detect
and apply the configured proxy (AliveHttpProxy/AliveSocksProxy) and perform TLS
impersonated dialing through that proxy (leveraging the existing proxy dialer
code path used by the original transport), ensuring TLSClientConfig (tlsConfig)
and impersonate.Random are preserved.
---
Nitpick comments:
In `@pkg/protocols/headless/engine/rules.go`:
- Around line 52-54: The code logs an error from ctx.LoadResponse(httpClient,
true) but continues processing; change the logic in rules.go to return early
when ctx.LoadResponse(...) returns an error—i.e., after the gologger.Verbose()
call, exit the current function or loop (depending on surrounding context) to
prevent running cookie handling, response rule processing, and history storage
on an incomplete/invalid response; reference ctx.LoadResponse and the subsequent
cookie/response rule/history handling blocks when making the early-return
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 47c3ab3c-2d17-4064-8b89-1991debf96d5
📒 Files selected for processing (2)
pkg/protocols/headless/engine/http_client.gopkg/protocols/headless/engine/rules.go
| var roundTripper http.RoundTripper = transport | ||
|
|
||
| // TLS impersonation via utls advertises h2 in ALPN to mimic a real browser. | ||
| // Since utls returns *utls.UConn (not *tls.Conn), Go's http.Transport can't | ||
| // detect the negotiated protocol and tries HTTP/1.x on an h2 connection, | ||
| // causing "malformed HTTP response" errors. We use http2.Transport directly | ||
| // with utls dialing to properly handle h2 connections (see #6360). | ||
| if options.TlsImpersonate { | ||
| roundTripper = &http2.Transport{ | ||
| DialTLSContext: func(ctx context.Context, network, addr string, _ *tls.Config) (net.Conn, error) { | ||
| return dialers.Fastdialer.DialTLSWithConfigImpersonate(ctx, network, addr, tlsConfig, impersonate.Random, nil) | ||
| }, | ||
| TLSClientConfig: tlsConfig, | ||
| AllowHTTP: false, | ||
| DisableCompression: false, | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's existing proxy handling with http2.Transport elsewhere in the codebase
echo "=== Searching for http2.Transport proxy handling patterns ==="
rg -n -C3 'http2\.Transport' --type=go
echo ""
echo "=== Check how fastdialer handles proxy with TLS impersonation ==="
ast-grep --pattern $'DialTLSWithConfigImpersonate($$$)'Repository: projectdiscovery/nuclei
Length of output: 1340
🏁 Script executed:
#!/bin/bash
# Read the full context around lines 64-100 in http_client.go
echo "=== Full context of http_client.go lines 50-105 ==="
sed -n '50,105p' pkg/protocols/headless/engine/http_client.go | cat -n
echo ""
echo "=== Search for proxy configuration setup ==="
rg -n "Proxy|proxy|AliveHttpProxy|AliveSocksProxy" pkg/protocols/headless/engine/http_client.go -B2 -A2
echo ""
echo "=== Locate Fastdialer and DialTLSWithConfigImpersonate ==="
fd -name "*.go" -type f | xargs grep -l "DialTLSWithConfigImpersonate"Repository: projectdiscovery/nuclei
Length of output: 3519
🏁 Script executed:
#!/bin/bash
# Find Fastdialer implementation
echo "=== Locating Fastdialer DialTLSWithConfigImpersonate ==="
fd "fastdialer" -type d
echo ""
echo "=== Search in common vendor/import paths ==="
fd -name "*.go" -type f 2>/dev/null | head -20 | xargs grep -l "DialTLSWithConfigImpersonate" 2>/dev/null | head -5
echo ""
echo "=== Alternative: check imports in http_client.go ==="
head -30 pkg/protocols/headless/engine/http_client.go | grep -E "import|dialers"
echo ""
echo "=== Check if fastdialer is external dependency ==="
if [ -f go.mod ]; then
grep -i fastdialer go.mod | head -5
fiRepository: projectdiscovery/nuclei
Length of output: 684
🏁 Script executed:
#!/bin/bash
# Verify http2.Transport capabilities and proxy handling
echo "=== Check golang.org/x/net/http2 import and usage ==="
grep -n "http2" pkg/protocols/headless/engine/http_client.go | head -10
echo ""
echo "=== Verify that http2.Transport is indeed without Proxy field ==="
# Check if anyone in codebase tries to set Proxy on http2.Transport
rg "http2\.Transport\s*\{" -A 15 pkg/
echo ""
echo "=== Check how this file uses roundTripper ==="
grep -n "roundTripper" pkg/protocols/headless/engine/http_client.goRepository: projectdiscovery/nuclei
Length of output: 1748
Proxy configuration is lost when TLS impersonation is enabled.
When TlsImpersonate is true, the roundTripper is replaced with a new http2.Transport that doesn't incorporate any proxy settings configured on lines 64-82. If a user enables both -tlsi and a proxy (AliveHttpProxy or AliveSocksProxy), the proxy will be silently ignored.
The http2.Transport doesn't have a direct Proxy field. You need to integrate the proxy dialer into the DialTLSContext function for http2.Transport to preserve proxy support when TLS impersonation is enabled.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/protocols/headless/engine/http_client.go` around lines 84 - 100, The new
http2.Transport assigned to roundTripper when options.TlsImpersonate is true
ignores any proxy settings (AliveHttpProxy/AliveSocksProxy) set earlier; update
the DialTLSContext used in that http2.Transport to use the same proxy-aware
dialing logic as the original transport (i.e., route through the configured
HTTP/SOCKS proxy) instead of calling
dialers.Fastdialer.DialTLSWithConfigImpersonate directly. Concretely, modify the
DialTLSContext implementation in the http2.Transport block to detect and apply
the configured proxy (AliveHttpProxy/AliveSocksProxy) and perform TLS
impersonated dialing through that proxy (leveraging the existing proxy dialer
code path used by the original transport), ensuring TLSClientConfig (tlsConfig)
and impersonate.Random are preserved.
Proposed changes
Closes #6360
When
-tlsiis used with headless templates containingaddheader/setheader, all requests get hijacked through Go's HTTP client with utls-based TLS impersonation. utls negotiates h2 via ALPN but returns*utls.UConninstead of*tls.Conn, so Go'shttp.Transportcan't detect the negotiated protocol and fails with "malformed HTTP response" when reading h2 frames.Fix: use
http2.Transportdirectly with utls dialing when TLS impersonation is enabled, properly handling h2 connections. Also logLoadResponseerrors instead of silently discarding them.Proof
before:
after:
Checklist