Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion internal/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func New(options *types.Options) (*Runner, error) {
var httpclient *retryablehttp.Client
if options.ProxyInternal && options.AliveHttpProxy != "" || options.AliveSocksProxy != "" {
var err error
httpclient, err = httpclientpool.Get(options, &httpclientpool.Configuration{})
httpclient, err = httpclientpool.Get(options, &httpclientpool.Configuration{}, "")
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -427,6 +427,11 @@ func (r *Runner) Close() {
if r.httpStats != nil {
r.httpStats.DisplayTopStats(r.options.NoColor)
}
if newConns, reusedConns := httpclientpool.GetConnectionStats(); newConns+reusedConns > 0 {
total := newConns + reusedConns
ratio := float64(reusedConns) / float64(total) * 100
gologger.Info().Msgf("HTTP connections: %d total, %d new, %d reused (%.1f%%)", total, newConns, reusedConns, ratio)
}
Comment on lines +430 to +434
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Connection stats will be cumulative across multiple in-process scans.

GetConnectionStats() reads package-global counters, so a second SDK/embedded run in the same process will log totals from earlier executions too. If this is meant to diagnose one scan, reset or scope these stats by ExecutionId when a run starts or ends.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/runner/runner.go` around lines 430 - 434, GetConnectionStats()
returns package-global cumulative counters, so per-scan logs in runner.go show
aggregated totals across in-process runs; update the httpclientpool usage to
produce per-execution stats by either calling a reset or using a scoped API:
add/ call httpclientpool.ResetConnectionStats() at the start (or end) of a run
inside the runner (around where GetConnectionStats() is invoked) or change to an
API like httpclientpool.GetConnectionStatsForExecution(executionId) /
httpclientpool.ScopedStats(executionId) and pass the run's ExecutionId so the
logged totals (from GetConnectionStats / new scoped method) reflect only the
current scan. Ensure you reference and modify the call site in runner.go where
GetConnectionStats() is used and wire in ExecutionId from the current Runner
context.

// dump hosterrors cache
if r.hostErrors != nil {
r.hostErrors.Close()
Expand Down
2 changes: 1 addition & 1 deletion lib/sdk_private.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (e *NucleiEngine) init(ctx context.Context) error {
}

if e.opts.ProxyInternal && e.opts.AliveHttpProxy != "" || e.opts.AliveSocksProxy != "" {
httpclient, err := httpclientpool.Get(e.opts, &httpclientpool.Configuration{})
httpclient, err := httpclientpool.Get(e.opts, &httpclientpool.Configuration{}, "")
if err != nil {
return err
}
Expand Down
7 changes: 1 addition & 6 deletions pkg/protocols/common/automaticscan/automaticscan.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/contextargs"
"github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/helpers/writer"
"github.com/projectdiscovery/nuclei/v3/pkg/protocols/http/httpclientpool"
httputil "github.com/projectdiscovery/nuclei/v3/pkg/protocols/utils/http"
"github.com/projectdiscovery/nuclei/v3/pkg/scan"
"github.com/projectdiscovery/nuclei/v3/pkg/templates"
"github.com/projectdiscovery/nuclei/v3/pkg/testutils"
Expand Down Expand Up @@ -95,11 +94,7 @@ func New(opts Options) (*Service, error) {
return nil, err
}

httpclient, err := httpclientpool.Get(opts.ExecuterOpts.Options, &httpclientpool.Configuration{
Connection: &httpclientpool.ConnectionConfiguration{
DisableKeepAlive: httputil.ShouldDisableKeepAlive(opts.ExecuterOpts.Options),
},
})
httpclient, err := httpclientpool.Get(opts.ExecuterOpts.Options, &httpclientpool.Configuration{}, "")
if err != nil {
return nil, errors.Wrap(err, "could not get http client")
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/protocols/common/protocolstate/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,10 @@ func initDialers(options *types.Options) error {
networkPolicy, _ := networkpolicy.New(*npOptions)

httpClientPool := mapsutil.NewSyncLockMap(
// evicts inactive httpclientpool entries after 24 hours
// of inactivity (long running instances)
mapsutil.WithEviction[string, *retryablehttp.Client](24*time.Hour, 12*time.Hour),
// Per-host HTTP clients are evicted after 90 seconds of inactivity.
// Combined with IdleConnTimeout on each transport, this ensures
// connections to already-scanned hosts are cleaned up promptly.
mapsutil.WithEviction[string, *retryablehttp.Client](90*time.Second, 30*time.Second),
)

dialersInstance := &Dialers{
Expand Down
7 changes: 4 additions & 3 deletions pkg/protocols/http/build_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
protocolutils "github.com/projectdiscovery/nuclei/v3/pkg/protocols/utils"
httputil "github.com/projectdiscovery/nuclei/v3/pkg/protocols/utils/http"
"github.com/projectdiscovery/nuclei/v3/pkg/types"
"github.com/projectdiscovery/nuclei/v3/pkg/types/scanstrategy"
"github.com/projectdiscovery/rawhttp"
"github.com/projectdiscovery/retryablehttp-go"
"github.com/projectdiscovery/utils/errkit"
Expand Down Expand Up @@ -485,8 +484,10 @@ func (r *requestGenerator) fillRequest(req *retryablehttp.Request, values map[st
}
}

// In case of multiple threads the underlying connection should remain open to allow reuse
if r.request.Threads <= 0 && req.Header.Get("Connection") == "" && r.options.Options.ScanStrategy != scanstrategy.HostSpray.String() {
// Per-host clients always have keep-alive enabled for connection reuse.
// Only force-close connections when a template explicitly disables keep-alive.
if r.request.connConfiguration != nil && r.request.connConfiguration.Connection != nil &&
r.request.connConfiguration.Connection.DisableKeepAlive && req.Header.Get("Connection") == "" {
req.Close = true
}

Expand Down
21 changes: 5 additions & 16 deletions pkg/protocols/http/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,8 @@ import (
"github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/protocolstate"
"github.com/projectdiscovery/nuclei/v3/pkg/protocols/http/httpclientpool"
"github.com/projectdiscovery/nuclei/v3/pkg/protocols/network/networkclientpool"
httputil "github.com/projectdiscovery/nuclei/v3/pkg/protocols/utils/http"
"github.com/projectdiscovery/nuclei/v3/pkg/utils/stats"
"github.com/projectdiscovery/rawhttp"
"github.com/projectdiscovery/retryablehttp-go"
fileutil "github.com/projectdiscovery/utils/file"
)

Expand Down Expand Up @@ -143,10 +141,9 @@ type Request struct {
options *protocols.ExecutorOptions
connConfiguration *httpclientpool.Configuration
totalRequests int
customHeaders map[string]string
generator *generators.PayloadGenerator // optional, only enabled when using payloads
httpClient *retryablehttp.Client
rawhttpClient *rawhttp.Client
customHeaders map[string]string
generator *generators.PayloadGenerator // optional, only enabled when using payloads
rawhttpClient *rawhttp.Client
dialer *fastdialer.Dialer

// description: |
Expand Down Expand Up @@ -310,10 +307,8 @@ func (request *Request) Compile(options *protocols.ExecutorOptions) error {
MaxRedirects: request.MaxRedirects,
NoTimeout: false,
DisableCookie: request.DisableCookie,
Connection: &httpclientpool.ConnectionConfiguration{
DisableKeepAlive: httputil.ShouldDisableKeepAlive(options.Options),
},
RedirectFlow: httpclientpool.DontFollowRedirect,
Connection: &httpclientpool.ConnectionConfiguration{},
RedirectFlow: httpclientpool.DontFollowRedirect,
}
var customTimeout int
if request.Analyzer != nil && request.Analyzer.Name == "time_delay" {
Expand Down Expand Up @@ -345,13 +340,7 @@ func (request *Request) Compile(options *protocols.ExecutorOptions) error {
}
}
request.connConfiguration = connectionConfiguration

client, err := httpclientpool.Get(options.Options, connectionConfiguration)
if err != nil {
return errors.Wrap(err, "could not get dns client")
}
request.customHeaders = make(map[string]string)
request.httpClient = client

dialer, err := networkclientpool.Get(options.Options, &networkclientpool.Configuration{
CustomDialer: options.CustomFastdialer,
Expand Down
109 changes: 70 additions & 39 deletions pkg/protocols/http/httpclientpool/clientpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import (
"net"
"net/http"
"net/http/cookiejar"
"net/http/httptrace"
"net/url"
"strconv"
"strings"
"sync"
"sync/atomic"
"time"

"github.com/pkg/errors"
Expand All @@ -22,16 +24,48 @@ import (
"github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/protocolstate"
"github.com/projectdiscovery/nuclei/v3/pkg/protocols/utils"
"github.com/projectdiscovery/nuclei/v3/pkg/types"
"github.com/projectdiscovery/nuclei/v3/pkg/types/scanstrategy"
"github.com/projectdiscovery/rawhttp"
"github.com/projectdiscovery/retryablehttp-go"
urlutil "github.com/projectdiscovery/utils/url"
)

var (
forceMaxRedirects int
connStats ConnectionStats
)

// ConnectionStats tracks HTTP connection reuse across the scan.
type ConnectionStats struct {
New atomic.Int64
Reused atomic.Int64
}

// GetConnectionStats returns the current connection statistics.
func GetConnectionStats() (newConns, reused int64) {
return connStats.New.Load(), connStats.Reused.Load()
}

// connTrackingTransport wraps an http.RoundTripper to track connection reuse
// via httptrace. Every request gets a GotConn callback that increments the
// appropriate counter before delegating to the underlying transport.
type connTrackingTransport struct {
base http.RoundTripper
}

func (t *connTrackingTransport) RoundTrip(req *http.Request) (*http.Response, error) {
trace := &httptrace.ClientTrace{
GotConn: func(info httptrace.GotConnInfo) {
if info.Reused {
connStats.Reused.Add(1)
} else {
connStats.New.Add(1)
}
},
}
req = req.WithContext(httptrace.WithClientTrace(req.Context(), trace))
return t.base.RoundTrip(req)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Init initializes the clientpool implementation
func Init(options *types.Options) error {
if options.ShouldFollowHTTPRedirects() {
Expand Down Expand Up @@ -167,21 +201,15 @@ func GetRawHTTP(options *protocols.ExecutorOptions) *rawhttp.Client {
return dialers.RawHTTPClient
}

// Get creates or gets a client for the protocol based on custom configuration
func Get(options *types.Options, configuration *Configuration) (*retryablehttp.Client, error) {
if configuration.HasStandardOptions() {
dialers := protocolstate.GetDialersWithId(options.ExecutionId)
if dialers == nil {
return nil, fmt.Errorf("dialers not initialized for %s", options.ExecutionId)
}
return dialers.DefaultHTTPClient, nil
}

return wrappedGet(options, configuration)
// Get creates or gets a client for the protocol based on custom configuration.
// The host parameter scopes the client to a specific target, enabling per-host
// connection reuse with keep-alive. Pass an empty string for non-scanning uses.
func Get(options *types.Options, configuration *Configuration, host string) (*retryablehttp.Client, error) {
return wrappedGet(options, configuration, host)
}

// wrappedGet wraps a get operation without normal client check
func wrappedGet(options *types.Options, configuration *Configuration) (*retryablehttp.Client, error) {
func wrappedGet(options *types.Options, configuration *Configuration, host string) (*retryablehttp.Client, error) {
var err error

dialers := protocolstate.GetDialersWithId(options.ExecutionId)
Expand All @@ -190,28 +218,30 @@ func wrappedGet(options *types.Options, configuration *Configuration) (*retryabl
}

hash := configuration.Hash()
if host != "" {
hash += ":" + host
}
if client, ok := dialers.HTTPClientPool.Get(hash); ok {
return client, nil
}

// Multiple Host
retryableHttpOptions := retryablehttp.DefaultOptionsSpraying
disableKeepAlives := true
maxIdleConns := 0
maxConnsPerHost := 0
maxIdleConnsPerHost := -1
// do not split given timeout into chunks for retry
// because this won't work on slow hosts
// Each client is scoped to a single host, so we optimize for connection
// reuse: keep-alive always on, small idle pool, and an idle timeout that
// lets the transport reclaim unused connections automatically.
retryableHttpOptions := retryablehttp.DefaultOptionsSingle
retryableHttpOptions.NoAdjustTimeout = true

if configuration.Threads > 0 || options.ScanStrategy == scanstrategy.HostSpray.String() {
// Single host
retryableHttpOptions = retryablehttp.DefaultOptionsSingle
disableKeepAlives = false
maxIdleConnsPerHost = 500
maxConnsPerHost = 500
maxIdleConns := 4
maxIdleConnsPerHost := 4
maxConnsPerHost := 25
if configuration.Threads > 0 {
maxIdleConnsPerHost = configuration.Threads
maxIdleConns = configuration.Threads
maxConnsPerHost = configuration.Threads
}

disableKeepAlives := configuration.Connection != nil && configuration.Connection.DisableKeepAlive

Comment on lines 235 to +258
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Include Connection.DisableKeepAlive in the pool key.

wrappedGet caches by configuration.Hash() plus host, but Configuration.Hash() does not encode Connection.DisableKeepAlive. Any two non-nil ConnectionConfigurations that differ only in that flag will collide and can return a client with the wrong keep-alive behavior.

🩹 Proposed fix
 func (c *Configuration) Hash() string {
 	builder := &strings.Builder{}
 	builder.Grow(16)
@@
 	builder.WriteString("c")
 	builder.WriteString(strconv.FormatBool(c.Connection != nil))
+	if c.Connection != nil {
+		builder.WriteString("d")
+		builder.WriteString(strconv.FormatBool(c.Connection.DisableKeepAlive))
+	}
 	if c.Connection != nil && c.Connection.CustomMaxTimeout > 0 {
 		builder.WriteString("k")
 		builder.WriteString(c.Connection.CustomMaxTimeout.String())
 	}
🤖 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 220 - 244,
wrappedGet currently keys the pool by configuration.Hash() plus host but
configuration.Hash() doesn't include Connection.DisableKeepAlive, so clients
with different DisableKeepAlive values can collide; update the pool key
construction in clientpool.go (the code around wrappedGet / the local hash
variable) to append the effective DisableKeepAlive flag (e.g.,
strconv.FormatBool(disableKeepAlives) or "keepalive=off"/"on") to the hash
before calling dialers.HTTPClientPool.Get, ensuring the same unique symbols
(configuration.Hash(), host, disableKeepAlives) are used when creating and
retrieving the client so keep-alive behavior is correctly honored.

retryableHttpOptions.RetryWaitMax = 10 * time.Second
retryableHttpOptions.RetryMax = options.Retries
retryableHttpOptions.Timeout = time.Duration(options.Timeout) * time.Second
Expand All @@ -222,7 +252,6 @@ func wrappedGet(options *types.Options, configuration *Configuration) (*retryabl
maxRedirects := configuration.MaxRedirects

if forceMaxRedirects > 0 {
// by default we enable general redirects following
switch {
case options.FollowHostRedirects:
redirectFlow = FollowSameHostRedirect
Expand All @@ -238,30 +267,23 @@ func wrappedGet(options *types.Options, configuration *Configuration) (*retryabl
maxRedirects = 0
}

// override connection's settings if required
if configuration.Connection != nil {
disableKeepAlives = configuration.Connection.DisableKeepAlive
}

// Set the base TLS configuration definition
tlsConfig := &tls.Config{
Renegotiation: tls.RenegotiateOnceAsClient,
InsecureSkipVerify: true,
MinVersion: tls.VersionTLS10,
ClientSessionCache: tls.NewLRUClientSessionCache(1024),
ClientSessionCache: tls.NewLRUClientSessionCache(128),
}

if options.SNI != "" {
tlsConfig.ServerName = options.SNI
}

// Add the client certificate authentication to the request if it's configured
tlsConfig, err = utils.AddConfiguredClientCertToRequest(tlsConfig, options)
if err != nil {
return nil, errors.Wrap(err, "could not create client certificate")
}

// responseHeaderTimeout is max timeout for response headers to be read
responseHeaderTimeout := options.GetTimeouts().HttpResponseHeaderTimeout
if configuration.ResponseHeaderTimeout != 0 {
responseHeaderTimeout = configuration.ResponseHeaderTimeout
Expand Down Expand Up @@ -292,6 +314,7 @@ func wrappedGet(options *types.Options, configuration *Configuration) (*retryabl
MaxConnsPerHost: maxConnsPerHost,
TLSClientConfig: tlsConfig,
DisableKeepAlives: disableKeepAlives,
IdleConnTimeout: 30 * time.Second,
ResponseHeaderTimeout: responseHeaderTimeout,
}

Expand Down Expand Up @@ -333,6 +356,11 @@ func wrappedGet(options *types.Options, configuration *Configuration) (*retryabl
}
}

// Each per-host client gets its own default cookie jar. This is safe
// because cookies are domain-scoped per RFC 6265, and same-host iterations
// (workflows, multi-step templates) hit the same cached client so cookies
// are retained across requests. Explicit jars from input.CookieJar bypass
// the cache entirely for full isolation.
var jar *cookiejar.Jar
if configuration.Connection != nil && configuration.Connection.HasCookieJar() {
jar = configuration.Connection.GetCookieJar()
Expand All @@ -343,7 +371,7 @@ func wrappedGet(options *types.Options, configuration *Configuration) (*retryabl
}

httpclient := &http.Client{
Transport: transport,
Transport: &connTrackingTransport{base: transport},
CheckRedirect: makeCheckRedirectFunc(redirectFlow, maxRedirects),
}
if !configuration.NoTimeout {
Expand All @@ -358,8 +386,11 @@ func wrappedGet(options *types.Options, configuration *Configuration) (*retryabl
}
client.CheckRetry = retryablehttp.HostSprayRetryPolicy()

// Only add to client pool if we don't have a cookie jar in place.
if jar == nil {
// Cache the client unless it has an explicit per-request cookie jar.
// Default jars (from DisableCookie=false) are fine to cache since they
// just provide standard cookie handling within a host's connection pool.
hasExplicitJar := configuration.Connection != nil && configuration.Connection.HasCookieJar()
if !hasExplicitJar {
if err := dialers.HTTPClientPool.Set(hash, client); err != nil {
return nil, err
}
Expand Down
Loading
Loading