refactor(script): drop the per-call maxBodySize and its client cache

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.

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)

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)

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)
-}

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

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
+	client         *fasthttp.Client
-	clients        sync.Map
+	insecureClient *fasthttp.Client
-	clientCount    atomic.Int64
 	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,
+		ctx:            ctx,
-		newClient: func(key scriptHTTPClientKey) *fasthttp.Client {
+		client:         newClient(false),
-			// Read and write timeouts stay unset because they would cap the per-call timeout,
+		insecureClient: newClient(true),
-			// 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,
-			}
-		},
 		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.client
-	client := c.clientFor(r.Insecure, limit)
+	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
-}

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

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)
 		}

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)
 		}

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)