Improve HTTP request and response correctness

docs/cli-reference.md

@@ -46,6 +46,11 @@ fetch -X DELETE example.com/resource/123
 
 Set custom headers. Can be used multiple times.
 
+Request framing is validated before connecting. Repeated `Content-Length`
+values must agree and are normalized to one value; `Content-Length` cannot be
+combined with `Transfer-Encoding`, and `chunked` is the only supported request
+transfer encoding.
+
 ```sh
 fetch -H "Authorization: Bearer token" example.com
 fetch -H "X-Custom: value" -H "Accept: application/json" example.com
@@ -469,7 +474,7 @@ allowed.
 Maximum number of retries for transient failures. Default: `0` (no retries).
 The value must be between `0` and `100`.
 
-Retries occur on connection errors and retryable status codes (429, 502, 503, 504) for GET, HEAD, OPTIONS, and TRACE requests. Non-retryable errors (4xx, TLS certificate errors) are not retried. PUT and DELETE are not retried by default: although HTTP describes them as idempotent, individual APIs may implement side effects that are unsafe to repeat. POST, PATCH, PUT, DELETE, and custom methods require the explicit `--retry-unsafe` opt-in. Uses exponential backoff with jitter between attempts.
+Retries occur on transient connection errors and retryable status codes (408, 425, 429, 500, 502, 503, 504) for GET, HEAD, OPTIONS, and TRACE requests. DNS name-not-found failures, other non-retryable 4xx responses, and TLS certificate errors are not retried. PUT and DELETE are not retried by default: although HTTP describes them as idempotent, individual APIs may implement side effects that are unsafe to repeat. POST, PATCH, PUT, DELETE, and custom methods require the explicit `--retry-unsafe` opt-in. Uses exponential backoff with jitter between attempts.
 
 All attempts, redirects, response reads, bounded drains, and retry delays share one `--timeout` wall-clock budget. A request body must be replayable before a retry starts. Only the final attempt's response body is written to stdout. Retry notifications are printed to stderr (suppressed with `--silent`).
 

docs/configuration.md

@@ -381,7 +381,7 @@ redirects remain allowed.
 **Type**: Integer
 **Default**: `0` (no retries)
 
-Maximum number of retries for transient failures. Retries occur on connection errors and retryable status codes (429, 502, 503, 504) only for GET, HEAD, OPTIONS, and TRACE by default. PUT, DELETE, POST, PATCH, and custom methods require `retry-unsafe = true`.
+Maximum number of retries for transient failures. Retries occur on transient connection errors and retryable status codes (408, 425, 429, 500, 502, 503, 504) only for GET, HEAD, OPTIONS, and TRACE by default. DNS name-not-found failures are not retried. PUT, DELETE, POST, PATCH, and custom methods require `retry-unsafe = true`.
 The value must be between `0` and `100`.
 
 ```ini

internal/client/client.go

