Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

escaped parameters fail to generate the correct url path when a base path is present. #1083

Closed
james-lawrence opened this issue Jun 29, 2017 · 1 comment · Fixed by #3029
Closed
Labels
bug generate client pending PR runtime Relates to github.com/go-openapi/runtime

Comments

@james-lawrence
Copy link

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, d507f49
go version: 1.8.1
OS: linux

@casualjim casualjim added the bug label Jun 30, 2017
@fredbi fredbi added the runtime Relates to github.com/go-openapi/runtime label Jun 21, 2019
@fredbi
Copy link
Contributor

fredbi commented Dec 27, 2023

Verified today the status of this long standing issue... :)

OK so this works just fine.
c.ResourceMethod((&ResourceMethodParameters{}).WithParam("param/value")) will automatically escape the parameter and I could not see any issue with routing.
On the server side, the param is properly unescaped.

Preparing a PR that adds this interesting test to exercise the full rountrip with all runtime dependencies.

fredbi added a commit to fredbi/go-swagger that referenced this issue Dec 27, 2023
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 go-swagger#1083 (asserts was already working)

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
fredbi added a commit to fredbi/go-swagger that referenced this issue Dec 27, 2023
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 go-swagger#1083 (asserts was already working)

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
fredbi added a commit that referenced this issue Dec 28, 2023
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)

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug generate client pending PR runtime Relates to github.com/go-openapi/runtime
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants