fix(http): keep request URL normalization reusable

integration/integration_test.go

@@ -94,6 +94,26 @@ func TestMain(t *testing.T) {
 		assertBufContains(t, res.stderr, "POST / HTTP/1.1")
 	})
 
+	t.Run("schemeless plaintext hint uses the effective URL", func(t *testing.T) {
+		t.Parallel()
+		server := startServer(func(http.ResponseWriter, *http.Request) {})
+		defer server.Close()
+
+		_, port, err := net.SplitHostPort(strings.TrimPrefix(server.URL, "http://"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		host := "plaintext-hint.example.test"
+		target := host + ":" + port + "/path?existing=one"
+		res := runFetchOpts(t, fetchPath, fetchOpts{env: []string{
+			"HTTP_PROXY=", "http_proxy=", "HTTPS_PROXY=", "https_proxy=", "ALL_PROXY=", "all_proxy=",
+			"NO_PROXY=*", "no_proxy=*",
+		}}, target, "--query", "added=two words", "--resolve", host+":"+port+":127.0.0.1")
+		assertExitCode(t, 1, res)
+		assertBufEmpty(t, res.stdout)
+		assertBufContains(t, res.stderr, "If this is a plaintext service, use http://"+host+":"+port+"/path?existing=one&added=two%20words.")
+	})
+
 	t.Run("invalid flag", func(t *testing.T) {
 		t.Parallel()
 		res := runFetch(t, fetchPath, "--invalid")

internal/client/client.go

@@ -1001,6 +1001,10 @@ type RequestConfig struct {
 
 // NewRequest returns an *http.Request given the provided configuration.
 func (c *Client) NewRequest(ctx context.Context, cfg RequestConfig) (*http.Request, error) {
+	if cfg.URL == nil {
+		return nil, errors.New("request URL is required")
+	}
+
 	// URL userinfo is an authentication source, not part of the request URL.
 	// Convert it to Basic auth before constructing the request so diagnostics,
 	// redirects, and signatures never retain credentials in the URL. Explicit
@@ -1016,6 +1020,12 @@ func (c *Client) NewRequest(ctx context.Context, cfg RequestConfig) (*http.Reque
 		cfg.URL.User = nil
 	}
 
+	// Scheme defaults and query parameters belong to this request. Apply them
+	// to a copy so callers can safely reuse their parsed URL without accumulating
+	// query parameters. URL userinfo is intentionally scrubbed above.
+	requestURL := *cfg.URL
+	cfg.URL = &requestURL
+
 	// Append query params directly to RawQuery. url.Values.Encode sorts keys,
 	// which loses the user's ordering even though duplicate parameters are
 	// valid and meaningful to many servers.

internal/client/client_test.go

@@ -210,6 +210,37 @@ func TestNewRequestUsesLazyReplayableFileBody(t *testing.T) {
 	}
 }
 
+func TestNewRequestDoesNotAccumulateURLDefaults(t *testing.T) {
+	u := &url.URL{Host: "example.com", Path: "/path", RawQuery: "existing=one"}
+	wantURL := u.String()
+	c := NewClient(ClientConfig{})
+	cfg := RequestConfig{
+		QueryParams: []core.KeyVal[string]{{Key: "added", Val: "two words"}},
+		URL:         u,
+	}
+
+	for range 2 {
+		req, err := c.NewRequest(context.Background(), cfg)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if got := req.URL.String(); got != "https://example.com/path?existing=one&added=two%20words" {
+			t.Fatalf("request URL = %q", got)
+		}
+	}
+
+	if got := u.String(); got != wantURL {
+		t.Fatalf("input URL mutated: got %q, want %q", got, wantURL)
+	}
+}
+
+func TestNewRequestRejectsNilURL(t *testing.T) {
+	_, err := NewClient(ClientConfig{}).NewRequest(context.Background(), RequestConfig{})
+	if err == nil || err.Error() != "request URL is required" {
+		t.Fatalf("error = %v, want request URL is required", err)
+	}
+}
+
 func TestCLI003RequestDefaultsAndOrdering(t *testing.T) {
 	u, err := url.Parse("https://example.com/path?z=old&space=hello+world")
 	if err != nil {

internal/fetch/fetch.go

@@ -122,6 +122,7 @@ type Request struct {
 
 	// responseDescriptor is set internally after proto setup for response formatting.
 	responseDescriptor protoreflect.MessageDescriptor
+	effectiveURL       *url.URL
 
 	// harRecorder is reserved before the request starts and records the final
 	// response exchange. It remains private so callers only provide a path.
@@ -168,6 +169,7 @@ func Fetch(ctx context.Context, r *Request) int {
 }
 
 func fetch(ctx context.Context, r *Request) (int, error) {
+	r.effectiveURL = nil
 	if r.HAR != "" {
 		if r.WS || r.GRPCList || r.GRPCDescribe != "" || r.DryRun {
 			return 0, errors.New("--har cannot be used with WebSocket, gRPC discovery, or --dry-run")
@@ -292,6 +294,10 @@ func fetch(ctx context.Context, r *Request) (int, error) {
 	if err != nil {
 		return 0, err
 	}
+	// Keep the effective URL separately for diagnostics that run after the
+	// request has failed. The caller's input must remain reusable without
+	// accumulating query parameters.
+	r.effectiveURL = req.URL
 	defer func() {
 		if req.Body != nil {
 			req.Body.Close()
@@ -867,6 +873,8 @@ func formatArticleResponse(ctx context.Context, r *Request, resp *http.Response,
 	pageURL := ""
 	if resp.Request != nil && resp.Request.URL != nil {
 		pageURL = resp.Request.URL.String()
+	} else if r.effectiveURL != nil {
+		pageURL = r.effectiveURL.String()
 	} else if r.URL != nil {
 		pageURL = r.URL.String()
 	}
@@ -1059,11 +1067,18 @@ func addHeader(headers []core.KeyVal[string], h core.KeyVal[string]) []core.KeyV
 // schemeless HTTPS connection when the failure is not a certificate or timeout
 // error.
 func schemelessPlaintextHint(r *Request, err error) string {
-	if r == nil || !r.SchemelessURL || r.URL == nil || r.URL.Scheme != "https" || isCertificateErr(err) {
+	if r == nil || !r.SchemelessURL || isCertificateErr(err) {
+		return ""
+	}
+	effectiveURL := r.effectiveURL
+	if effectiveURL == nil {
+		effectiveURL = r.URL
+	}
+	if effectiveURL == nil || effectiveURL.Scheme != "https" {
 		return ""
 	}
 	var recordErr tls.RecordHeaderError
-	if !errors.As(err, &recordErr) {
+	if !errors.As(err, &recordErr) && !errors.Is(err, http.ErrSchemeMismatch) {
 		return ""
 	}
 	if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
@@ -1078,7 +1093,7 @@ func schemelessPlaintextHint(r *Request, err error) string {
 		return ""
 	}
 
-	hintURL := *r.URL
+	hintURL := *effectiveURL
 	hintURL.Scheme = "http"
 	return core.RedactedURL(&hintURL)
 }

internal/fetch/fetch_test.go

@@ -0,0 +1,54 @@
+package fetch
+
+import (
+	"context"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"github.com/ryanfowler/fetch/internal/core"
+)
+
+func TestFetchCanReuseRequestWithoutAccumulatingQueryParams(t *testing.T) {
+	queries := make(chan string, 2)
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		queries <- r.URL.RawQuery
+		_, _ = io.WriteString(w, "ok")
+	}))
+	defer server.Close()
+
+	u, err := url.Parse(server.URL + "/path?existing=one")
+	if err != nil {
+		t.Fatal(err)
+	}
+	wantURL := u.String()
+	r := &Request{
+		Compression:   core.CompressionOff,
+		Discard:       true,
+		PrinterHandle: core.NewHandle(core.ColorOff),
+		QueryParams:   []core.KeyVal[string]{{Key: "added", Val: "two words"}},
+		URL:           u,
+		Verbosity:     core.VSilent,
+	}
+
+	for range 2 {
+		code, err := fetch(context.Background(), r)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if code != 0 {
+			t.Fatalf("exit code = %d, want 0", code)
+		}
+	}
+
+	for range 2 {
+		if got := <-queries; got != "existing=one&added=two%20words" {
+			t.Fatalf("query = %q", got)
+		}
+	}
+	if got := r.URL.String(); got != wantURL {
+		t.Fatalf("input URL mutated: got %q, want %q", got, wantURL)
+	}
+}

internal/fetch/retry_test.go

@@ -196,6 +196,10 @@ func TestSchemelessPlaintextHint(t *testing.T) {
 	if got := schemelessPlaintextHint(r, connectErr); got != "http://example.com:8080/path?debug=true" {
 		t.Fatalf("hint = %q", got)
 	}
+	schemeErr := &url.Error{Op: "Get", URL: u.String(), Err: http.ErrSchemeMismatch}
+	if got := schemelessPlaintextHint(r, schemeErr); got != "http://example.com:8080/path?debug=true" {
+		t.Fatalf("scheme mismatch hint = %q", got)
+	}
 
 	r.SchemelessURL = false
 	if got := schemelessPlaintextHint(r, connectErr); got != "" {