From 9c7b7c26a0a504b50dde322fecf43b593ff14344 Mon Sep 17 00:00:00 2001 From: Aykhan Shahsuvarov Date: Tue, 29 Sep 2026 10:58:25 +0400 Subject: [PATCH] refactor(script): drop the per-call maxBodySize and its client cache --- docs/configuration.md | 7 +-- internal/config/file.go | 5 ++ internal/sarin/filecache.go | 5 ++ internal/sarin/helpers.go | 7 --- internal/sarin/runner.go | 2 +- internal/sarin/scripthttp.go | 105 ++++++++++++----------------------- internal/script/http.go | 40 ------------- internal/script/js_http.go | 15 ----- internal/script/lua_http.go | 9 --- internal/script/script.go | 5 ++ 10 files changed, 54 insertions(+), 146 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 53cc207..c6d869e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -173,13 +173,13 @@ Valid time units: `ns`, `us` (or `µs`), `ms`, `s`, `m`, `h` Largest response body sarin reads. Defaults to `10MiB`, and `0` removes the limit. -The value is a whole number of bytes, with or without a unit: `B`, `KB`, `MB`, `GB`, `TB` are powers of 1000, `KiB`, `MiB`, `GiB`, `TiB` are powers of 1024. Case is ignored and a space before the unit is allowed. The largest accepted value is `1GiB`, because the limit is also the most memory one response may take. +The value is a whole number of bytes, with or without a unit: `B`, `KB`, `MB`, `GB`, `TB` are powers of 1000, `KiB`, `MiB`, `GiB`, `TiB` are powers of 1024. Case is ignored and a space before the unit is allowed. The largest accepted value is `1GiB`, because one response can take a few times the limit in memory. **Examples:** `1048576`, `10MiB`, `2MB`, `512KiB` A server can declare a `Content-Length` far larger than it sends, and the body is allocated from that declared length, so without a limit a single response can exhaust memory. A response above the limit fails with `body size exceeds the given limit` and is counted like any other error. -The limit applies per response in flight, and a declared Content-Length is allocated up to it, so the worst case is a few times the limit times the concurrency. Requests made by scripts have their own limit, see [HTTP Requests in Scripts](#http-requests-in-scripts). +The limit applies per response in flight, and a declared Content-Length is allocated up to it, so the worst case is a few times the limit times the concurrency. It covers the requests scripts make as well. ## Concurrency @@ -596,7 +596,6 @@ http(url, opts) | `timeout` | duration string (`"500ms"`) | `30s` | Request timeout (the `timeout` config does not apply), applied to each hop when redirects are followed | | `insecure` | boolean | `false` | Skip TLS verification (the `insecure` config does not apply) | | `maxRedirects` | number | `0` | How many redirects to follow, `0` follows none, maximum 100 | -| `maxBodySize` | number or size (`"10MiB"`) | `10MiB` | Largest response body to accept, `0` accepts any size, maximum `1GiB` | Unknown options and values of the wrong type raise an error. @@ -619,7 +618,7 @@ In Lua, call the helpers as `res:header("name")` or `res.header("name")`. - **Same proxy:** when proxies are configured, a script's requests go through the same proxy as the main request they belong to. - **Status codes:** non-2xx responses are returned normally; check `status` in the script. - **Errors:** network failures and timeouts raise a script error. The main request is not sent and the error is counted in the results. Scripts can catch it with `pcall` (Lua) or `try`/`catch` (JavaScript). -- **Response bodies:** script requests carry their own limit, like their own timeout, so the max response body config does not apply to them. A body above `maxBodySize` raises an error instead of being truncated, and a body whose declared `Content-Length` is already above it is refused before it is read. The limit is also the most memory one response may take, so `0` gives a server as much memory as it asks for. +- **Response bodies:** the [Max Response Body](#max-response-body) config bounds these requests too, and a body above it raises an error the script can catch. - **Only inside `transform`:** `http` works anywhere while `transform` runs, including in helper functions it calls. Calling it at the top level of a script raises an error, because that code also runs during config validation and when each worker starts. - **Dry run:** script requests are still sent in dry-run mode. - **Not measured:** script requests are not included in the results. diff --git a/internal/config/file.go b/internal/config/file.go index a774610..ed91b28 100644 --- a/internal/config/file.go +++ b/internal/config/file.go @@ -79,6 +79,11 @@ func fetchHTTP(ctx context.Context, url string) ([]byte, error) { return nil, types.NewHTTPStatusError(url, resp.StatusCode, resp.Status) } + // A declared size above the limit is refused before anything is read. + if resp.ContentLength > types.RemoteFetchLimit { + return nil, types.NewHTTPFetchError(url, types.ErrRemoteFileTooLarge) + } + data, err := io.ReadAll(io.LimitReader(resp.Body, types.RemoteFetchLimit+1)) if err != nil { return nil, types.NewHTTPFetchError(url, err) diff --git a/internal/sarin/filecache.go b/internal/sarin/filecache.go index d451c97..e866edd 100644 --- a/internal/sarin/filecache.go +++ b/internal/sarin/filecache.go @@ -93,6 +93,11 @@ func (fc *FileCache) fetchURL(url string) ([]byte, string, error) { return nil, "", types.NewHTTPStatusError(url, resp.StatusCode, resp.Status) } + // A declared size above the limit is refused before anything is read. + if resp.ContentLength > types.RemoteFetchLimit { + return nil, "", types.NewHTTPFetchError(url, types.ErrRemoteFileTooLarge) + } + content, err := io.ReadAll(io.LimitReader(resp.Body, types.RemoteFetchLimit+1)) if err != nil { return nil, "", types.NewHTTPFetchError(url, err) diff --git a/internal/sarin/helpers.go b/internal/sarin/helpers.go index 91c15b5..7cd6de1 100644 --- a/internal/sarin/helpers.go +++ b/internal/sarin/helpers.go @@ -34,10 +34,3 @@ func safeUint64ToInt(u uint64) int { } return int(u) } - -func safeInt64ToInt(i int64) int { - if i > math.MaxInt { - return math.MaxInt - } - return int(i) -} diff --git a/internal/sarin/runner.go b/internal/sarin/runner.go index bf7e7b3..60b72e8 100644 --- a/internal/sarin/runner.go +++ b/internal/sarin/runner.go @@ -281,7 +281,7 @@ func NewSarin( var scriptHTTPClients []*scriptHTTPClient if !scriptChain.IsEmpty() { - scriptHTTPClients, err = newScriptHTTPClients(jobsCtx, proxyURLs, workers) + scriptHTTPClients, err = newScriptHTTPClients(jobsCtx, proxyURLs, workers, maxResponseBody) if err != nil { jobsCancel() return nil, err diff --git a/internal/sarin/scripthttp.go b/internal/sarin/scripthttp.go index 6cca8c7..f5f48c1 100644 --- a/internal/sarin/scripthttp.go +++ b/internal/sarin/scripthttp.go @@ -6,8 +6,6 @@ import ( "errors" "net" "net/url" - "sync" - "sync/atomic" "time" "github.com/valyala/fasthttp" @@ -18,26 +16,11 @@ import ( // scriptHTTPDefaultTimeout is the default timeout for script requests, independent of -T. const scriptHTTPDefaultTimeout = 30 * time.Second -// scriptHTTPDefaultMaxBodySize is the default body limit for script requests, -// independent of the max response body config. -const scriptHTTPDefaultMaxBodySize = 10 << 20 // 10 MiB - -// scriptHTTPMaxClients bounds the client cache, since each one holds a connection pool. -const scriptHTTPMaxClients = 16 - -// scriptHTTPClientKey picks the client a request needs, since fasthttp takes the body -// limit per client rather than per request. -type scriptHTTPClientKey struct { - insecure bool - maxBodySize int -} - // scriptHTTPClient sends scripts' http.* requests. It is safe for concurrent use. type scriptHTTPClient struct { ctx context.Context //nolint:containedctx - newClient func(key scriptHTTPClientKey) *fasthttp.Client - clients sync.Map - clientCount atomic.Int64 + client *fasthttp.Client + insecureClient *fasthttp.Client defaultTimeout time.Duration } @@ -46,9 +29,9 @@ var _ script.HTTPDoer = (*scriptHTTPClient)(nil) // newScriptHTTPClients creates one client per proxy, in the same order as NewHostClients. // It can return the following errors: // - types.ProxyDialError -func newScriptHTTPClients(ctx context.Context, proxies []url.URL, maxConns uint) ([]*scriptHTTPClient, error) { +func newScriptHTTPClients(ctx context.Context, proxies []url.URL, maxConns uint, maxResponseBody uint64) ([]*scriptHTTPClient, error) { if len(proxies) == 0 { - return []*scriptHTTPClient{newScriptHTTPClient(ctx, fasthttp.DialDualStackTimeout, scriptHTTPDefaultTimeout, maxConns)}, nil + return []*scriptHTTPClient{newScriptHTTPClient(ctx, fasthttp.DialDualStackTimeout, scriptHTTPDefaultTimeout, maxConns, maxResponseBody)}, nil } clients := make([]*scriptHTTPClient, 0, len(proxies)) @@ -57,42 +40,42 @@ func newScriptHTTPClients(ctx context.Context, proxies []url.URL, maxConns uint) if err != nil { return nil, types.NewProxyDialError(proxy.String(), err) } - clients = append(clients, newScriptHTTPClient(ctx, dial, scriptHTTPDefaultTimeout, maxConns)) + clients = append(clients, newScriptHTTPClient(ctx, dial, scriptHTTPDefaultTimeout, maxConns, maxResponseBody)) } return clients, nil } -func newScriptHTTPClient(ctx context.Context, dial fasthttp.DialFuncWithTimeout, defaultTimeout time.Duration, maxConns uint) *scriptHTTPClient { +func newScriptHTTPClient( + ctx context.Context, + dial fasthttp.DialFuncWithTimeout, + defaultTimeout time.Duration, + maxConns uint, + maxResponseBody uint64, +) *scriptHTTPClient { + newClient := func(insecure bool) *fasthttp.Client { + // Read and write timeouts stay unset because they would cap the per-call timeout, + // so dialWithDeadline is what bounds the TLS handshake. + return &fasthttp.Client{ + DialTimeout: dialWithDeadline(dial), + MaxConnsPerHost: safeUintToInt(maxConns), + MaxResponseBodySize: safeUint64ToInt(maxResponseBody), + TLSConfig: &tls.Config{ + InsecureSkipVerify: insecure, //nolint:gosec + }, + DisableHeaderNamesNormalizing: true, + DisablePathNormalizing: true, + NoDefaultUserAgentHeader: true, + } + } + return &scriptHTTPClient{ - ctx: ctx, - newClient: func(key scriptHTTPClientKey) *fasthttp.Client { - // Read and write timeouts stay unset because they would cap the per-call timeout, - // so dialWithDeadline is what bounds the TLS handshake. - return &fasthttp.Client{ - DialTimeout: dialWithDeadline(dial), - MaxConnsPerHost: safeUintToInt(maxConns), - MaxResponseBodySize: key.maxBodySize, - TLSConfig: &tls.Config{ - InsecureSkipVerify: key.insecure, //nolint:gosec - }, - DisableHeaderNamesNormalizing: true, - DisablePathNormalizing: true, - NoDefaultUserAgentHeader: true, - } - }, + ctx: ctx, + client: newClient(false), + insecureClient: newClient(true), defaultTimeout: defaultTimeout, } } -// scriptHTTPBodyLimit resolves the limit a request is read with, where a request -// without one takes the default and zero means no limit. -func scriptHTTPBodyLimit(maxBodySize *int64) int { - if maxBodySize == nil { - return scriptHTTPDefaultMaxBodySize - } - return safeInt64ToInt(*maxBodySize) -} - // dialWithDeadline dials within the request timeout and keeps it as the connection deadline, // so the TLS handshake fasthttp runs on the first write cannot outlive the request. func dialWithDeadline(dial fasthttp.DialFuncWithTimeout) fasthttp.DialFuncWithTimeout { @@ -163,8 +146,10 @@ func (c *scriptHTTPClient) Do(r *script.HTTPRequest) (*script.HTTPResponse, erro timeout = c.defaultTimeout } - limit := scriptHTTPBodyLimit(r.MaxBodySize) - client := c.clientFor(r.Insecure, limit) + client := c.client + if r.Insecure { + client = c.insecureClient + } req.SetTimeout(timeout) if r.MaxRedirects > 0 { @@ -187,23 +172,3 @@ func (c *scriptHTTPClient) Do(r *script.HTTPRequest) (*script.HTTPResponse, erro Body: string(body), }, nil } - -// clientFor returns the client enforcing maxBodySize, building it once. -func (c *scriptHTTPClient) clientFor(insecure bool, maxBodySize int) *fasthttp.Client { - key := scriptHTTPClientKey{insecure: insecure, maxBodySize: maxBodySize} - if client, ok := c.clients.Load(key); ok { - return client.(*fasthttp.Client) //nolint:forcetypeassert - } - - client := c.newClient(key) - // A script that varies its limit would otherwise grow the cache without end. - if c.clientCount.Load() >= scriptHTTPMaxClients { - return client - } - - stored, loaded := c.clients.LoadOrStore(key, client) - if !loaded { - c.clientCount.Add(1) - } - return stored.(*fasthttp.Client) //nolint:forcetypeassert -} diff --git a/internal/script/http.go b/internal/script/http.go index aabf3f2..0b80b1f 100644 --- a/internal/script/http.go +++ b/internal/script/http.go @@ -21,7 +21,6 @@ type HTTPRequest struct { Timeout time.Duration // zero means the doer's default timeout Insecure bool MaxRedirects int - MaxBodySize *int64 // nil takes the doer default, zero means no limit } // HTTPResponse is the response returned to a script by the http.* functions. @@ -67,9 +66,6 @@ const httpDefaultMethod = http.MethodGet // httpMaxRedirectsLimit is the highest maxRedirects a script may ask for. const httpMaxRedirectsLimit = 100 -// httpMaxBodySizeLimit is the highest maxBodySize a script may ask for. -const httpMaxBodySizeLimit int64 = types.ByteSizeLimit - // Option keys accepted by the http function. const ( httpOptionMethod = "method" @@ -80,7 +76,6 @@ const ( httpOptionTimeout = "timeout" httpOptionInsecure = "insecure" httpOptionMaxRedirects = "maxRedirects" - httpOptionMaxBodySize = "maxBodySize" ) // httpBridge connects http to the doer. Calls only work while transform runs. @@ -123,41 +118,6 @@ func parseHTTPMaxRedirects(count float64) (int, error) { return int(count), nil } -// parseHTTPMaxBodySize checks that the body limit is a non-negative whole number. -// It can return the following errors: -// - types.ScriptHTTPOptionError -func parseHTTPMaxBodySize(size float64) (*int64, error) { - if math.IsNaN(size) || size < 0 || size > float64(httpMaxBodySizeLimit) || size != math.Trunc(size) { - return nil, types.NewScriptHTTPOptionError( - httpOptionMaxBodySize, - types.NewScriptTypeError( - "a whole number of bytes between 0 and "+strconv.FormatInt(httpMaxBodySizeLimit, 10), - strconv.FormatFloat(size, 'f', -1, 64), - ), - ) - } - limit := int64(size) - return &limit, nil -} - -// parseHTTPMaxBodySizeString reads a body limit written with a unit, such as "10MiB". -// It can return the following errors: -// - types.ScriptHTTPOptionError -func parseHTTPMaxBodySizeString(value string) (*int64, error) { - size, err := types.ParseByteSize(value) - if err != nil { - return nil, types.NewScriptHTTPOptionError(httpOptionMaxBodySize, err) - } - if size > uint64(httpMaxBodySizeLimit) { - return nil, types.NewScriptHTTPOptionError( - httpOptionMaxBodySize, - types.NewScriptTypeError("a size up to "+types.FormatByteSize(uint64(httpMaxBodySizeLimit)), value), - ) - } - limit := int64(size) - return &limit, nil -} - // parseHTTPTimeout parses a Go duration string such as "500ms" or "2s". // It can return the following errors: // - types.ScriptHTTPOptionError diff --git a/internal/script/js_http.go b/internal/script/js_http.go index becf8cc..7cdce59 100644 --- a/internal/script/js_http.go +++ b/internal/script/js_http.go @@ -117,21 +117,6 @@ func (e *JsEngine) parseHTTPOptions(opts *goja.Object, req *HTTPRequest) error { if req.MaxRedirects, err = parseHTTPMaxRedirects(count); err != nil { return err } - case httpOptionMaxBodySize: - var err error - switch size := value.Export().(type) { - case string: - req.MaxBodySize, err = parseHTTPMaxBodySizeString(size) - default: - number, ok := numberValue(value) - if !ok { - return types.NewScriptHTTPOptionError(name, types.NewScriptTypeError("number or size string", jsTypeName(value))) - } - req.MaxBodySize, err = parseHTTPMaxBodySize(number) - } - if err != nil { - return err - } default: return types.NewScriptHTTPOptionError(name, types.ErrScriptHTTPUnknownOption) } diff --git a/internal/script/lua_http.go b/internal/script/lua_http.go index 7750c6b..a1e3a14 100644 --- a/internal/script/lua_http.go +++ b/internal/script/lua_http.go @@ -106,15 +106,6 @@ func (e *LuaEngine) parseHTTPOptions(opts *lua.LTable, req *HTTPRequest) error { return } req.MaxRedirects, err = parseHTTPMaxRedirects(float64(count)) - case httpOptionMaxBodySize: - switch size := v.(type) { - case lua.LNumber: - req.MaxBodySize, err = parseHTTPMaxBodySize(float64(size)) - case lua.LString: - req.MaxBodySize, err = parseHTTPMaxBodySizeString(string(size)) - default: - err = types.NewScriptHTTPOptionError(name, types.NewScriptTypeError("number or size string", v.Type().String())) - } default: err = types.NewScriptHTTPOptionError(name, types.ErrScriptHTTPUnknownOption) } diff --git a/internal/script/script.go b/internal/script/script.go index 124fda0..30aaf0e 100644 --- a/internal/script/script.go +++ b/internal/script/script.go @@ -171,6 +171,11 @@ func fetchURL(ctx context.Context, url string) (string, error) { return "", types.NewHTTPStatusError(url, resp.StatusCode, resp.Status) } + // A declared size above the limit is refused before anything is read. + if resp.ContentLength > types.RemoteFetchLimit { + return "", types.NewHTTPFetchError(url, types.ErrRemoteFileTooLarge) + } + data, err := io.ReadAll(io.LimitReader(resp.Body, types.RemoteFetchLimit+1)) if err != nil { return "", types.NewHTTPFetchError(url, err)