fix(runtime): fix path parsing to use URL.EscapedPath() instead of Pa…

…th and unescape individual components

Basically we want to route for instance http://example.com/resources/some%2Fpath/something from this http annotation :

    get: /resources/{key}/something

We want the  key field in gRPC to be able to contain 0 or many slashes

grpc-ecosystem#660

go.mod

@@ -8,6 +8,7 @@ require (
 	github.com/golang/glog v0.0.0-20210429001901-424d2337a529
 	github.com/golang/protobuf v1.5.2
 	github.com/google/go-cmp v0.5.6
+	github.com/grpc-ecosystem/grpc-gateway v1.16.0
 	github.com/rogpeppe/fastuuid v1.2.0
 	golang.org/x/oauth2 v0.0.0-20210628180205-a41e5a781914
 	google.golang.org/genproto v0.0.0-20210728212813-7823e685a01f

runtime/mux.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/textproto"
+	"net/url"
 	"strings"
 
 	"github.com/grpc-ecosystem/grpc-gateway/v2/internal/httprule"
@@ -197,14 +198,48 @@ func (s *ServeMux) HandlePath(meth string, pathPattern string, h HandlerFunc) er
 func (s *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 
-	path := r.URL.Path
+	path := r.URL.EscapedPath()
 	if !strings.HasPrefix(path, "/") {
 		_, outboundMarshaler := MarshalerForRequest(s, r)
 		s.routingErrorHandler(ctx, s, outboundMarshaler, w, r, http.StatusBadRequest)
 		return
 	}
 
-	components := strings.Split(path[1:], "/")
+	pathParts := strings.Split(path[1:], "/")
+	components := make([]string, len(pathParts))
+	for i, comp := range pathParts {
+		comp, err := url.PathUnescape(comp)
+		if err != nil {
+			if s.errorHandler != nil {
+				_, outboundMarshaler := MarshalerForRequest(s, r)
+				sterr := status.Error(codes.InvalidArgument, err.Error())
+				s.errorHandler(ctx, s, outboundMarshaler, w, r, sterr)
+			} else {
+				_, outboundMarshaler := MarshalerForRequest(s, r)
+				s.routingErrorHandler(ctx, s, outboundMarshaler, w, r, http.StatusBadRequest)
+			}
+			return
+		}
+		components[i] = comp
+	}
+	l := len(components)
+
+	// Verb out here is to memoize for the fallback case below
+	var verb string
+
+	if idx := strings.LastIndex(components[l-1], ":"); idx == 0 {
+		if s.errorHandler != nil {
+			_, outboundMarshaler := MarshalerForRequest(s, r)
+			s.errorHandler(ctx, s, outboundMarshaler, w, r, fmt.Errorf("unknown URI"))
+		} else {
+			_, outboundMarshaler := MarshalerForRequest(s, r)
+			s.routingErrorHandler(ctx, s, outboundMarshaler,  w, r, http.StatusNotFound)
+		}
+		return
+	} else if idx > 0 {
+		c := components[l-1]
+		components[l-1], verb = c[:idx], c[idx+1:]
+	}
 
 	if override := r.Header.Get("X-HTTP-Method-Override"); override != "" && s.isPathLengthFallback(r) {
 		r.Method = strings.ToUpper(override)
@@ -216,9 +251,6 @@ func (s *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	// Verb out here is to memoize for the fallback case below
-	var verb string
-
 	for _, h := range s.handlers[r.Method] {
 		// If the pattern has a verb, explicitly look for a suffix in the last
 		// component that matches a colon plus the verb. This allows us to

runtime/mux_test.go

@@ -5,9 +5,11 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"reflect"
 	"strconv"
 	"testing"
 
+	"github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway/httprule"
 	"github.com/grpc-ecosystem/grpc-gateway/v2/runtime"
 	"github.com/grpc-ecosystem/grpc-gateway/v2/utilities"
 )
@@ -466,3 +468,67 @@ func TestServeMux_HandlePath(t *testing.T) {
 		})
 	}
 }
+
+// TestSlashEncoding ensures that slashes are supported as part of path when encoded
+func TestSlashEncoding(t *testing.T) {
+	for _, spec := range []struct {
+		method     string
+		template   string
+		reqPath    string
+		respStatus int
+		pathParams map[string]string
+	}{
+		{
+			method:     "GET",
+			template:   "/v1/users/{name}/profile",
+			reqPath:    "/v1/users/some%2Fuser/profile",
+			respStatus: http.StatusOK,
+			pathParams: map[string]string{"name": "some/user"},
+		},
+		{
+			method:     "GET",
+			template:   "/v1/users/{name}",
+			reqPath:    "/v1/users/some%2Fuser",
+			respStatus: http.StatusOK,
+			pathParams: map[string]string{"name": "some/user"},
+		},
+		{
+			method:     "GET",
+			template:   "/v1/users/{name}/profile",
+			reqPath:    "/v1/users/some/user/profile",
+			respStatus: http.StatusNotFound,
+		},
+		{
+			method:     "GET",
+			template:   "/v1/users/{name}",
+			reqPath:    "/v1/users/some/user",
+			respStatus: http.StatusNotFound,
+		},
+	} {
+		mux := runtime.NewServeMux()
+		compiled, err := httprule.Parse(spec.template)
+		if err != nil {
+			t.Fatalf("httprule.Parse(%s) failed with %v; want success", spec.template, err)
+		}
+		template := compiled.Compile()
+		pat, err := runtime.NewPattern(1, template.OpCodes, template.Pool, template.Verb)
+		if err != nil {
+			t.Fatalf("runtime.NewPattern(1, %#v, %#v, %q) failed with %v; want success", template.OpCodes, template.Pool, template.Verb, err)
+		}
+		mux.Handle(spec.method, pat, func(w http.ResponseWriter, r *http.Request, pathParams map[string]string) {
+			if !reflect.DeepEqual(pathParams, spec.pathParams) {
+				t.Errorf("pathParams got: %v, want: %v", pathParams, spec.pathParams)
+			}
+		})
+		url := fmt.Sprintf("http://host.example%s", spec.reqPath)
+		r, err := http.NewRequest(spec.method, url, bytes.NewReader(nil))
+		if err != nil {
+			t.Fatalf("http.NewRequest(%q, %q, nil) failed with %v; want success", spec.method, url, err)
+		}
+		w := httptest.NewRecorder()
+		mux.ServeHTTP(w, r)
+		if got, want := w.Code, spec.respStatus; got != want {
+			t.Errorf("w.Code = %d; want %d; req=%v", got, want, r)
+		}
+	}
+}