Repository: go-swagger/go-swagger (go)

Issue:
escaped parameters fail to generate the correct url path when a base path is present.
If a path parameter needs to be escaped the runtime code generates an invalid path.

BasePath = "/foo"
Path = "/v1/{param}"

I suggest adding a transport to the HTTP client that dumps the request.
```
import openapiclient "github.com/go-openapi/runtime/client"
c := generatedClient.New(
		openapiclient.NewWithClient("localhost", "/foo", []string{"http"}, http.DefaultClient),
		nil,
)

// all of the following fail.
// passing in a raw string will invoke an invalid path: /foo/v1/param/value
c.ResourceMethod((&ResourceMethodParameters{}).WithParam("param/value"))
// passing in an escaped string will invoke an invalid path: /foo/v1/param/value
c.ResourceMethod((&ResourceMethodParameters{}).WithParam(url.PathEscape("param/value")))
```
These fail because of the runtime/client's interaction with URL.
The http client uses URL.EscapedPath() to determine what path to use.
```
func (u *URL) EscapedPath() string {
	if u.RawPath != "" && validEncodedPath(u.RawPath) {
		p, err := unescape(u.RawPath, encodePath)
		if err == nil && p == u.Path {
			return u.RawPath
		}
	}
	if u.Path == "*" {
		return "*" // don't escape (Issue 11202)
	}
	return escape(u.Path, encodePath)
}
```

client.Runtime.Submit has the following code:
```
func (r *Runtime) Submit(operation *runtime.ClientOperation) (interface{}, error) {
        // ...
	if req.URL.Path != "" && req.URL.Path != "/" && req.URL.Path[len(req.URL.Path)-1] == '/' {
		reinstateSlash = true
	}
	req.URL.Path = path.Join(r.BasePath, req.URL.Path)
	if reinstateSlash {
		req.URL.Path = req.URL.Path + "/"
	}
        // ...
}
```

setting `req.URL.Path = path.Join(r.BasePath, req.URL.Path)` causes url.EscapedPath() to return
req.URL.Path unescaped, since escape(u.Path, encodePath) only escapes the ?, it assumes the fragments have already been properly escaped.

the fix is to properly set both the RawPath and the Path in the submit function.
```
	req.URL.Path = path.Join(r.BasePath, req.URL.Path)
        req.URL.RawPath = path.Join(r.BasePath, req.URL.RawPath)
```

I also believe that `func (r *request) BuildHTTP(...) (*http.Request, error) {` should properly escape the path parameters, but that breaks current behaviour.
```
	// create http request
	path := r.pathPattern
	for k, v := range r.pathParams {
		log.Println("replacing", "{"+k+"}", "with", v, "in", path)
		path = strings.Replace(path, "{"+k+"}", url.PathEscape(v), -1)
	}
```

## Environment
swagger version: master, d507f49829bea7a9c1cc313e80de65bae4a9a6bc 
go version: 1.8.1
OS: linux


Pull request:
test(client): added dynamic test to assert path params
Added a test that constructs a generated client, dynamic untyped server and exercise the stack (in this case, asserting how an escaped path parameter moves across the stack from runtime client to runtime server).

* fixes #1083 (asserts was already working)

Patch:
diff --git a/fixtures/bugs/1083/petstore.yaml b/fixtures/bugs/1083/petstore.yaml
new file mode 100644
index 00000000..432f5317
--- /dev/null
+++ b/fixtures/bugs/1083/petstore.yaml
@@ -0,0 +1,76 @@
+schemes:
+    - http
+swagger: "2.0"
+info:
+    contact:
+        name: wordnik api team
+        url: http://developer.wordnik.com
+    license:
+        name: Creative Commons 4.0 International
+        url: http://creativecommons.org/licenses/by/4.0/
+    title: Swagger Petstore
+    version: 1.0.0
+host: petstore.swagger.wordnik.com
+basePath: /api
+paths:
+    /pets:
+        get:
+            responses:
+                "200":
+                    description: pet response
+                    headers:
+                        x-expires:
+                            type: string
+                    schema:
+                        items:
+                            $ref: '#/definitions/Pet'
+                        type: array
+                default:
+                    description: unexpected error
+                    schema:
+                        $ref: '#/definitions/Error'
+            summary: finds pets in the system
+            tags:
+                - Pet Operations
+    /pets/{id}:
+        get:
+            parameters:
+            - name: id
+              in: path
+              required: true
+              type: string
+            responses:
+                "200":
+                    description: pet response
+                    schema:
+                        $ref: '#/definitions/Pet'
+                default:
+                    description: unexpected error
+                    schema:
+                        $ref: '#/definitions/Error'
+            tags:
+                - Pet Operations
+
+definitions:
+    Error:
+        properties:
+            code:
+                format: int32
+                type: integer
+            message:
+                type: string
+        required:
+            - code
+            - message
+    Pet:
+        properties:
+            id:
+                format: int64
+                type: integer
+            name:
+                type: string
+            tag:
+                type: string
+        required:
+            - id
+            - name


