[object Object]

← back to Cli Printing Press

fix(cli): percent-encode path param values in replacePathParam (#1366)

77fce43f42bf32c3c9d286ed69b506d93b3635da · 2026-05-13 21:15:30 -0700 · Trevin Chow

* fix(cli): percent-encode path param values in replacePathParam

Generated helpers.go emitted a bare strings.ReplaceAll for {placeholder}
substitution, so any path-param value containing a path-reserved character
(/, :, ?, #, space, %) silently produced a malformed request URL. Concrete
hit cited in #1192: Search Console's siteUrl / sitemap feedpath and GitHub
Contents API's {path} 404 against the live API because slashes and colons
were not encoded.

Route the value through url.PathEscape. Opaque IDs and dashed/dotted values
are byte-identical (the no-op set covers [A-Za-z0-9-._~]); slashes encode
to %2F so the URL stays in a single segment. Widen the net/url import gate
in helpers.go.tmpl so HasPathParams alone is sufficient.

Spec-driven full encoding (format: uri -> url.QueryEscape) for the Google
Search Console family is out of scope for this PR and tracked separately.

Closes #1192

* test(cli): scope path-escape behavior cases as named subtests

Greptile P2: a flat for-loop attributed failures only by message; t.Run
gives each case its own name so a regression reports the failing input
directly in the test runner and -run can isolate it.

Files touched

Diff

commit 77fce43f42bf32c3c9d286ed69b506d93b3635da
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Wed May 13 21:15:30 2026 -0700

    fix(cli): percent-encode path param values in replacePathParam (#1366)
    
    * fix(cli): percent-encode path param values in replacePathParam
    
    Generated helpers.go emitted a bare strings.ReplaceAll for {placeholder}
    substitution, so any path-param value containing a path-reserved character
    (/, :, ?, #, space, %) silently produced a malformed request URL. Concrete
    hit cited in #1192: Search Console's siteUrl / sitemap feedpath and GitHub
    Contents API's {path} 404 against the live API because slashes and colons
    were not encoded.
    
    Route the value through url.PathEscape. Opaque IDs and dashed/dotted values
    are byte-identical (the no-op set covers [A-Za-z0-9-._~]); slashes encode
    to %2F so the URL stays in a single segment. Widen the net/url import gate
    in helpers.go.tmpl so HasPathParams alone is sufficient.
    
    Spec-driven full encoding (format: uri -> url.QueryEscape) for the Google
    Search Console family is out of scope for this PR and tracked separately.
    
    Closes #1192
    
    * test(cli): scope path-escape behavior cases as named subtests
    
    Greptile P2: a flat for-loop attributed failures only by message; t.Run
    gives each case its own name so a regression reports the failing input
    directly in the test runner and -run can isolate it.
---
 internal/generator/path_param_encoding_test.go     | 93 ++++++++++++++++++++++
 internal/generator/templates/helpers.go.tmpl       |  6 +-
 .../embedded-paged-api/internal/cli/helpers.go     |  5 +-
 .../printing-press-golden/internal/cli/helpers.go  |  6 +-
 4 files changed, 106 insertions(+), 4 deletions(-)

diff --git a/internal/generator/path_param_encoding_test.go b/internal/generator/path_param_encoding_test.go
new file mode 100644
index 00000000..9db89ca0
--- /dev/null
+++ b/internal/generator/path_param_encoding_test.go
@@ -0,0 +1,93 @@
+package generator
+
+import (
+	"net/url"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/mvanhorn/cli-printing-press/v4/internal/naming"
+	"github.com/mvanhorn/cli-printing-press/v4/internal/spec"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestReplacePathParamPercentEncodesValue pins the helpers.go template so the
+// emitted replacePathParam routes the user-supplied value through
+// url.PathEscape before substituting it into the URL path. Without the escape,
+// values that contain path-reserved characters (e.g. "src/main.go" for the
+// GitHub Contents API or "https://example.com/" for Search Console's siteUrl)
+// silently produce malformed request URLs that 404 against the live API.
+//
+// We assert at the template-output level (helpers.go contains the escape call
+// and imports net/url) so the contract is checked once and every printed CLI
+// inherits the fix.
+func TestReplacePathParamPercentEncodesValue(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := &spec.APISpec{
+		Name:    "encpath",
+		Version: "0.1.0",
+		BaseURL: "https://api.example.com",
+		Auth:    spec.AuthConfig{Type: "none"},
+		Config: spec.ConfigSpec{
+			Format: "toml",
+			Path:   "~/.config/encpath-pp-cli/config.toml",
+		},
+		Resources: map[string]spec.Resource{
+			"items": {
+				Description: "Items",
+				Endpoints: map[string]spec.Endpoint{
+					"get": {
+						Method:      "GET",
+						Path:        "/v1/items/{itemId}",
+						Description: "Get an item",
+						Params: []spec.Param{
+							{Name: "itemId", Type: "string", Required: true, Positional: true},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), naming.CLI(apiSpec.Name))
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	helpersPath := filepath.Join(outputDir, "internal", "cli", "helpers.go")
+	helpersGo, err := os.ReadFile(helpersPath)
+	require.NoError(t, err)
+	src := string(helpersGo)
+
+	assert.Contains(t, src, `"net/url"`,
+		"helpers.go must import net/url when replacePathParam is emitted")
+	assert.Contains(t, src,
+		`return strings.ReplaceAll(path, "{"+name+"}", url.PathEscape(value))`,
+		"replacePathParam must percent-encode the value via url.PathEscape so "+
+			"path-reserved characters (/, :, ?, #, space, %) don't produce a malformed URL")
+}
+
+// TestURLPathEscapeBehaviorPinsContract is a stdlib-behavior pin: if Go ever
+// changed url.PathEscape's encoding shape, every printed CLI's request URLs
+// would change too. We document the inputs the issue cited (opaque IDs round-
+// trip; slashes/colons encode) so a future stdlib regression is caught here
+// rather than in a live-API 404.
+func TestURLPathEscapeBehaviorPinsContract(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		in, want string
+	}{
+		{"abc-123-def", "abc-123-def"},
+		{"2026-01-15", "2026-01-15"},
+		{"src/cli/main.go", "src%2Fcli%2Fmain.go"},
+		{"https://example.com/", "https:%2F%2Fexample.com%2F"},
+		{"sc-domain:example.com", "sc-domain:example.com"},
+	}
+	for _, c := range cases {
+		t.Run(c.in, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, c.want, url.PathEscape(c.in))
+		})
+	}
+}
diff --git a/internal/generator/templates/helpers.go.tmpl b/internal/generator/templates/helpers.go.tmpl
index fcc1e2bd..7d58d0d8 100644
--- a/internal/generator/templates/helpers.go.tmpl
+++ b/internal/generator/templates/helpers.go.tmpl
@@ -12,7 +12,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
-{{- if .HasEmbeddedPaged}}
+{{- if or .HasEmbeddedPaged .HasPathParams}}
 	"net/url"
 {{- end}}
 	"os"
@@ -578,8 +578,10 @@ func newTabWriter(w io.Writer) *tabwriter.Writer {
 }
 
 {{- if .HasPathParams}}
+// replacePathParam percent-encodes value so path-reserved characters in
+// user input do not collapse into extra path segments.
 func replacePathParam(path, name, value string) string {
-	return strings.ReplaceAll(path, "{"+name+"}", value)
+	return strings.ReplaceAll(path, "{"+name+"}", url.PathEscape(value))
 }
 {{- end}}
 
diff --git a/testdata/golden/expected/generate-embedded-paged-api/embedded-paged-api/internal/cli/helpers.go b/testdata/golden/expected/generate-embedded-paged-api/embedded-paged-api/internal/cli/helpers.go
index bfc80e0b..146efdb7 100644
--- a/testdata/golden/expected/generate-embedded-paged-api/embedded-paged-api/internal/cli/helpers.go
+++ b/testdata/golden/expected/generate-embedded-paged-api/embedded-paged-api/internal/cli/helpers.go
@@ -380,8 +380,11 @@ func truncate(s string, max int) string {
 func newTabWriter(w io.Writer) *tabwriter.Writer {
 	return tabwriter.NewWriter(w, 2, 4, 2, ' ', 0)
 }
+
+// replacePathParam percent-encodes value so path-reserved characters in
+// user input do not collapse into extra path segments.
 func replacePathParam(path, name, value string) string {
-	return strings.ReplaceAll(path, "{"+name+"}", value)
+	return strings.ReplaceAll(path, "{"+name+"}", url.PathEscape(value))
 }
 
 // paginatedGet fetches pages and concatenates array results. The headers
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/helpers.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/helpers.go
index e9f50563..feba1e1e 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/helpers.go
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/helpers.go
@@ -11,6 +11,7 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"io"
+	"net/url"
 	"os"
 	"path/filepath"
 	"printing-press-golden-pp-cli/internal/client"
@@ -379,8 +380,11 @@ func truncate(s string, max int) string {
 func newTabWriter(w io.Writer) *tabwriter.Writer {
 	return tabwriter.NewWriter(w, 2, 4, 2, ' ', 0)
 }
+
+// replacePathParam percent-encodes value so path-reserved characters in
+// user input do not collapse into extra path segments.
 func replacePathParam(path, name, value string) string {
-	return strings.ReplaceAll(path, "{"+name+"}", value)
+	return strings.ReplaceAll(path, "{"+name+"}", url.PathEscape(value))
 }
 
 // paginatedGet fetches pages and concatenates array results. The headers

← 1fc5f62b fix(cli): drop MCP handler pre-marshal that base64-encoded m  ·  back to Cli Printing Press  ·  fix(cli): detect cross-cutting novel features in dogfood sco a6010384 →