From 58634e4f66ed8ad83c0a66649fe0197ab4698e76 Mon Sep 17 00:00:00 2001 From: Matthew Vance <92820658+betmix-matt@users.noreply.github.com> Date: Tue, 14 Dec 2021 18:02:36 -0800 Subject: [PATCH] Fixes #407 and #1700 - google.api.http path parameter constraints and multiple HTTP path/method endpoint support (#2461) * Support standard OpenAPI paths for all valid path params for gRPC endpoints in protoc-gen-openapiv2 (#407) * Fix issue with URL encoded path segments not being split when unescaping mode is legacy, default or all characters * Adding docs for path template syntax and new functionality (#1700) * Removed unused old code * Additional tests for regular expressions * Formatting code * Fix lint error * Removing duplicate test * Adding additional test for path escaping * Fix failing test because of path change * Fix failing tests in code coverage * Change determination of unescaping by supporting default mode change * Regenerated files from changes * Remove unused parameters in test * Remove code warnings in test * Implement changes from PR #2461 comments * Correcting incorrect documentation * Adding test case to show change in docs is warranted --- README.md | 17 +- .../internal/clients/abe/api/swagger.yaml | 11 +- .../abe/api_a_bit_of_everything_service.go | 4 +- .../a_bit_of_everything.swagger.json | 25 +- .../internal/genopenapi/template.go | 144 ++++- .../internal/genopenapi/template_test.go | 538 +++++++++++++++++- .../internal/genopenapi/types.go | 1 + runtime/mux.go | 24 +- runtime/mux_test.go | 76 ++- 9 files changed, 779 insertions(+), 61 deletions(-) diff --git a/README.md b/README.md index 7fa417076a1..4175d4716b8 100644 --- a/README.md +++ b/README.md @@ -429,6 +429,19 @@ Alternatively, see the section on remotely managed plugin versions below. Note that this plugin also supports generating OpenAPI definitions for unannotated methods; use the `generate_unbound_methods` option to enable this. + It is possible with the HTTP mapping for a gRPC service method to create duplicate mappings + with the only difference being constraints on the path parameter. + + `/v1/{name=projects/*}` and `/v1/{name=organizations/*}` both become `/v1/{name}`. When + this occurs the plugin will rename the path parameter with a "_1" (or "_2" etc) suffix + to differentiate the different operations. So in the above example, the 2nd path would become + `/v1/{name_1=organizations/*}`. This can also cause OpenAPI clients to URL encode the "/" that is + part of the path parameter as that is what OpenAPI defines in the specification. To allow gRPC gateway to + accept the URL encoded slash and still route the request, use the UnescapingModeAllCharacters or + UnescapingModeLegacy (which is the default currently though may change in future versions). See + [Customizing Your Gateway](https://grpc-ecosystem.github.io/grpc-gateway/docs/mapping/customizing_your_gateway/) + for more information. + ## Usage with remote plugins As an alternative to all of the above, you can use `buf` with @@ -564,13 +577,15 @@ But patches are welcome. - HTTP request host is added as `X-Forwarded-Host` gRPC request header. - HTTP `Authorization` header is added as `authorization` gRPC request header. - Remaining Permanent HTTP header keys (as specified by the IANA - [here](http://www.iana.org/assignments/message-headers/message-headers.xhtml) + [here](http://www.iana.org/assignments/message-headers/message-headers.xhtml)) are prefixed with `grpcgateway-` and added with their values to gRPC request header. - HTTP headers that start with 'Grpc-Metadata-' are mapped to gRPC metadata (prefixed with `grpcgateway-`). - While configurable, the default {un,}marshaling uses [protojson](https://pkg.go.dev/google.golang.org/protobuf/encoding/protojson). +- The path template used to map gRPC service methods to HTTP endpoints supports the [google.api.http](https://github.com/googleapis/googleapis/blob/master/google/api/http.proto) + path template syntax. For example, `/api/v1/{name=projects/*/topics/*}` or `/prefix/{path=organizations/**}`. ## Contribution diff --git a/examples/internal/clients/abe/api/swagger.yaml b/examples/internal/clients/abe/api/swagger.yaml index c9f96ecf811..d32665ec21a 100644 --- a/examples/internal/clients/abe/api/swagger.yaml +++ b/examples/internal/clients/abe/api/swagger.yaml @@ -158,6 +158,7 @@ paths: in: "query" required: false type: "string" + pattern: "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}" x-exportParamName: "Uuid" x-optionalDataType: "String" - name: "floatValue" @@ -470,6 +471,7 @@ paths: in: "query" required: false type: "string" + pattern: "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}" x-exportParamName: "Uuid" x-optionalDataType: "String" - name: "floatValue" @@ -773,6 +775,7 @@ paths: in: "query" required: false type: "string" + pattern: "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}" x-exportParamName: "Uuid" x-optionalDataType: "String" - name: "floatValue" @@ -1445,6 +1448,7 @@ paths: in: "path" required: true type: "string" + pattern: "strprefix/[^/]+" x-exportParamName: "StringValue" - name: "uint32Value" in: "path" @@ -1544,6 +1548,7 @@ paths: in: "query" required: false type: "string" + pattern: "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}" x-exportParamName: "Uuid" x-optionalDataType: "String" - name: "bytesValue" @@ -2076,7 +2081,7 @@ paths: description: "An unexpected error response." schema: $ref: "#/definitions/rpcStatus" - /v1/{book.name=publishers/*/books/*}: + /v1/{book.name}: patch: tags: - "ABitOfEverythingService" @@ -2088,6 +2093,7 @@ paths: \nExample: `publishers/1257894000000000000/books/my-book`" required: true type: "string" + pattern: "publishers/[^/]+/books/[^/]+" x-exportParamName: "BookName" - in: "body" name: "body" @@ -2138,7 +2144,7 @@ paths: description: "An unexpected error response." schema: $ref: "#/definitions/rpcStatus" - /v1/{parent=publishers/*}/books: + /v1/{parent}/books: post: tags: - "ABitOfEverythingService" @@ -2151,6 +2157,7 @@ paths: \nExample: `publishers/1257894000000000000`" required: true type: "string" + pattern: "publishers/[^/]+" x-exportParamName: "Parent" - in: "body" name: "body" diff --git a/examples/internal/clients/abe/api_a_bit_of_everything_service.go b/examples/internal/clients/abe/api_a_bit_of_everything_service.go index 45e6cde0b5c..bda25866910 100644 --- a/examples/internal/clients/abe/api_a_bit_of_everything_service.go +++ b/examples/internal/clients/abe/api_a_bit_of_everything_service.go @@ -1943,7 +1943,7 @@ func (a *ABitOfEverythingServiceApiService) ABitOfEverythingServiceCreateBook(ct ) // create path and map variables - localVarPath := a.client.cfg.BasePath + "/v1/{parent=publishers/*}/books" + localVarPath := a.client.cfg.BasePath + "/v1/{parent}/books" localVarPath = strings.Replace(localVarPath, "{"+"parent"+"}", fmt.Sprintf("%v", parent), -1) localVarHeaderParams := make(map[string]string) @@ -4080,7 +4080,7 @@ func (a *ABitOfEverythingServiceApiService) ABitOfEverythingServiceUpdateBook(ct ) // create path and map variables - localVarPath := a.client.cfg.BasePath + "/v1/{book.name=publishers/*/books/*}" + localVarPath := a.client.cfg.BasePath + "/v1/{book.name}" localVarPath = strings.Replace(localVarPath, "{"+"book.name"+"}", fmt.Sprintf("%v", bookName), -1) localVarHeaderParams := make(map[string]string) diff --git a/examples/internal/proto/examplepb/a_bit_of_everything.swagger.json b/examples/internal/proto/examplepb/a_bit_of_everything.swagger.json index 673736534b5..28a81dff4cd 100644 --- a/examples/internal/proto/examplepb/a_bit_of_everything.swagger.json +++ b/examples/internal/proto/examplepb/a_bit_of_everything.swagger.json @@ -238,7 +238,8 @@ "name": "uuid", "in": "query", "required": false, - "type": "string" + "type": "string", + "pattern": "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}" }, { "name": "floatValue", @@ -579,7 +580,8 @@ "name": "uuid", "in": "query", "required": false, - "type": "string" + "type": "string", + "pattern": "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}" }, { "name": "floatValue", @@ -913,7 +915,8 @@ "name": "uuid", "in": "query", "required": false, - "type": "string" + "type": "string", + "pattern": "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}" }, { "name": "floatValue", @@ -1652,7 +1655,8 @@ "name": "stringValue", "in": "path", "required": true, - "type": "string" + "type": "string", + "pattern": "strprefix/[^/]+" }, { "name": "uint32Value", @@ -1766,7 +1770,8 @@ "name": "uuid", "in": "query", "required": false, - "type": "string" + "type": "string", + "pattern": "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}" }, { "name": "bytesValue", @@ -2615,7 +2620,7 @@ ] } }, - "/v1/{book.name=publishers/*/books/*}": { + "/v1/{book.name}": { "patch": { "operationId": "ABitOfEverythingService_UpdateBook", "responses": { @@ -2661,7 +2666,8 @@ "description": "The resource name of the book.\n\nFormat: `publishers/{publisher}/books/{book}`\n\nExample: `publishers/1257894000000000000/books/my-book`", "in": "path", "required": true, - "type": "string" + "type": "string", + "pattern": "publishers/[^/]+/books/[^/]+" }, { "name": "body", @@ -2692,7 +2698,7 @@ ] } }, - "/v1/{parent=publishers/*}/books": { + "/v1/{parent}/books": { "post": { "summary": "Create a book.", "operationId": "ABitOfEverythingService_CreateBook", @@ -2739,7 +2745,8 @@ "description": "The publisher in which to create the book.\n\nFormat: `publishers/{publisher}`\n\nExample: `publishers/1257894000000000000`", "in": "path", "required": true, - "type": "string" + "type": "string", + "pattern": "publishers/[^/]+" }, { "name": "body", diff --git a/protoc-gen-openapiv2/internal/genopenapi/template.go b/protoc-gen-openapiv2/internal/genopenapi/template.go index b9350b70a3e..54067999309 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template.go @@ -28,6 +28,15 @@ import ( "google.golang.org/protobuf/types/known/structpb" ) +// The OpenAPI specification does not allow for more than one endpoint with the same HTTP method and path. +// This prevents multiple gRPC service methods from sharing the same stripped version of the path and method. +// For example: `GET /v1/{name=organizations/*}/roles` and `GET /v1/{name=users/*}/roles` both get stripped to `GET /v1/{name}/roles`. +// We must make the URL unique by adding a suffix and an incrementing index to each path parameter +// to differentiate the endpoints. +// Since path parameter names do not affect the request contents (i.e. they're replaced in the path) +// this will be hidden from the real grpc gateway consumer. +const pathParamUniqueSuffixDeliminator = "_" + // wktSchemas are the schemas of well-known-types. // The schemas must match with the behavior of the JSON unmarshaler in // https://github.com/protocolbuffers/protobuf-go/blob/v1.25.0/encoding/protojson/well_known_types.go @@ -244,6 +253,7 @@ func nestedQueryParams(message *descriptor.Message, field *descriptor.Field, pre Type: schema.Type, Items: schema.Items, Format: schema.Format, + Pattern: schema.Pattern, Required: required, } if param.Type == "array" { @@ -747,10 +757,11 @@ func resolveFullyQualifiedNameToOpenAPINames(messages []string, namingStrategy s return strategyFn(messages) } -var canRegexp = regexp.MustCompile("{([a-zA-Z][a-zA-Z0-9_.]*).*}") +var canRegexp = regexp.MustCompile("{([a-zA-Z][a-zA-Z0-9_.]*)([^}]*)}") -// OpenAPI expects paths of the form /path/{string_value} but gRPC-Gateway paths are expected to be of the form /path/{string_value=strprefix/*}. This should reformat it correctly. -func templateToOpenAPIPath(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message) string { +// templateToParts will split a URL template as defined by https://github.com/googleapis/googleapis/blob/master/google/api/http.proto +// into a string slice with each part as an element of the slice for use by `partsToOpenAPIPath` and `partsToRegexpMap`. +func templateToParts(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message) []string { // It seems like the right thing to do here is to just use // strings.Split(path, "/") but that breaks badly when you hit a url like // /{my_field=prefix/*}/ and end up with 2 sections representing my_field. @@ -802,28 +813,51 @@ func templateToOpenAPIPath(path string, reg *descriptor.Registry, fields []*desc // Now append the last element to parts parts = append(parts, buffer) - // Parts is now an array of segments of the path. Interestingly, since the - // syntax for this subsection CAN be handled by a regexp since it has no - // memory. + return parts +} + +// partsToOpenAPIPath converts each path part of the form /path/{string_value=strprefix/*} which is defined in +// https://github.com/googleapis/googleapis/blob/master/google/api/http.proto to the OpenAPI expected form /path/{string_value}. +// For example this would replace the path segment of "{foo=bar/*}" with "{foo}" or "prefix{bang=bash/**}" with "prefix{bang}". +// OpenAPI 2 only allows simple path parameters with the constraints on that parameter specified in the OpenAPI +// schema's "pattern" instead of in the path parameter itself. +func partsToOpenAPIPath(parts []string) string { for index, part := range parts { - // If part is a resource name such as "parent", "name", "user.name", the format info must be retained. - prefix := canRegexp.ReplaceAllString(part, "$1") - if isResourceName(prefix) { - continue - } parts[index] = canRegexp.ReplaceAllString(part, "{$1}") } - return strings.Join(parts, "/") } -func isResourceName(prefix string) bool { - words := strings.Split(prefix, ".") - l := len(words) - field := words[l-1] - words = strings.Split(field, ":") - field = words[0] - return field == "parent" || field == "name" +// partsToRegexpMap returns a map of parameter name to ECMA 262 patterns +// which is what the "pattern" field on an OpenAPI parameter expects. +// See https://swagger.io/specification/v2/ (Parameter Object) and +// https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-5.2.3. +// The expression is generated based on expressions defined by https://github.com/googleapis/googleapis/blob/master/google/api/http.proto +// "Path Template Syntax" section which allow for a "param_name=foobar/*/bang/**" style expressions inside +// the path parameter placeholders that indicate constraints on the values of those parameters. +// This function will scan the split parts of a path template for parameters and +// outputs a map of the name of the parameter to a ECMA regular expression. See the http.proto file for descriptions +// of the supported syntax. This function will ignore any path parameters that don't contain a "=" after the +// parameter name. For supported parameters, we assume "*" represent all characters except "/" as it's +// intended to match a single path element and we assume "**" matches any character as it's intended to match multiple +// path elements. +// For example "{name=organizations/*/roles/*}" would produce the regular expression for the "name" parameter of +// "organizations/[^/]+/roles/[^/]+" or "{bar=bing/*/bang/**}" would produce the regular expression for the "bar" +// parameter of "bing/[^/]+/bang/.+". +func partsToRegexpMap(parts []string) map[string]string { + regExps := make(map[string]string) + for _, part := range parts { + if submatch := canRegexp.FindStringSubmatch(part); len(submatch) > 2 { + if strings.HasPrefix(submatch[2], "=") { // this part matches the standard and should be made into a regular expression + // assume the string's characters other than "**" and "*" are literals (not necessarily a good assumption 100% of the times, but it will support most use cases) + regex := submatch[2][1:] + regex = strings.ReplaceAll(regex, "**", ".+") // ** implies any character including "/" + regex = strings.ReplaceAll(regex, "*", "[^/]+") // * implies any character except "/" + regExps[submatch[1]] = regex + } + } + } + return regExps } func renderServiceTags(services []*descriptor.Service) []openapiTagObject { @@ -864,8 +898,13 @@ func renderServices(services []*descriptor.Service, paths openapiPathsObject, re } for methIdx, meth := range svc.Methods { for bIdx, b := range meth.Bindings { + operationFunc := operationForMethod(b.HTTPMethod) // Iterate over all the OpenAPI parameters parameters := openapiParametersObject{} + // split the path template into its parts + parts := templateToParts(b.PathTmpl.Template, reg, meth.RequestType.Fields, msgs) + // extract any constraints specified in the path placeholders into ECMA regular expressions + pathParamRegexpMap := partsToRegexpMap(parts) for _, parameter := range b.PathParams { var paramType, paramFormat, desc, collectionFormat, defaultValue string @@ -936,6 +975,10 @@ func renderServices(services []*descriptor.Service, paths openapiPathsObject, re if reg.GetUseJSONNamesForFields() { parameterString = lowerCamelCase(parameterString, meth.RequestType.Fields, msgs) } + var pattern string + if regExp, ok := pathParamRegexpMap[parameterString]; ok { + pattern = regExp + } parameters = append(parameters, openapiParameterObject{ Name: parameterString, Description: desc, @@ -949,6 +992,7 @@ func renderServices(services []*descriptor.Service, paths openapiPathsObject, re Items: items, CollectionFormat: collectionFormat, MinItems: minItems, + Pattern: pattern, }) } // Now check if there is a body parameter @@ -1020,9 +1064,50 @@ func renderServices(services []*descriptor.Service, paths openapiPathsObject, re } parameters = append(parameters, queryParams...) - pathItemObject, ok := paths[templateToOpenAPIPath(b.PathTmpl.Template, reg, meth.RequestType.Fields, msgs)] + path := partsToOpenAPIPath(parts) + pathItemObject, ok := paths[path] if !ok { pathItemObject = openapiPathItemObject{} + } else { + // handle case where we have an existing mapping for the same path and method + existingOperationObject := operationFunc(&pathItemObject) + if existingOperationObject != nil { + var firstPathParameter *openapiParameterObject + var firstParamIndex int + for index, param := range parameters { + if param.In == "path" { + firstPathParameter = ¶m + firstParamIndex = index + break + } + } + if firstPathParameter == nil { + // Without a path parameter, there is nothing to vary to support multiple mappings of the same path/method. + // Previously this did not log an error and only overwrote the mapping, we now log the error but + // still overwrite the mapping + glog.Errorf("Duplicate mapping for path %s %s", b.HTTPMethod, path) + } else { + newPathCount := 0 + var newPath string + var newPathElement string + // Iterate until there is not an existing operation that matches the same escaped path. + // Most of the time this will only be a single iteration, but a large API could technically have + // a pretty large amount of these if it used similar patterns for all its functions. + for existingOperationObject != nil { + newPathCount += 1 + newPathElement = firstPathParameter.Name + pathParamUniqueSuffixDeliminator + strconv.Itoa(newPathCount) + newPath = strings.ReplaceAll(path, "{"+firstPathParameter.Name+"}", "{"+newPathElement+"}") + if newPathItemObject, ok := paths[newPath]; ok { + existingOperationObject = operationFunc(&newPathItemObject) + } else { + existingOperationObject = nil + } + } + firstPathParameter.Name = newPathElement + path = newPath + parameters[firstParamIndex] = *firstPathParameter + } + } } methProtoPath := protoPathIndex(reflect.TypeOf((*descriptorpb.ServiceDescriptorProto)(nil)), "Method") @@ -1247,7 +1332,7 @@ func renderServices(services []*descriptor.Service, paths openapiPathsObject, re case "PATCH": pathItemObject.Patch = operationObject } - paths[templateToOpenAPIPath(b.PathTmpl.Template, reg, meth.RequestType.Fields, msgs)] = pathItemObject + paths[path] = pathItemObject } } } @@ -1256,6 +1341,23 @@ func renderServices(services []*descriptor.Service, paths openapiPathsObject, re return nil } +func operationForMethod(httpMethod string) func(*openapiPathItemObject) *openapiOperationObject { + switch httpMethod { + case "GET": + return func(obj *openapiPathItemObject) *openapiOperationObject { return obj.Get } + case "POST": + return func(obj *openapiPathItemObject) *openapiOperationObject { return obj.Post } + case "PUT": + return func(obj *openapiPathItemObject) *openapiOperationObject { return obj.Put } + case "DELETE": + return func(obj *openapiPathItemObject) *openapiOperationObject { return obj.Delete } + case "PATCH": + return func(obj *openapiPathItemObject) *openapiOperationObject { return obj.Patch } + default: + return nil + } +} + // This function is called with a param which contains the entire definition of a method. func applyTemplate(p param) (*openapiSwaggerObject, error) { // Create the basic template object. This is the object that everything is diff --git a/protoc-gen-openapiv2/internal/genopenapi/template_test.go b/protoc-gen-openapiv2/internal/genopenapi/template_test.go index a32b13ae4bb..8f655e05269 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template_test.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template_test.go @@ -1,10 +1,13 @@ package genopenapi import ( + "bytes" "encoding/json" "errors" "fmt" + "io" "math" + "os" "reflect" "strings" "testing" @@ -58,6 +61,33 @@ func reqFromFile(f *descriptor.File) *pluginpb.CodeGeneratorRequest { } } +// captureStderr executes the given function and returns the full string of what +// was written to os.Stderr during execution and any error the function may have returned +func captureStderr(f func() error) (string, error) { + old := os.Stderr // keep backup of the real stderr + r, w, err := os.Pipe() + if err != nil { + return "", err + } + os.Stderr = w + + outC := make(chan string) + // copy the output in a separate goroutine so printing can't block indefinitely + go func() { + var buf bytes.Buffer + _, _ = io.Copy(&buf, r) + outC <- buf.String() + }() + + // calling function which stderr we are going to capture: + err = f() + + // back to normal state + _ = w.Close() + os.Stderr = old // restoring the real stderr + return <-outC, err +} + func TestMessageToQueryParametersWithEnumAsInt(t *testing.T) { type test struct { MsgDescs []*descriptorpb.DescriptorProto @@ -198,7 +228,7 @@ func TestMessageToQueryParametersWithEnumAsInt(t *testing.T) { for _, test := range tests { reg := descriptor.NewRegistry() reg.SetEnumsAsInts(true) - msgs := []*descriptor.Message{} + var msgs []*descriptor.Message for _, msgdesc := range test.MsgDescs { msgs = append(msgs, &descriptor.Message{DescriptorProto: msgdesc}) } @@ -1519,7 +1549,7 @@ func TestApplyTemplateHeaders(t *testing.T) { } openapiOperation := openapi_options.Operation{ Responses: map[string]*openapi_options.Response{ - "200": &openapi_options.Response{ + "200": { Description: "Testing Headers", Headers: map[string]*openapi_options.Header{ "string": { @@ -2702,17 +2732,17 @@ func TestApplyTemplateRequestWithBodyQueryParameters(t *testing.T) { return } - if _, ok := result.Paths["/v1/{parent=publishers/*}/books"].Post.Responses["200"]; !ok { - t.Errorf("applyTemplate(%#v).%s = expected 200 response to be defined", tt.args.file, `result.Paths["/v1/{parent=publishers/*}/books"].Post.Responses["200"]`) + if _, ok := result.Paths["/v1/{parent}/books"].Post.Responses["200"]; !ok { + t.Errorf("applyTemplate(%#v).%s = expected 200 response to be defined", tt.args.file, `result.Paths["/v1/{parent}/books"].Post.Responses["200"]`) } else { - if want, got, name := 3, len(result.Paths["/v1/{parent=publishers/*}/books"].Post.Parameters), `len(result.Paths["/v1/{parent=publishers/*}/books"].Post.Parameters)`; !reflect.DeepEqual(got, want) { + if want, got, name := 3, len(result.Paths["/v1/{parent}/books"].Post.Parameters), `len(result.Paths["/v1/{parent}/books"].Post.Parameters)`; !reflect.DeepEqual(got, want) { t.Errorf("applyTemplate(%#v).%s = %d want to be %d", tt.args.file, name, got, want) } for i, want := range tt.want { - p := result.Paths["/v1/{parent=publishers/*}/books"].Post.Parameters[i] - if got, name := (paramOut{p.Name, p.In, p.Required}), `result.Paths["/v1/{parent=publishers/*}/books"].Post.Parameters[0]`; !reflect.DeepEqual(got, want) { + p := result.Paths["/v1/{parent}/books"].Post.Parameters[i] + if got, name := (paramOut{p.Name, p.In, p.Required}), `result.Paths["/v1/{parent}/books"].Post.Parameters[0]`; !reflect.DeepEqual(got, want) { t.Errorf("applyTemplate(%#v).%s = %v want to be %v", tt.args.file, name, got, want) } } @@ -3045,16 +3075,16 @@ func TestTemplateToOpenAPIPath(t *testing.T) { {"/{test=prefix/that/has/multiple/parts/to/it/*}", "/{test}"}, {"/{test1}/{test2}", "/{test1}/{test2}"}, {"/{test1}/{test2}/", "/{test1}/{test2}/"}, - {"/{name=prefix/*}", "/{name=prefix/*}"}, - {"/{name=prefix1/*/prefix2/*}", "/{name=prefix1/*/prefix2/*}"}, - {"/{user.name=prefix/*}", "/{user.name=prefix/*}"}, - {"/{user.name=prefix1/*/prefix2/*}", "/{user.name=prefix1/*/prefix2/*}"}, - {"/{parent=prefix/*}/children", "/{parent=prefix/*}/children"}, - {"/{name=prefix/*}:customMethod", "/{name=prefix/*}:customMethod"}, - {"/{name=prefix1/*/prefix2/*}:customMethod", "/{name=prefix1/*/prefix2/*}:customMethod"}, - {"/{user.name=prefix/*}:customMethod", "/{user.name=prefix/*}:customMethod"}, - {"/{user.name=prefix1/*/prefix2/*}:customMethod", "/{user.name=prefix1/*/prefix2/*}:customMethod"}, - {"/{parent=prefix/*}/children:customMethod", "/{parent=prefix/*}/children:customMethod"}, + {"/{name=prefix/*}", "/{name}"}, + {"/{name=prefix1/*/prefix2/*}", "/{name}"}, + {"/{user.name=prefix/*}", "/{user.name}"}, + {"/{user.name=prefix1/*/prefix2/*}", "/{user.name}"}, + {"/{parent=prefix/*}/children", "/{parent}/children"}, + {"/{name=prefix/*}:customMethod", "/{name}:customMethod"}, + {"/{name=prefix1/*/prefix2/*}:customMethod", "/{name}:customMethod"}, + {"/{user.name=prefix/*}:customMethod", "/{user.name}:customMethod"}, + {"/{user.name=prefix1/*/prefix2/*}:customMethod", "/{user.name}:customMethod"}, + {"/{parent=prefix/*}/children:customMethod", "/{parent}/children:customMethod"}, } reg := descriptor.NewRegistry() reg.SetUseJSONNamesForFields(false) @@ -3178,6 +3208,38 @@ func TestResolveFullyQualifiedNameToOpenAPIName(t *testing.T) { } } +func templateToOpenAPIPath(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message) string { + return partsToOpenAPIPath(templateToParts(path, reg, fields, msgs)) +} + +func templateToRegexpMap(path string, reg *descriptor.Registry, fields []*descriptor.Field, msgs []*descriptor.Message) map[string]string { + return partsToRegexpMap(templateToParts(path, reg, fields, msgs)) +} + +func TestFQMNToRegexpMap(t *testing.T) { + var tests = []struct { + input string + expected map[string]string + }{ + {"/test", map[string]string{}}, + {"/{test}", map[string]string{}}, + {"/{test" + pathParamUniqueSuffixDeliminator + "1=prefix/*}", map[string]string{"test" + pathParamUniqueSuffixDeliminator + "1": "prefix/[^/]+"}}, + {"/{test=prefix/that/has/multiple/parts/to/it/**}", map[string]string{"test": "prefix/that/has/multiple/parts/to/it/.+"}}, + {"/{test1=organizations/*}/{test2=divisions/*}", map[string]string{ + "test1": "organizations/[^/]+", + "test2": "divisions/[^/]+", + }}, + {"/v1/{name=projects/*/topics/*}:delete", map[string]string{"name": "projects/[^/]+/topics/[^/]+"}}, + } + reg := descriptor.NewRegistry() + for _, data := range tests { + actual := templateToRegexpMap(data.input, reg, generateFieldsForJSONReservedName(), generateMsgsForJSONReservedName()) + if !reflect.DeepEqual(data.expected, actual) { + t.Errorf("Expected partsToRegexpMap(%v) = %v, actual: %v", data.input, data.expected, actual) + } + } +} + func TestFQMNtoOpenAPIName(t *testing.T) { var tests = []struct { input string @@ -3189,6 +3251,7 @@ func TestFQMNtoOpenAPIName(t *testing.T) { {"/{test=prefix/that/has/multiple/parts/to/it/*}", "/{test}"}, {"/{test1}/{test2}", "/{test1}/{test2}"}, {"/{test1}/{test2}/", "/{test1}/{test2}/"}, + {"/v1/{name=tests/*}/tests", "/v1/{name}/tests"}, } reg := descriptor.NewRegistry() reg.SetUseJSONNamesForFields(false) @@ -4779,6 +4842,447 @@ func TestTemplateWithoutErrorDefinition(t *testing.T) { } } +func TestTemplateWithInvalidDuplicateOperations(t *testing.T) { + msgdesc := &descriptorpb.DescriptorProto{ + Name: proto.String("ExampleMessage"), + Field: []*descriptorpb.FieldDescriptorProto{}, + } + meth1 := &descriptorpb.MethodDescriptorProto{ + Name: proto.String("Method1"), + InputType: proto.String("ExampleMessage"), + OutputType: proto.String("ExampleMessage"), + } + meth2 := &descriptorpb.MethodDescriptorProto{ + Name: proto.String("Method2"), + InputType: proto.String("ExampleMessage"), + OutputType: proto.String("ExampleMessage"), + } + svc1 := &descriptorpb.ServiceDescriptorProto{ + Name: proto.String("Service1"), + Method: []*descriptorpb.MethodDescriptorProto{meth1, meth2}, + } + + msg := &descriptor.Message{ + DescriptorProto: msgdesc, + } + + file := descriptor.File{ + FileDescriptorProto: &descriptorpb.FileDescriptorProto{ + SourceCodeInfo: &descriptorpb.SourceCodeInfo{}, + Name: proto.String("service1.proto"), + Package: proto.String("example"), + MessageType: []*descriptorpb.DescriptorProto{msgdesc}, + Service: []*descriptorpb.ServiceDescriptorProto{svc1}, + Options: &descriptorpb.FileOptions{ + GoPackage: proto.String("github.com/grpc-ecosystem/grpc-gateway/runtime/internal/examplepb;example"), + }, + }, + GoPkg: descriptor.GoPackage{ + Path: "example.com/path/to/example/example.pb", + Name: "example_pb", + }, + Messages: []*descriptor.Message{msg}, + Services: []*descriptor.Service{ + { + ServiceDescriptorProto: svc1, + Methods: []*descriptor.Method{ + { + MethodDescriptorProto: meth1, + RequestType: msg, + ResponseType: msg, + Bindings: []*descriptor.Binding{ + { + HTTPMethod: "GET", + PathTmpl: httprule.Template{ + Version: 1, + OpCodes: []int{0, 0}, + Template: "/v1/{name=organizations/*}/roles", + }, + Body: &descriptor.Body{ + FieldPath: descriptor.FieldPath([]descriptor.FieldPathComponent{}), + }, + }, + }, + }, + { + MethodDescriptorProto: meth2, + RequestType: msg, + ResponseType: msg, + Bindings: []*descriptor.Binding{ + { + HTTPMethod: "GET", + PathTmpl: httprule.Template{ + Version: 1, + OpCodes: []int{0, 0}, + Template: "/v1/{name=users/*}/roles", + }, + Body: &descriptor.Body{ + FieldPath: descriptor.FieldPath([]descriptor.FieldPathComponent{}), + }, + }, + }, + }, + }, + }, + }, + } + reg := descriptor.NewRegistry() + err := reg.Load(&pluginpb.CodeGeneratorRequest{ProtoFile: []*descriptorpb.FileDescriptorProto{file.FileDescriptorProto}}) + if err != nil { + t.Errorf("failed to reg.Load(): %v", err) + return + } + if stdErr, err := captureStderr(func() error { + swagger, err := applyTemplate(param{File: crossLinkFixture(&file), reg: reg}) + if opid := swagger.Paths["/v1/{name}/roles"].Get.OperationID; opid != "Service1_Method2" { + t.Errorf("Swagger should use last in wins for the service methods: %s", opid) + } + return err + }); !strings.Contains(stdErr, "Duplicate mapping for path GET /v1/{name}/roles") { + t.Errorf("Error for duplicate mapping was incorrect: %s", stdErr) + } else if err != nil { + t.Error(err) + } +} + +func TestTemplateWithDuplicateHttp1Operations(t *testing.T) { + fieldType := descriptorpb.FieldDescriptorProto_TYPE_STRING + field1 := &descriptorpb.FieldDescriptorProto{ + Name: proto.String("name"), + Number: proto.Int32(1), + Type: &fieldType, + } + field2 := &descriptorpb.FieldDescriptorProto{ + Name: proto.String("role"), + Number: proto.Int32(2), + Type: &fieldType, + } + msgdesc := &descriptorpb.DescriptorProto{ + Name: proto.String("ExampleMessage"), + Field: []*descriptorpb.FieldDescriptorProto{ + field1, + field2, + }, + } + meth1 := &descriptorpb.MethodDescriptorProto{ + Name: proto.String("Method1"), + InputType: proto.String("ExampleMessage"), + OutputType: proto.String("ExampleMessage"), + } + meth2 := &descriptorpb.MethodDescriptorProto{ + Name: proto.String("Method2"), + InputType: proto.String("ExampleMessage"), + OutputType: proto.String("ExampleMessage"), + } + svc1 := &descriptorpb.ServiceDescriptorProto{ + Name: proto.String("Service1"), + Method: []*descriptorpb.MethodDescriptorProto{meth1, meth2}, + } + meth3 := &descriptorpb.MethodDescriptorProto{ + Name: proto.String("Method3"), + InputType: proto.String("ExampleMessage"), + OutputType: proto.String("ExampleMessage"), + } + meth4 := &descriptorpb.MethodDescriptorProto{ + Name: proto.String("Method4"), + InputType: proto.String("ExampleMessage"), + OutputType: proto.String("ExampleMessage"), + } + svc2 := &descriptorpb.ServiceDescriptorProto{ + Name: proto.String("Service2"), + Method: []*descriptorpb.MethodDescriptorProto{meth3, meth4}, + } + msg := &descriptor.Message{ + DescriptorProto: msgdesc, + } + + file := descriptor.File{ + FileDescriptorProto: &descriptorpb.FileDescriptorProto{ + SourceCodeInfo: &descriptorpb.SourceCodeInfo{}, + Name: proto.String("service1.proto"), + Package: proto.String("example"), + MessageType: []*descriptorpb.DescriptorProto{msgdesc}, + Service: []*descriptorpb.ServiceDescriptorProto{svc1, svc2}, + Options: &descriptorpb.FileOptions{ + GoPackage: proto.String("github.com/grpc-ecosystem/grpc-gateway/runtime/internal/examplepb;example"), + }, + }, + GoPkg: descriptor.GoPackage{ + Path: "example.com/path/to/example/example.pb", + Name: "example_pb", + }, + Messages: []*descriptor.Message{msg}, + Services: []*descriptor.Service{ + { + ServiceDescriptorProto: svc1, + Methods: []*descriptor.Method{ + { + MethodDescriptorProto: meth1, + RequestType: msg, + ResponseType: msg, + Bindings: []*descriptor.Binding{ + { + HTTPMethod: "GET", + PathTmpl: httprule.Template{ + Version: 1, + OpCodes: []int{0, 0}, + Template: "/v1/{name=organizations/*}/{role=roles/*}", + }, + PathParams: []descriptor.Parameter{ + { + Target: &descriptor.Field{ + FieldDescriptorProto: field1, + Message: msg, + }, + FieldPath: descriptor.FieldPath{ + { + Name: "name", + }, + }, + }, + { + Target: &descriptor.Field{ + FieldDescriptorProto: field2, + Message: msg, + }, + FieldPath: descriptor.FieldPath{ + { + Name: "role", + }, + }, + }, + }, + Body: &descriptor.Body{ + FieldPath: descriptor.FieldPath([]descriptor.FieldPathComponent{}), + }, + }, + }, + }, + { + MethodDescriptorProto: meth2, + RequestType: msg, + ResponseType: msg, + Bindings: []*descriptor.Binding{ + { + HTTPMethod: "GET", + PathTmpl: httprule.Template{ + Version: 1, + OpCodes: []int{0, 0}, + Template: "/v1/{name=users/*}/{role=roles/*}", + }, + PathParams: []descriptor.Parameter{ + { + Target: &descriptor.Field{ + FieldDescriptorProto: field1, + Message: msg, + }, + FieldPath: descriptor.FieldPath{ + { + Name: "name", + }, + }, + }, + { + Target: &descriptor.Field{ + FieldDescriptorProto: field2, + Message: msg, + }, + FieldPath: descriptor.FieldPath{ + { + Name: "role", + }, + }, + }, + }, + Body: &descriptor.Body{ + FieldPath: descriptor.FieldPath([]descriptor.FieldPathComponent{}), + }, + }, + }, + }, + }, + }, + { + ServiceDescriptorProto: svc2, + Methods: []*descriptor.Method{ + { + MethodDescriptorProto: meth3, + RequestType: msg, + ResponseType: msg, + Bindings: []*descriptor.Binding{ + { + HTTPMethod: "GET", + PathTmpl: httprule.Template{ + Version: 1, + OpCodes: []int{0, 0}, + Template: "/v1/{name=users/*}/roles", + }, + PathParams: []descriptor.Parameter{ + { + Target: &descriptor.Field{ + FieldDescriptorProto: field1, + Message: msg, + }, + FieldPath: descriptor.FieldPath{ + { + Name: "name", + }, + }, + }, + }, + Body: &descriptor.Body{ + FieldPath: descriptor.FieldPath([]descriptor.FieldPathComponent{}), + }, + }, + }, + }, + { + MethodDescriptorProto: meth4, + RequestType: msg, + ResponseType: msg, + Bindings: []*descriptor.Binding{ + { + HTTPMethod: "GET", + PathTmpl: httprule.Template{ + Version: 1, + OpCodes: []int{0, 0}, + Template: "/v1/{name=groups/*}/{role=roles/*}", + }, + PathParams: []descriptor.Parameter{ + { + Target: &descriptor.Field{ + FieldDescriptorProto: field1, + Message: msg, + }, + FieldPath: descriptor.FieldPath{ + { + Name: "name", + }, + }, + }, + { + Target: &descriptor.Field{ + FieldDescriptorProto: field2, + Message: msg, + }, + FieldPath: descriptor.FieldPath{ + { + Name: "role", + }, + }, + }, + }, + Body: &descriptor.Body{ + FieldPath: descriptor.FieldPath([]descriptor.FieldPathComponent{}), + }, + }, + }, + }, + }, + }, + }, + } + reg := descriptor.NewRegistry() + err := reg.Load(&pluginpb.CodeGeneratorRequest{ProtoFile: []*descriptorpb.FileDescriptorProto{file.FileDescriptorProto}}) + if err != nil { + t.Fatalf("failed to reg.Load(): %v", err) + } + result, err := applyTemplate(param{File: crossLinkFixture(&file), reg: reg}) + if err != nil { + t.Fatalf("applyTemplate(%#v) failed with %v; want success", file, err) + } + + if got, want := len(result.Paths), 4; got != want { + t.Fatalf("Results path length differed, got %d want %d", got, want) + } + + firstOp := result.Paths["/v1/{name}/{role}"].Get + if got, want := firstOp.OperationID, "Service1_Method1"; got != want { + t.Fatalf("First operation id differed, got %s want %s", got, want) + } + if got, want := len(firstOp.Parameters), 3; got != want { + t.Fatalf("First operation params length differed, got %d want %d", got, want) + } + if got, want := firstOp.Parameters[0].Name, "name"; got != want { + t.Fatalf("First operation first param name differed, got %s want %s", got, want) + } + if got, want := firstOp.Parameters[0].Pattern, "organizations/[^/]+"; got != want { + t.Fatalf("First operation first param pattern differed, got %s want %s", got, want) + } + if got, want := firstOp.Parameters[1].Name, "role"; got != want { + t.Fatalf("First operation second param name differed, got %s want %s", got, want) + } + if got, want := firstOp.Parameters[1].Pattern, "roles/[^/]+"; got != want { + t.Fatalf("First operation second param pattern differed, got %s want %s", got, want) + } + if got, want := firstOp.Parameters[2].In, "body"; got != want { + t.Fatalf("First operation third param 'in' differed, got %s want %s", got, want) + } + + secondOp := result.Paths["/v1/{name"+pathParamUniqueSuffixDeliminator+"1}/{role}"].Get + if got, want := secondOp.OperationID, "Service1_Method2"; got != want { + t.Fatalf("Second operation id differed, got %s want %s", got, want) + } + if got, want := len(secondOp.Parameters), 3; got != want { + t.Fatalf("Second operation params length differed, got %d want %d", got, want) + } + if got, want := secondOp.Parameters[0].Name, "name"+pathParamUniqueSuffixDeliminator+"1"; got != want { + t.Fatalf("Second operation first param name differed, got %s want %s", got, want) + } + if got, want := secondOp.Parameters[0].Pattern, "users/[^/]+"; got != want { + t.Fatalf("Second operation first param pattern differed, got %s want %s", got, want) + } + if got, want := secondOp.Parameters[1].Name, "role"; got != want { + t.Fatalf("Second operation second param name differed, got %s want %s", got, want) + } + if got, want := secondOp.Parameters[1].Pattern, "roles/[^/]+"; got != want { + t.Fatalf("Second operation second param pattern differed, got %s want %s", got, want) + } + if got, want := secondOp.Parameters[2].In, "body"; got != want { + t.Fatalf("Second operation third param 'in' differed, got %s want %s", got, want) + } + + thirdOp := result.Paths["/v1/{name}/roles"].Get + if got, want := thirdOp.OperationID, "Service2_Method3"; got != want { + t.Fatalf("Third operation id differed, got %s want %s", got, want) + } + if got, want := len(thirdOp.Parameters), 2; got != want { + t.Fatalf("Third operation params length differed, got %d want %d", got, want) + } + if got, want := thirdOp.Parameters[0].Name, "name"; got != want { + t.Fatalf("Third operation first param name differed, got %s want %s", got, want) + } + if got, want := thirdOp.Parameters[0].Pattern, "users/[^/]+"; got != want { + t.Fatalf("Third operation first param pattern differed, got %s want %s", got, want) + } + if got, want := thirdOp.Parameters[1].In, "body"; got != want { + t.Fatalf("Third operation second param 'in' differed, got %s want %s", got, want) + } + + forthOp := result.Paths["/v1/{name"+pathParamUniqueSuffixDeliminator+"2}/{role}"].Get + if got, want := forthOp.OperationID, "Service2_Method4"; got != want { + t.Fatalf("Fourth operation id differed, got %s want %s", got, want) + } + if got, want := len(forthOp.Parameters), 3; got != want { + t.Fatalf("Fourth operation params length differed, got %d want %d", got, want) + } + if got, want := forthOp.Parameters[0].Name, "name"+pathParamUniqueSuffixDeliminator+"2"; got != want { + t.Fatalf("Fourth operation first param name differed, got %s want %s", got, want) + } + if got, want := forthOp.Parameters[0].Pattern, "groups/[^/]+"; got != want { + t.Fatalf("Fourth operation first param pattern differed, got %s want %s", got, want) + } + if got, want := forthOp.Parameters[1].Name, "role"; got != want { + t.Fatalf("Fourth operation second param name differed, got %s want %s", got, want) + } + if got, want := forthOp.Parameters[1].Pattern, "roles/[^/]+"; got != want { + t.Fatalf("Fourth operation second param pattern differed, got %s want %s", got, want) + } + if got, want := forthOp.Parameters[2].In, "body"; got != want { + t.Fatalf("Fourth operation second param 'in' differed, got %s want %s", got, want) + } +} + func Test_getReservedJsonName(t *testing.T) { type args struct { fieldName string diff --git a/protoc-gen-openapiv2/internal/genopenapi/types.go b/protoc-gen-openapiv2/internal/genopenapi/types.go index 861c4c73338..2769053e615 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/types.go +++ b/protoc-gen-openapiv2/internal/genopenapi/types.go @@ -143,6 +143,7 @@ type openapiParameterObject struct { CollectionFormat string `json:"collectionFormat,omitempty"` Default string `json:"default,omitempty"` MinItems *int `json:"minItems,omitempty"` + Pattern string `json:"pattern,omitempty"` // Or you can explicitly refer to another type. If this is defined all // other fields should be empty diff --git a/runtime/mux.go b/runtime/mux.go index f2698776f20..e7c75dc8763 100644 --- a/runtime/mux.go +++ b/runtime/mux.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "net/textproto" + "regexp" "strings" "github.com/grpc-ecosystem/grpc-gateway/v2/internal/httprule" @@ -24,15 +25,15 @@ const ( // path string before doing any routing. UnescapingModeLegacy UnescapingMode = iota - // EscapingTypeExceptReserved unescapes all path parameters except RFC 6570 + // UnescapingModeAllExceptReserved unescapes all path parameters except RFC 6570 // reserved characters. UnescapingModeAllExceptReserved - // EscapingTypeExceptSlash unescapes URL path parameters except path - // seperators, which will be left as "%2F". + // UnescapingModeAllExceptSlash unescapes URL path parameters except path + // separators, which will be left as "%2F". UnescapingModeAllExceptSlash - // URL path parameters will be fully decoded. + // UnescapingModeAllCharacters unescapes all URL path parameters. UnescapingModeAllCharacters // UnescapingModeDefault is the default escaping type. @@ -41,6 +42,10 @@ const ( UnescapingModeDefault = UnescapingModeLegacy ) +var ( + encodedPathSplitter = regexp.MustCompile("(/|%2F)") +) + // A HandlerFunc handles a specific pair of path pattern and HTTP method. type HandlerFunc func(w http.ResponseWriter, r *http.Request, pathParams map[string]string) @@ -265,7 +270,16 @@ func (s *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) { path = r.URL.RawPath } - components := strings.Split(path[1:], "/") + var components []string + // since in UnescapeModeLegacy, the URL will already have been fully unescaped, if we also split on "%2F" + // in this escaping mode we would be double unescaping but in UnescapingModeAllCharacters, we still do as the + // path is the RawPath (i.e. unescaped). That does mean that the behavior of this function will change its default + // behavior when the UnescapingModeDefault gets changed from UnescapingModeLegacy to UnescapingModeAllExceptReserved + if s.unescapingMode == UnescapingModeAllCharacters { + components = encodedPathSplitter.Split(path[1:], -1) + } else { + components = strings.Split(path[1:], "/") + } if override := r.Header.Get("X-HTTP-Method-Override"); override != "" && s.isPathLengthFallback(r) { r.Method = strings.ToUpper(override) diff --git a/runtime/mux_test.go b/runtime/mux_test.go index a1b75588143..0bdc72c2e23 100644 --- a/runtime/mux_test.go +++ b/runtime/mux_test.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "net/url" "strconv" "testing" @@ -369,6 +370,48 @@ func TestMuxServeHTTP(t *testing.T) { unescapingMode: runtime.UnescapingModeAllExceptReserved, respContent: "GET /foo/{id=*}/bar", }, + { + patterns: []stubPattern{ + { + method: "GET", + ops: []int{ + int(utilities.OpLitPush), 0, + int(utilities.OpPush), 0, + int(utilities.OpConcatN), 1, + int(utilities.OpCapture), 1, + int(utilities.OpLitPush), 2}, + pool: []string{"foo", "id", "bar"}, + }, + }, + reqMethod: "GET", + reqPath: "/foo/success%2fwith%2Fspace/bar", + headers: map[string]string{ + "Content-Type": "application/json", + }, + respStatus: http.StatusNotFound, + unescapingMode: runtime.UnescapingModeAllCharacters, + }, + { + patterns: []stubPattern{ + { + method: "GET", + ops: []int{ + int(utilities.OpLitPush), 0, + int(utilities.OpPush), 0, + int(utilities.OpConcatN), 1, + int(utilities.OpCapture), 1, + int(utilities.OpLitPush), 2}, + pool: []string{"foo", "id", "bar"}, + }, + }, + reqMethod: "GET", + reqPath: "/foo/success%2fwith%2Fspace/bar", + headers: map[string]string{ + "Content-Type": "application/json", + }, + respStatus: http.StatusNotFound, + unescapingMode: runtime.UnescapingModeLegacy, + }, { patterns: []stubPattern{ { @@ -391,6 +434,31 @@ func TestMuxServeHTTP(t *testing.T) { unescapingMode: runtime.UnescapingModeAllExceptReserved, respContent: "GET /foo/{id=**}", }, + { + patterns: []stubPattern{ + { + method: "POST", + ops: []int{ + int(utilities.OpLitPush), 0, + int(utilities.OpLitPush), 1, + int(utilities.OpLitPush), 2, + int(utilities.OpPush), 0, + int(utilities.OpConcatN), 2, + int(utilities.OpCapture), 3, + }, + pool: []string{"api", "v1", "organizations", "name"}, + verb: "action", + }, + }, + reqMethod: "POST", + reqPath: "/api/v1/" + url.QueryEscape("organizations/foo") + ":action", + headers: map[string]string{ + "Content-Type": "application/json", + }, + respStatus: http.StatusOK, + unescapingMode: runtime.UnescapingModeAllCharacters, + respContent: "POST /api/v1/{name=organizations/*}:action", + }, } { t.Run(strconv.Itoa(i), func(t *testing.T) { var opts []runtime.ServeMuxOption @@ -408,15 +476,15 @@ func TestMuxServeHTTP(t *testing.T) { t.Fatalf("runtime.NewPattern(1, %#v, %#v, %q) failed with %v; want success", p.ops, p.pool, p.verb, err) } 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()) + _, _ = fmt.Fprintf(w, "%s %s", p.method, pat.String()) }) }(p) } - url := fmt.Sprintf("http://host.example%s", spec.reqPath) - r, err := http.NewRequest(spec.reqMethod, url, bytes.NewReader(nil)) + reqUrl := fmt.Sprintf("https://host.example%s", spec.reqPath) + r, err := http.NewRequest(spec.reqMethod, reqUrl, bytes.NewReader(nil)) if err != nil { - t.Fatalf("http.NewRequest(%q, %q, nil) failed with %v; want success", spec.reqMethod, url, err) + t.Fatalf("http.NewRequest(%q, %q, nil) failed with %v; want success", spec.reqMethod, reqUrl, err) } for name, value := range spec.headers { r.Header.Set(name, value)