From 9c8cc1a45559165130993bcc5d8e69163129473c Mon Sep 17 00:00:00 2001 From: Chris Rodwell Date: Tue, 4 May 2021 10:53:14 +0000 Subject: [PATCH 1/5] bug? --- broken_test.go | 148 ++++++++++++++++++++++++++++++ routers/legacy/testdata/spec.yaml | 96 +++++++++++++++++++ 2 files changed, 244 insertions(+) create mode 100644 broken_test.go create mode 100644 routers/legacy/testdata/spec.yaml diff --git a/broken_test.go b/broken_test.go new file mode 100644 index 000000000..5dcecffd5 --- /dev/null +++ b/broken_test.go @@ -0,0 +1,148 @@ +package main + +import ( + "context" + "net/http" + "strings" + "testing" + + "github.com/getkin/kin-openapi/openapi3" + "github.com/getkin/kin-openapi/openapi3filter" + "github.com/getkin/kin-openapi/routers/legacy" + "github.com/stretchr/testify/require" +) + +func TestValidateRequest(t *testing.T) { + spec := `openapi: 3.0.0 +info: + title: Example + version: '1.0' + contact: + name: Chris Rodwell + email: crodwell@github.com + description: test + termsOfService: test +servers: + - url: 'http://localhost:3000' + description: API +paths: + /test: + post: + summary: Create + tags: + - test + responses: + '201': + description: Created + content: + application/json: + schema: + type: object + properties: + name: + type: string + parent_id: + type: integer + start_date: + type: string + end_date: + type: string + total_budget: + type: string + currency_code: + type: string + zone_name: + type: string + operationId: post-test + requestBody: + content: + application/json: + schema: + type: object + properties: + name: + type: string + minLength: 1 + maxLength: 256 + parent_id: + type: integer + format: int32 + start_date: + type: string + format: date-time + end_date: + type: string + format: date-time + total_budget: + type: number + format: float + minimum: 1 + maximum: 99999999999999.98 + currency_code: + type: string + maxLength: 3 + zone_name: + type: string + required: + - name + - parent_id + - start_date + - end_date + - total_budget + - currency_code + - zone_name + examples: + Example: + value: + parent_id: 190 + name: valid test item + start_date: '2050-01-01T10:00:00Z' + end_date: '2050-01-10T23:59:00Z' + currency_code: USD + total_budget: 100 + zone_name: America/New_York + description: '' + description: Create a test object +components: + schemas: {} +tags: + - name: test + description: A test item + +` + + loader := &openapi3.Loader{Context: context.Background()} + doc, err := loader.LoadFromData([]byte(spec)) + require.NoError(t, err) + err = doc.Validate(context.Background()) + require.NoError(t, err) + router, err := legacy.NewRouter(doc) + require.NoError(t, err) + + jsonBody := `{ + "parent_id": 190, + "name": "valid test item", + "start_date": "abc", + "end_date": "2050-01-01T10:00:00Z", + "currency_code": "USD", + "total_budget": 100, + "zone_name": "America/New_York" + }` + + httpReq, err := http.NewRequest(http.MethodPost, "/test", strings.NewReader(jsonBody)) + require.NoError(t, err) + + // Find route + route, pathParams, err := router.FindRoute(httpReq) + require.NoError(t, err) // this fails with No Operation + + // Validate request + requestValidationInput := &openapi3filter.RequestValidationInput{ + Request: httpReq, + PathParams: pathParams, + Route: route, + } + err = openapi3filter.ValidateRequest(context.Background(), requestValidationInput) + require.NoError(t, err) + +} diff --git a/routers/legacy/testdata/spec.yaml b/routers/legacy/testdata/spec.yaml new file mode 100644 index 000000000..516cbdc13 --- /dev/null +++ b/routers/legacy/testdata/spec.yaml @@ -0,0 +1,96 @@ +openapi: 3.0.0 +info: + title: Example + version: '1.0' + contact: + name: Chris Rodwell + email: crodwell@github.com + description: test + termsOfService: test +servers: + - url: 'http://localhost:3000' + description: API +paths: + /test: + post: + summary: Create + tags: + - test + responses: + '201': + description: Created + content: + application/json: + schema: + type: object + properties: + name: + type: string + parent_id: + type: integer + start_date: + type: string + end_date: + type: string + total_budget: + type: string + currency_code: + type: string + zone_name: + type: string + operationId: post-test + requestBody: + content: + application/json: + schema: + type: object + properties: + name: + type: string + minLength: 1 + maxLength: 256 + parent_id: + type: integer + format: int32 + start_date: + type: string + format: date-time + end_date: + type: string + format: date-time + total_budget: + type: number + format: float + minimum: 1 + maximum: 99999999999999.98 + currency_code: + type: string + maxLength: 3 + zone_name: + type: string + required: + - name + - parent_id + - start_date + - end_date + - total_budget + - currency_code + - zone_name + examples: + Example: + value: + parent_id: 190 + name: valid test item + start_date: '2050-01-01T10:00:00Z' + end_date: '2050-01-10T23:59:00Z' + currency_code: USD + total_budget: 100 + zone_name: America/New_York + description: '' + description: Create a test object +components: + schemas: {} +tags: + - name: test + description: A test item + From 13abf50479c4c1c58817cdab69b3ff58b7efe507 Mon Sep 17 00:00:00 2001 From: Chris Rodwell Date: Tue, 4 May 2021 10:53:41 +0000 Subject: [PATCH 2/5] bug? --- routers/legacy/testdata/spec.yaml | 96 ------------------------------- 1 file changed, 96 deletions(-) delete mode 100644 routers/legacy/testdata/spec.yaml diff --git a/routers/legacy/testdata/spec.yaml b/routers/legacy/testdata/spec.yaml deleted file mode 100644 index 516cbdc13..000000000 --- a/routers/legacy/testdata/spec.yaml +++ /dev/null @@ -1,96 +0,0 @@ -openapi: 3.0.0 -info: - title: Example - version: '1.0' - contact: - name: Chris Rodwell - email: crodwell@github.com - description: test - termsOfService: test -servers: - - url: 'http://localhost:3000' - description: API -paths: - /test: - post: - summary: Create - tags: - - test - responses: - '201': - description: Created - content: - application/json: - schema: - type: object - properties: - name: - type: string - parent_id: - type: integer - start_date: - type: string - end_date: - type: string - total_budget: - type: string - currency_code: - type: string - zone_name: - type: string - operationId: post-test - requestBody: - content: - application/json: - schema: - type: object - properties: - name: - type: string - minLength: 1 - maxLength: 256 - parent_id: - type: integer - format: int32 - start_date: - type: string - format: date-time - end_date: - type: string - format: date-time - total_budget: - type: number - format: float - minimum: 1 - maximum: 99999999999999.98 - currency_code: - type: string - maxLength: 3 - zone_name: - type: string - required: - - name - - parent_id - - start_date - - end_date - - total_budget - - currency_code - - zone_name - examples: - Example: - value: - parent_id: 190 - name: valid test item - start_date: '2050-01-01T10:00:00Z' - end_date: '2050-01-10T23:59:00Z' - currency_code: USD - total_budget: 100 - zone_name: America/New_York - description: '' - description: Create a test object -components: - schemas: {} -tags: - - name: test - description: A test item - From dd811bafc0afa989ae0b2be8e9e9dd6b0dff6ae2 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Sat, 18 Dec 2021 18:13:24 +0100 Subject: [PATCH 3/5] work around localhost host mismatch with relative server url Signed-off-by: Pierre Fenoll --- broken_test.go | 148 ----------------------------------- routers/gorillamux/router.go | 6 +- routers/issue356_test.go | 137 ++++++++++++++++++++++++++++++++ routers/types.go | 7 ++ 4 files changed, 148 insertions(+), 150 deletions(-) delete mode 100644 broken_test.go create mode 100644 routers/issue356_test.go diff --git a/broken_test.go b/broken_test.go deleted file mode 100644 index 5dcecffd5..000000000 --- a/broken_test.go +++ /dev/null @@ -1,148 +0,0 @@ -package main - -import ( - "context" - "net/http" - "strings" - "testing" - - "github.com/getkin/kin-openapi/openapi3" - "github.com/getkin/kin-openapi/openapi3filter" - "github.com/getkin/kin-openapi/routers/legacy" - "github.com/stretchr/testify/require" -) - -func TestValidateRequest(t *testing.T) { - spec := `openapi: 3.0.0 -info: - title: Example - version: '1.0' - contact: - name: Chris Rodwell - email: crodwell@github.com - description: test - termsOfService: test -servers: - - url: 'http://localhost:3000' - description: API -paths: - /test: - post: - summary: Create - tags: - - test - responses: - '201': - description: Created - content: - application/json: - schema: - type: object - properties: - name: - type: string - parent_id: - type: integer - start_date: - type: string - end_date: - type: string - total_budget: - type: string - currency_code: - type: string - zone_name: - type: string - operationId: post-test - requestBody: - content: - application/json: - schema: - type: object - properties: - name: - type: string - minLength: 1 - maxLength: 256 - parent_id: - type: integer - format: int32 - start_date: - type: string - format: date-time - end_date: - type: string - format: date-time - total_budget: - type: number - format: float - minimum: 1 - maximum: 99999999999999.98 - currency_code: - type: string - maxLength: 3 - zone_name: - type: string - required: - - name - - parent_id - - start_date - - end_date - - total_budget - - currency_code - - zone_name - examples: - Example: - value: - parent_id: 190 - name: valid test item - start_date: '2050-01-01T10:00:00Z' - end_date: '2050-01-10T23:59:00Z' - currency_code: USD - total_budget: 100 - zone_name: America/New_York - description: '' - description: Create a test object -components: - schemas: {} -tags: - - name: test - description: A test item - -` - - loader := &openapi3.Loader{Context: context.Background()} - doc, err := loader.LoadFromData([]byte(spec)) - require.NoError(t, err) - err = doc.Validate(context.Background()) - require.NoError(t, err) - router, err := legacy.NewRouter(doc) - require.NoError(t, err) - - jsonBody := `{ - "parent_id": 190, - "name": "valid test item", - "start_date": "abc", - "end_date": "2050-01-01T10:00:00Z", - "currency_code": "USD", - "total_budget": 100, - "zone_name": "America/New_York" - }` - - httpReq, err := http.NewRequest(http.MethodPost, "/test", strings.NewReader(jsonBody)) - require.NoError(t, err) - - // Find route - route, pathParams, err := router.FindRoute(httpReq) - require.NoError(t, err) // this fails with No Operation - - // Validate request - requestValidationInput := &openapi3filter.RequestValidationInput{ - Request: httpReq, - PathParams: pathParams, - Route: route, - } - err = openapi3filter.ValidateRequest(context.Background(), requestValidationInput) - require.NoError(t, err) - -} diff --git a/routers/gorillamux/router.go b/routers/gorillamux/router.go index eec1f0122..cec702a4f 100644 --- a/routers/gorillamux/router.go +++ b/routers/gorillamux/router.go @@ -17,6 +17,8 @@ import ( "github.com/gorilla/mux" ) +var _ routers.Router = &Router{} + // Router helps link http.Request.s and an OpenAPIv3 spec type Router struct { muxes []*mux.Route @@ -107,10 +109,10 @@ func (r *Router) FindRoute(req *http.Request) (*routers.Route, map[string]string if err := match.MatchErr; err != nil { // What then? } - route := r.routes[i] + route := *r.routes[i] route.Method = req.Method route.Operation = route.Spec.Paths[route.Path].GetOperation(route.Method) - return route, match.Vars, nil + return &route, match.Vars, nil } switch match.MatchErr { case nil: diff --git a/routers/issue356_test.go b/routers/issue356_test.go new file mode 100644 index 000000000..919c2ed86 --- /dev/null +++ b/routers/issue356_test.go @@ -0,0 +1,137 @@ +package routers_test + +import ( + "context" + "io/ioutil" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/getkin/kin-openapi/openapi3" + "github.com/getkin/kin-openapi/openapi3filter" + "github.com/getkin/kin-openapi/routers" + "github.com/getkin/kin-openapi/routers/gorillamux" + "github.com/getkin/kin-openapi/routers/legacy" + "github.com/stretchr/testify/require" +) + +func TestIssue356(t *testing.T) { + spec := func(servers string) []byte { + return []byte(` +openapi: 3.0.0 +info: + title: Example + version: '1.0' + description: test +servers: +` + servers + ` +paths: + /test: + post: + responses: + '201': + description: Created + content: + application/json: + schema: {type: object} + requestBody: + content: + application/json: + schema: {type: object} + description: '' + description: Create a test object +`) + } + + for servers, expectError := range map[string]bool{ + ` +- url: http://localhost:3000/base +- url: /base +`: false, + + ` +- url: /base +- url: http://localhost:3000/base +`: false, + + `- url: /base`: false, + + `- url: http://localhost:3000/base`: true, + + ``: false, + } { + loader := &openapi3.Loader{Context: context.Background()} + doc, err := loader.LoadFromData(spec(servers)) + require.NoError(t, err) + err = doc.Validate(context.Background()) + require.NoError(t, err) + + for _, newRouter := range []func(*openapi3.T) (routers.Router, error){gorillamux.NewRouter, legacy.NewRouter} { + router, err := newRouter(doc) + require.NoError(t, err) + + { + httpReq, err := http.NewRequest(http.MethodPost, "/base/test", strings.NewReader(`{}`)) + require.NoError(t, err) + httpReq.Header.Set("Content-Type", "application/json") + + route, pathParams, err := router.FindRoute(httpReq) + if expectError { + require.Error(t, err, routers.ErrPathNotFound) + return + } + require.NoError(t, err) + + requestValidationInput := &openapi3filter.RequestValidationInput{ + Request: httpReq, + PathParams: pathParams, + Route: route, + } + err = openapi3filter.ValidateRequest(context.Background(), requestValidationInput) + require.NoError(t, err) + } + + { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + route, pathParams, err := router.FindRoute(r) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(err.Error())) + return + } + + requestValidationInput := &openapi3filter.RequestValidationInput{ + Request: r, + PathParams: pathParams, + Route: route, + } + err = openapi3filter.ValidateRequest(r.Context(), requestValidationInput) + require.NoError(t, err) + + w.Header().Set("Content-Type", "application/json") + w.Write([]byte("{}")) + })) + defer ts.Close() + + req, err := http.NewRequest(http.MethodPost, ts.URL+"/base/test", strings.NewReader(`{}`)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + + rep, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer rep.Body.Close() + body, err := ioutil.ReadAll(rep.Body) + require.NoError(t, err) + + if expectError { + require.Equal(t, 500, rep.StatusCode) + require.Equal(t, routers.ErrPathNotFound.Error(), string(body)) + return + } + require.Equal(t, 200, rep.StatusCode) + require.Equal(t, "{}", string(body)) + } + } + } +} diff --git a/routers/types.go b/routers/types.go index 3cdbee32c..93746cfe9 100644 --- a/routers/types.go +++ b/routers/types.go @@ -8,6 +8,13 @@ import ( // Router helps link http.Request.s and an OpenAPIv3 spec type Router interface { + // FindRoute matches an HTTP request with the operation it resolves to. + // Hosts are matched from the OpenAPIv3 servers key. + // + // If you experience ErrPathNotFound and have localhost hosts specified as your servers, + // turning these server URLs as relative (leaving only the path) should resolve this. + // + // See openapi3filter for example uses with request and response validation. FindRoute(req *http.Request) (route *Route, pathParams map[string]string, err error) } From 6d6307e3f630fcf780b7c9167290b8b0ead678ef Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Sat, 18 Dec 2021 18:27:50 +0100 Subject: [PATCH 4/5] ohwell Signed-off-by: Pierre Fenoll --- .github/workflows/go.yml | 1 + routers/issue356_test.go | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 4a5b87c21..fc619f030 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -64,6 +64,7 @@ jobs: - if: runner.os == 'Linux' run: git --no-pager diff && [[ $(git --no-pager diff --name-only | wc -l) = 0 ]] + - run: go test -count=10 -v -run TestIssue356 ./routers - run: go test ./... - run: go test -v -run TestRaceyPatternSchema -race ./... env: diff --git a/routers/issue356_test.go b/routers/issue356_test.go index 919c2ed86..b87589885 100644 --- a/routers/issue356_test.go +++ b/routers/issue356_test.go @@ -62,16 +62,19 @@ paths: ``: false, } { loader := &openapi3.Loader{Context: context.Background()} + t.Logf("using servers: %q (%v)", servers, expectError) doc, err := loader.LoadFromData(spec(servers)) require.NoError(t, err) err = doc.Validate(context.Background()) require.NoError(t, err) - for _, newRouter := range []func(*openapi3.T) (routers.Router, error){gorillamux.NewRouter, legacy.NewRouter} { + for i, newRouter := range []func(*openapi3.T) (routers.Router, error){gorillamux.NewRouter, legacy.NewRouter} { + t.Logf("using NewRouter from %s", map[int]string{0: "gorillamux", 1: "legacy"}[i]) router, err := newRouter(doc) require.NoError(t, err) - { + if true { + t.Logf("using naked newRouter") httpReq, err := http.NewRequest(http.MethodPost, "/base/test", strings.NewReader(`{}`)) require.NoError(t, err) httpReq.Header.Set("Content-Type", "application/json") @@ -92,7 +95,8 @@ paths: require.NoError(t, err) } - { + if true { + t.Logf("using httptest.NewServer") ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { route, pathParams, err := router.FindRoute(r) if err != nil { From 891d61aef2c347df00427fb8527be1012b905259 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Mon, 20 Dec 2021 13:30:16 +0100 Subject: [PATCH 5/5] fix Signed-off-by: Pierre Fenoll --- routers/issue356_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/issue356_test.go b/routers/issue356_test.go index b87589885..307e52aea 100644 --- a/routers/issue356_test.go +++ b/routers/issue356_test.go @@ -59,7 +59,7 @@ paths: `- url: http://localhost:3000/base`: true, - ``: false, + ``: true, } { loader := &openapi3.Loader{Context: context.Background()} t.Logf("using servers: %q (%v)", servers, expectError)