@@ -1088,6 +1088,7 @@ func (c *Client) NewRequest(ctx context.Context, cfg RequestConfig) (*http.Reque
 	if source != nil {
 		body.Attach(req, source)
 	}
+	inferredContentLength := req.ContentLength
 	if urlBasic != nil {
 		req.SetBasicAuth(urlBasic.Key, urlBasic.Val)
 		MarkCredentialHeaders(req, "Authorization")
@@ -1121,6 +1122,9 @@ func (c *Client) NewRequest(ctx context.Context, cfg RequestConfig) (*http.Reque
 	// explicit value, then append so repeated headers remain distinct.
 	seenHeaders := make(map[string]struct{}, len(cfg.Headers))
 	acceptEncodingSet := false
+	var contentLengths []int64
+	var transferEncodings []string
+	transferEncodingSet := false
 	for _, kv := range cfg.Headers {
 		if strings.EqualFold(kv.Key, "Host") {
 			req.Host = kv.Val
@@ -1147,20 +1151,62 @@ func (c *Client) NewRequest(ctx context.Context, cfg RequestConfig) (*http.Reque
 			if err != nil || length < 0 {
 				return nil, fmt.Errorf("invalid Content-Length header %q", kv.Val)
 			}
-			req.ContentLength = length
+			contentLengths = append(contentLengths, length)
 		case "transfer-encoding":
-			var encodings []string
+			transferEncodingSet = true
 			for encoding := range strings.SplitSeq(kv.Val, ",") {
-				encoding = strings.TrimSpace(encoding)
+				encoding = strings.ToLower(strings.TrimSpace(encoding))
 				if encoding != "" {
-					encodings = append(encodings, encoding)
+					transferEncodings = append(transferEncodings, encoding)
 				}
 			}
-			req.TransferEncoding = encodings
-			if len(encodings) > 0 {
-				req.ContentLength = -1
+		}
+	}
+
+	// Content-Length and Transfer-Encoding are framing metadata represented by
+	// dedicated Request fields. Validate them here so malformed or ambiguous
+	// framing fails before any connection is opened. Equal duplicate lengths
+	// are safe but are normalized to one value; differing values are rejected.
+	if len(contentLengths) > 0 {
+		length := contentLengths[0]
+		for _, other := range contentLengths[1:] {
+			if other != length {
+				return nil, fmt.Errorf("conflicting Content-Length headers: %d and %d", length, other)
 			}
 		}
+		if length == 0 && req.Body != nil && req.Body != http.NoBody {
+			return nil, errors.New("Content-Length 0 cannot be used with a request body")
+		}
+		if length == 0 && (req.Method == http.MethodGet || req.Method == http.MethodHead) {
+			return nil, fmt.Errorf("Content-Length 0 is not transmitted for %s requests without a body", req.Method)
+		}
+		if length > 0 && (req.Body == nil || req.Body == http.NoBody) {
+			return nil, errors.New("positive Content-Length cannot be used without a request body")
+		}
+		if inferredContentLength >= 0 && req.Body != nil && req.Body != http.NoBody && length != inferredContentLength {
+			return nil, fmt.Errorf("Content-Length %d does not match request body length %d", length, inferredContentLength)
+		}
+		req.ContentLength = length
+		req.Header.Set("Content-Length", strconv.FormatInt(length, 10))
+	}
+	if transferEncodingSet && len(transferEncodings) == 0 {
+		return nil, errors.New("Transfer-Encoding must specify chunked")
+	}
+	if len(transferEncodings) > 0 {
+		// Go's HTTP transports support chunked request framing only. Reject other
+		// codings early instead of surfacing an opaque error from RoundTrip.
+		if len(transferEncodings) != 1 || transferEncodings[0] != "chunked" {
+			return nil, fmt.Errorf("unsupported Transfer-Encoding %q; only chunked is supported", strings.Join(transferEncodings, ", "))
+		}
+		if len(contentLengths) > 0 {
+			return nil, errors.New("Content-Length and Transfer-Encoding cannot be used together")
+		}
+		if req.Body == nil || req.Body == http.NoBody {
+			return nil, errors.New("Transfer-Encoding chunked requires a request body")
+		}
+		req.TransferEncoding = transferEncodings
+		req.ContentLength = -1
+		req.Header.Set("Transfer-Encoding", "chunked")
 	}
 
 	// Set the compression policy after explicit headers have been applied. An

internal/client/client_test.go

@@ -234,6 +234,222 @@ func TestNewRequestDoesNotAccumulateURLDefaults(t *testing.T) {
 	}
 }
 
+func TestNewRequestValidatesFramingHeaders(t *testing.T) {
+	tests := []struct {
+		name    string
+		headers []core.KeyVal[string]
+		wantErr string
+	}{
+		{
+			name: "conflicting content lengths",
+			headers: []core.KeyVal[string]{
+				{Key: "Content-Length", Val: "3"},
+				{Key: "content-length", Val: "4"},
+			},
+			wantErr: "conflicting Content-Length headers",
+		},
+		{
+			name: "content length and transfer encoding",
+			headers: []core.KeyVal[string]{
+				{Key: "Content-Length", Val: "3"},
+				{Key: "Transfer-Encoding", Val: "chunked"},
+			},
+			wantErr: "cannot be used together",
+		},
+		{
+			name:    "unsupported transfer encoding",
+			headers: []core.KeyVal[string]{{Key: "Transfer-Encoding", Val: "gzip, chunked"}},
+			wantErr: "only chunked is supported",
+		},
+		{
+			name:    "zero content length with body",
+			headers: []core.KeyVal[string]{{Key: "Content-Length", Val: "0"}},
+			wantErr: "cannot be used with a request body",
+		},
+		{
+			name:    "content length smaller than body",
+			headers: []core.KeyVal[string]{{Key: "Content-Length", Val: "2"}},
+			wantErr: "does not match request body length 3",
+		},
+		{
+			name:    "content length larger than body",
+			headers: []core.KeyVal[string]{{Key: "Content-Length", Val: "4"}},
+			wantErr: "does not match request body length 3",
+		},
+		{
+			name:    "empty transfer encoding",
+			headers: []core.KeyVal[string]{{Key: "Transfer-Encoding", Val: " , "}},
+			wantErr: "must specify chunked",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := NewClient(ClientConfig{}).NewRequest(context.Background(), RequestConfig{
+				Data:    strings.NewReader("abc"),
+				Headers: tt.headers,
+				URL:     mustURL(t, "https://example.com"),
+			})
+			if err == nil || !strings.Contains(err.Error(), tt.wantErr) {
+				t.Fatalf("error = %v, want substring %q", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestNewRequestRejectsPositiveContentLengthWithoutBody(t *testing.T) {
+	_, err := NewClient(ClientConfig{}).NewRequest(context.Background(), RequestConfig{
+		Headers: []core.KeyVal[string]{{Key: "Content-Length", Val: "1"}},
+		URL:     mustURL(t, "https://example.com"),
+	})
+	if err == nil || !strings.Contains(err.Error(), "without a request body") {
+		t.Fatalf("error = %v, want bodyless Content-Length error", err)
+	}
+}
+
+func TestNewRequestAllowsZeroContentLengthForBodylessPost(t *testing.T) {
+	req, err := NewClient(ClientConfig{}).NewRequest(context.Background(), RequestConfig{
+		Headers: []core.KeyVal[string]{{Key: "Content-Length", Val: "0"}},
+		Method:  http.MethodPost,
+		URL:     mustURL(t, "https://example.com"),
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if req.Body != nil && req.Body != http.NoBody {
+		t.Fatalf("body = %T, want no body", req.Body)
+	}
+	if req.ContentLength != 0 || req.Header.Get("Content-Length") != "0" {
+		t.Fatalf("content length = %d, header = %q", req.ContentLength, req.Header.Get("Content-Length"))
+	}
+}
+
+func TestNewRequestRejectsFramingThatTransportWouldStrip(t *testing.T) {
+	tests := []struct {
+		name   string
+		method string
+		header core.KeyVal[string]
+		want   string
+	}{
+		{name: "zero length GET", method: http.MethodGet, header: core.KeyVal[string]{Key: "Content-Length", Val: "0"}, want: "is not transmitted"},
+		{name: "zero length HEAD", method: http.MethodHead, header: core.KeyVal[string]{Key: "Content-Length", Val: "0"}, want: "is not transmitted"},
+		{name: "bodyless chunked", method: http.MethodPost, header: core.KeyVal[string]{Key: "Transfer-Encoding", Val: "chunked"}, want: "requires a request body"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := NewClient(ClientConfig{}).NewRequest(context.Background(), RequestConfig{
+				Headers: []core.KeyVal[string]{tt.header},
+				Method:  tt.method,
+				URL:     mustURL(t, "https://example.com"),
+			})
+			if err == nil || !strings.Contains(err.Error(), tt.want) {
+				t.Fatalf("error = %v, want substring %q", err, tt.want)
+			}
+		})
+	}
+}
+
+func TestExplicitRequestFramingReachesWire(t *testing.T) {
+	type receivedRequest struct {
+		contentLength    int64
+		transferEncoding []string
+		body             string
+	}
+	received := make(chan receivedRequest, 2)
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		data, err := io.ReadAll(r.Body)
+		if err != nil {
+			t.Errorf("reading request body: %v", err)
+		}
+		received <- receivedRequest{
+			contentLength:    r.ContentLength,
+			transferEncoding: slices.Clone(r.TransferEncoding),
+			body:             string(data),
+		}
+		w.WriteHeader(http.StatusNoContent)
+	}))
+	defer server.Close()
+
+	c := NewClient(ClientConfig{})
+	defer c.Close()
+	tests := []struct {
+		name     string
+		cfg      RequestConfig
+		wantCL   int64
+		wantTE   []string
+		wantBody string
+	}{
+		{
+			name: "zero-length POST",
+			cfg: RequestConfig{
+				Headers: []core.KeyVal[string]{{Key: "Content-Length", Val: "0"}},
+				Method:  http.MethodPost,
+			},
+			wantCL: 0,
+		},
+		{
+			name: "chunked body",
+			cfg: RequestConfig{
+				Data:    strings.NewReader("abc"),
+				Headers: []core.KeyVal[string]{{Key: "Transfer-Encoding", Val: "chunked"}},
+				Method:  http.MethodPost,
+			},
+			wantCL:   -1,
+			wantTE:   []string{"chunked"},
+			wantBody: "abc",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tt.cfg.URL = mustURL(t, server.URL)
+			req, err := c.NewRequest(context.Background(), tt.cfg)
+			if err != nil {
+				t.Fatal(err)
+			}
+			resp, err := c.Do(req)
+			if err != nil {
+				t.Fatal(err)
+			}
+			resp.Body.Close()
+			got := <-received
+			if got.contentLength != tt.wantCL || !slices.Equal(got.transferEncoding, tt.wantTE) || got.body != tt.wantBody {
+				t.Fatalf("wire framing = length %d, encodings %q, body %q", got.contentLength, got.transferEncoding, got.body)
+			}
+		})
+	}
+}
+
+func TestNewRequestNormalizesSafeFramingHeaders(t *testing.T) {
+	req, err := NewClient(ClientConfig{}).NewRequest(context.Background(), RequestConfig{
+		Data: strings.NewReader("abc"),
+		Headers: []core.KeyVal[string]{
+			{Key: "Content-Length", Val: "3"},
+			{Key: "content-length", Val: "3"},
+		},
+		URL: mustURL(t, "https://example.com"),
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer req.Body.Close()
+	if req.ContentLength != 3 || !slices.Equal(req.Header.Values("Content-Length"), []string{"3"}) {
+		t.Fatalf("content length = %d, headers = %q", req.ContentLength, req.Header.Values("Content-Length"))
+	}
+
+	chunked, err := NewClient(ClientConfig{}).NewRequest(context.Background(), RequestConfig{
+		Data:    strings.NewReader("abc"),
+		Headers: []core.KeyVal[string]{{Key: "Transfer-Encoding", Val: "CHUNKED"}},
+		URL:     mustURL(t, "https://example.com"),
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer chunked.Body.Close()
+	if chunked.ContentLength != -1 || !slices.Equal(chunked.TransferEncoding, []string{"chunked"}) {
+		t.Fatalf("chunked framing = length %d, encodings %q", chunked.ContentLength, chunked.TransferEncoding)
+	}
+}
+
 func TestNewRequestRejectsNilURL(t *testing.T) {
 	_, err := NewClient(ClientConfig{}).NewRequest(context.Background(), RequestConfig{})
 	if err == nil || err.Error() != "request URL is required" {

internal/fetch/fetch.go

@@ -712,7 +712,11 @@ func formatResponse(ctx context.Context, r *Request, resp *http.Response, cc *cl
 	}
 
 	if output != "" && r.Output != "-" {
-		size := client.WireContentLength(resp)
+		// Progress tracks the bytes written to disk. Once a response has been
+		// decoded, its compressed wire length is not the output length and can
+		// make the progress bar exceed 100%. Decoders set ContentLength to -1,
+		// which correctly selects the unknown-size spinner in that case.
+		size := responseOutputSize(resp)
 		p := r.PrinterHandle.Stderr()
 		return nil, writeOutputToFile(output, resp.Body, size, p, r.Verbosity, r.Clobber)
 	}
@@ -818,6 +822,13 @@ func formatResponse(ctx context.Context, r *Request, resp *http.Response, cc *cl
 	return newUntrustedResponseReader(bytes.NewReader(buf)), nil
 }
 
+func responseOutputSize(resp *http.Response) int64 {
+	if resp == nil {
+		return -1
+	}
+	return resp.ContentLength
+}
+
 func rejectHAROutputPath(r *Request, output string) error {
 	if r == nil || r.harRecorder == nil || output == "" || output == "-" {
 		return nil

internal/fetch/format_response_test.go

@@ -26,6 +26,20 @@ func TestFormatWithBoundedOutput(t *testing.T) {
 	}
 }
 
+func TestResponseOutputSizeUsesDecodedLength(t *testing.T) {
+	resp := &http.Response{ContentLength: -1}
+	if got := responseOutputSize(resp); got != -1 {
+		t.Fatalf("responseOutputSize = %d, want unknown decoded length", got)
+	}
+	resp.ContentLength = 123
+	if got := responseOutputSize(resp); got != 123 {
+		t.Fatalf("responseOutputSize = %d, want 123", got)
+	}
+	if got := responseOutputSize(nil); got != -1 {
+		t.Fatalf("responseOutputSize(nil) = %d, want -1", got)
+	}
+}
+
 func TestFormatResponseFormatsExactMaxBodyBytes(t *testing.T) {
 	prefix := []byte(`{"a":1}`)
 	body := append(append([]byte(nil), prefix...), bytes.Repeat([]byte(" "), maxBodyBytes-len(prefix))...)