Path parameters can't have URL encoded values

[poy]

I've added a test to mux_test.go to demonstrate:

diff --git a/runtime/mux_test.go b/runtime/mux_test.go
index bb90a73..1e4698f 100644
--- a/runtime/mux_test.go
+++ b/runtime/mux_test.go
@@ -27,6 +27,7 @@ func TestMuxServeHTTP(t *testing.T) {

                respStatus  int
                respContent string
+               pathParams  map[string]string
        }{
                {
                        patterns:   nil,
@@ -176,6 +177,28 @@ func TestMuxServeHTTP(t *testing.T) {
                        respStatus:  http.StatusOK,
                        respContent: "POST /foo:bar",
                },
+               {
+                       patterns: []stubPattern{
+                               {
+                                       method: "GET",
+                                       ops: []int{
+                                               int(utilities.OpLitPush), 0,
+                                               int(utilities.OpLitPush), 1,
+                                               int(utilities.OpPush), 0,
+                                               int(utilities.OpConcatN), 1,
+                                               int(utilities.OpCapture), 2,
+                                       },
+                                       pool: []string{"v1", "bucket", "name"},
+                               },
+                       },
+                       reqMethod:   "GET",
+                       reqPath:     "/v1/bucket/my%2Fname",
+                       respStatus:  http.StatusOK,
+                       respContent: "GET /v1/bucket/{name=*}",
+                       pathParams: map[string]string{
+                               "name": "my%2Fname",
+                       },
+               },
        } {
                mux := runtime.NewServeMux()
                for _, p := range spec.patterns {
@@ -186,7 +209,15 @@ func TestMuxServeHTTP(t *testing.T) {
                                }
                                mux.Handle(p.method, pat, func(w http.ResponseWriter, r *http.Request, pathParams map[string]string) {
                                        fmt.Fprintf(w, "%s %s", p.method, pat.String())
+
+                                       // Assert agains the expected path parameters
+                                       for name, value := range spec.pathParams {
+                                               if pathParams[name] != value {
+                                                       t.Fatalf("expected pathParam %q to have value %q instead of %q", name, value, pathParams[name])
+                                               }
+                                       }
                                })
+
                        }(p)
                }

[poy]

We resolved our requirement via /v1/read/{source_id=**}

[scudette]

I am running into this issue but the suggested solution does not work for me. Is there a workaround?

[scudette]

Ok after looking into this more I think it is WAI although I am not sure that I understand what its supposed to do. It seems that if a parameter has a ":" in it (in the escaped URL), then the parameter is not passed directly to the grpc call but there is some kind of processing to extract the verb. So I am trying to map something like:

/v1/method/{param}

and the URL is /v1/method/F%3A1234

which does not work. A suitable workaround seems to be to add an extra : to the end of the URL:

/v1/method/F%3A1234%3A

this seems like a hack to me though since the parameter is properly URL encoded. If the proxy can not handle proper URL path mappings (with the proper escaping) then am I just safer to pass parameters in query strings?

[vgheri]

I am also running into this issue and it's pretty troublesome.
Would it be possible to re-open the issue and discuss if it can be fixed instead of using workarounds (that are impractical) ?

[johanbrandhorst]

I'll reopen the issue since this seems to still be a problem. I don't know of any workarounds myself, and I'm not sure how this could possibly be solved without something like a wildcard in the router which isn't supported by the standard library router. Do you have any ideas?

[Tommy-42]

Hi,

Basically :

where foo:bar:id is litteraly a string for example an alias constructed with 3 differents pieces separated by : will fail :

curl http://localhost:8080/v1/test/alias/foo:bar:id
404 Not Found

but if you do as said above :

curl http://localhost:8080/v1/test/alias/foo:bar:id:
// works as intended
will work fine.

I added some logs to start understanding how it works :

// line 165
// ...
components := strings.Split(path[1:], "/")
l := len(components)

log.Printf("COMP 1: %+v", components) // HERE

var verb string
if idx := strings.LastIndex(components[l-1], ":"); idx == 0 {
    if s.protoErrorHandler != nil {
        _, outboundMarshaler := MarshalerForRequest(s, r)
        sterr := status.Error(codes.Unimplemented, http.StatusText(http.StatusNotImplemented))
        s.protoErrorHandler(ctx, s, outboundMarshaler, w, r, sterr)
    } else {
        OtherErrorHandler(w, r, http.StatusText(http.StatusNotFound), http.StatusNotFound)
    }
    return
} else if idx > 0 {
    c := components[l-1]
    components[l-1], verb = c[:idx], c[idx+1:]
    log.Printf("COMP 2: %+v -- VERB: %s", components, verb) // HERE
}

it prints

2018/08/20 14:49:36 COMP 1: [v1 test alias foo:bar:test]
2018/08/20 14:49:36 COMP 2: [v1 test alias foo:bar] -- VERB: test

I don't know how it works, so i am posting what i've found so far, If it can helps

[johanbrandhorst]

Nice digging @Tommy-42, this will absolutely help others who may be wanting to test the same thing! If I find the time I might look at this but no promises I'm afraid.

[Tommy-42]

I found this issue, that is basically what we talked about

#224

cc @johanbrandhorst

[johanbrandhorst]

Seems like that issue was closed with 7226a0d. Is this still a problem or did that fix this as well?

[Tommy-42]

I've done my test on the 1.4.1, which was released on may 23 if I am no wrong. but this commit was merged on 2nd august.

but the tricky part is I am using golang/protobuf which enforce the version of grpc-gateway which constrain me from setting up the master branch ( and may not be compatible too ).

do you have a solution ?

[johanbrandhorst]

How do you mean using golang/protobuf enforces a specific version of grpc-gateway? master of grpc-gateway does use version 1.1 of golang/protobuf, but I don't know about any inverse requirement?

[Tommy-42]

my bad, I did say something wrong.

it is even more messed up ... :(

I've

# have to follow grpc-middleware
# see : https://github.com/grpc-ecosystem/go-grpc-middleware/blob/master/Gopkg.toml#L6
[[constraint]]
  branch = "master"
  name = "github.com/golang/protobuf"

because go-grpc-middleware constraint master on golang/protobuf ...

but grpc-gateway enforce golang/protobuf to 1.1

I am kinda stuck there.

[johanbrandhorst]

@Tommy-42 you can use an override to force a specific version:

[[override]]
    name = "github.com/golang/protobuf"
    version = "1.1.0"

https://golang.github.io/dep/docs/Gopkg.toml.html#dependency-rules-constraint-and-override

[Tommy-42]

Oh my ... huge thanks, it works great now !

[Tommy-42]

[johanbrandhorst]

Can you confirm this is fixed in master then so we can close this issue?

[Tommy-42]

yes, it is working with master branch, I am able to request foo:bar:joe or foo:bar:joe:

thanks for the help !

[johanbrandhorst]

Great, thanks for confirming!