Test patch:
diff --git a/fixtures/bugs/1083/pathparam_test.go b/fixtures/bugs/1083/pathparam_test.go
new file mode 100644
index 00000000..89610bcd
--- /dev/null
+++ b/fixtures/bugs/1083/pathparam_test.go
@@ -0,0 +1,102 @@
+//go:build ignore
+
+package main
+
+// this test is designed to be run dynamically by go-swagger test suite
+// (generator.generate_test.go), after a test client has been generated.
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"github.com/go-openapi/errors"
+	"github.com/go-openapi/loads"
+	"github.com/go-openapi/runtime"
+	rtclient "github.com/go-openapi/runtime/client"
+	"github.com/go-openapi/runtime/middleware"
+	"github.com/go-openapi/runtime/middleware/untyped"
+	"github.com/go-openapi/swag"
+	"github.com/go-swagger/go-swagger/fixtures/bugs/1083/codegen/client"
+	"github.com/go-swagger/go-swagger/fixtures/bugs/1083/codegen/client/pet_operations"
+	"github.com/go-swagger/go-swagger/fixtures/bugs/1083/codegen/models"
+	"github.com/stretchr/testify/require"
+)
+
+type operationHandler struct{}
+
+func (h *operationHandler) Handle(in interface{}) (interface{}, error) {
+	// this handler sends back the input path parameter in Name
+
+	paramsMap, ok := in.(map[string]interface{})
+	if !ok {
+		return nil, errors.New(http.StatusInternalServerError, "unexpected params: wants a map")
+	}
+	val, ok := paramsMap["id"]
+	if !ok {
+		return nil, errors.New(http.StatusBadRequest, "unexpected params: {id} required")
+	}
+	id, ok := val.(string)
+	if !ok {
+		return nil, errors.New(http.StatusBadRequest, "unexpected params: {id} should be a string")
+	}
+
+	return models.Pet{
+		ID:   swag.Int64(1),
+		Name: swag.String(id),
+		Tag:  "test",
+	}, nil
+}
+
+func TestEscapedPathParam(t *testing.T) {
+	// prepare an untyped test server
+	buildTestServer := func(t testing.TB) (string, func()) {
+		t.Helper()
+		spec, err := loads.Spec("petstore.yaml")
+		require.NoError(t, err)
+
+		api := untyped.NewAPI(spec)
+		api.RegisterConsumer("application/json", runtime.JSONConsumer())
+		api.RegisterProducer("application/json", runtime.JSONProducer())
+		api.RegisterOperation("GET", "/pets/{id}", &operationHandler{})
+		handler := middleware.Serve(spec, api)
+
+		server := httptest.NewServer(handler)
+		u, err := url.Parse(server.URL)
+		require.NoError(t, err)
+
+		return u.Host, server.Close
+	}
+
+	buildTestClient := func(t testing.TB, host string) *client.Issue1083 {
+		t.Helper()
+
+		c := client.Default
+		tr := rtclient.New(host, "/api", []string{"http"})
+		tr.Debug = true
+		c.SetTransport(tr)
+
+		return c
+	}
+
+	submitAndAssert := func(c *client.Issue1083, pathParam string) func(*testing.T) {
+		return func(t *testing.T) {
+			params := pet_operations.NewGetPetsIDParams().WithID(pathParam)
+			resp, err := c.PetOperations.GetPetsID(params)
+			require.NoError(t, err)
+			require.NotNil(t, resp)
+			require.Equal(t, int64(1), swag.Int64Value(resp.Payload.ID))
+			require.Equal(t, pathParam, swag.StringValue(resp.Payload.Name))
+		}
+	}
+
+	host, clean := buildTestServer(t)
+	t.Cleanup(clean)
+
+	c := buildTestClient(t, host)
+
+	t.Run("should route with unescaped path param", submitAndAssert(c, "part"))
+	t.Run("should route with escaped path param (1)", submitAndAssert(c, "part/ext"))
+	t.Run("should route with escaped path param (2)", submitAndAssert(c, "part#ext"))
+}
diff --git a/generator/generate_test.go b/generator/generate_test.go
index 2f613d82..cfae79b2 100644
--- a/generator/generate_test.go
+++ b/generator/generate_test.go
@@ -20,9 +20,9 @@ func TestGenerateAndTest(t *testing.T) {
 
 	cwd := testCwd(t)
 	const root = "generated"
-	defer func() {
+	t.Cleanup(func() {
 		_ = os.RemoveAll(filepath.Join(cwd, root))
-	}()
+	})
 
 	t.Run("server build", func(t *testing.T) {
 		for name, cas := range generateFixtures(t) {
@@ -67,6 +67,50 @@ func TestGenerateAndTest(t *testing.T) {
 			})
 		}
 	})
+
+	t.Run("client build", func(t *testing.T) {
+		for name, cas := range generateClientFixtures(t) {
+			thisCas := cas
+			thisName := name
+
+			t.Run(thisName, func(t *testing.T) {
+				t.Parallel()
+
+				log.SetOutput(io.Discard)
+				defer thisCas.warnFailed(t)
+
+				// default opts
+				opts := testClientGenOpts()
+
+				// create directory layout, defer clean
+				defer thisCas.prepareTarget(t, thisName, "server_test", root, opts)()
+
+				// preparation before generation
+				if thisCas.prepare != nil {
+					thisCas.prepare(t, opts)
+				}
+
+				t.Logf("generating test client at: %s, from %s", opts.Target, opts.Spec)
+
+				err := GenerateClient(thisName, nil, nil, opts)
+				if thisCas.wantError {
+					require.Errorf(t, err, "expected an error for client build fixture: %s", opts.Spec)
+				} else {
+					require.NoError(t, err, "unexpected error for client build fixture: %s", opts.Spec)
+				}
+
+				// verify
+				if thisCas.verify != nil {
+					thisCas.verify(t, opts.Target)
+				}
+
+				// fixture-specific clean
+				if thisCas.clean != nil {
+					thisCas.clean()
+				}
+			})
+		}
+	})
 }
 
 type generateFixture struct {
@@ -128,7 +172,7 @@ func generateFixtures(t testing.TB) map[string]generateFixture {
 		"issue 1943": {
 			spec:   "../fixtures/bugs/1943/fixture-1943.yaml",
 			target: "../fixtures/bugs/1943",
-			prepare: func(_ testing.TB, opts *GenOpts) {
+			prepare: func(t testing.TB, opts *GenOpts) {
 				input, err := os.ReadFile("../fixtures/bugs/1943/datarace_test.go")
 				require.NoError(t, err)
 
@@ -490,6 +534,57 @@ func generateFixtures(t testing.TB) map[string]generateFixture {
 	}
 }
 
+func generateClientFixtures(_ testing.TB) map[string]generateFixture {
+	return map[string]generateFixture{
+		"issue1083": {
+			spec:   "../fixtures/bugs/1083/petstore.yaml",
+			target: "../fixtures/bugs/1083/codegen",
+			prepare: func(t testing.TB, opts *GenOpts) {
+				input, err := os.ReadFile("../fixtures/bugs/1083/pathparam_test.go")
+				require.NoError(t, err)
+
+				// rewrite imports for the relocated test program
+				cwd := testCwd(t)
+				rebased := bytes.ReplaceAll(
+					input,
+					[]byte("/fixtures/bugs/1083/codegen"),
+					[]byte(filepath.ToSlash(strings.TrimPrefix(opts.Target, filepath.Dir(cwd)))),
+				)
+
+				require.NoError(t, os.WriteFile(filepath.Join(filepath.Dir(opts.Target), "pathparam_test.go"), rebased, 0o600))
+				opts.ExcludeSpec = false
+
+				// copy spec to run untyped server
+				f, err := os.Open(filepath.Join("..", "fixtures", "bugs", "1083", "petstore.yaml"))
+				require.NoError(t, err)
+				defer func() {
+					_ = f.Close()
+				}()
+				w, err := os.OpenFile(filepath.Join(filepath.Dir(opts.Target), "petstore.yaml"), os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0o700)
+				require.NoError(t, err)
+				defer func() {
+					_ = w.Close()
+				}()
+				_, err = io.Copy(w, f)
+				require.NoError(t, err)
+			},
+			verify: func(t testing.TB, target string) {
+				const packages = "./..."
+				testPrg := "pathparam_test.go"
+				testDir := filepath.Dir(target)
+
+				goExecInDir(t, testDir, "get", packages)
+
+				t.Log("running runtime request test on generated client")
+				// This test runs a generated client against a untyped API server.
+				// It verifies that path parameters are properly escaped and unescaped.
+				// It exercises the full stack of runtime client and server.
+				goExecInDir(t, testDir, "test", "-v", testPrg)
+			},
+		},
+	}
+}
+
 func addModelsToLocation(t testing.TB, location, file string) {
 	// writes some external model to a file to supplement codegen
 	// (test external types)