Repository: avenga/couper
Pull request #230 (2021-05-14 10:47:02)

OpenAPI relative server URLs


Patch:
diff --git a/handler/transport/backend.go b/handler/transport/backend.go
index c8ecce15..e602f203 100644
--- a/handler/transport/backend.go
+++ b/handler/transport/backend.go
@@ -106,8 +106,9 @@ func (b *Backend) RoundTrip(req *http.Request) (*http.Response, error) {
 	}
 
 	if b.openAPIValidator != nil {
-		if err = b.openAPIValidator.ValidateRequest(req); err != nil {
-			return nil, errors.BackendValidation.Label(b.name).With(err)
+		// FIXME tc.Origin should be an origin, not just a host!
+		if err = b.openAPIValidator.ValidateRequest(req, tc.hash(), tc.Scheme+"://"+tc.Origin); err != nil {
+			return nil, errors.BackendValidation.Label(b.name).Kind("backend_request_validation").With(err)
 		}
 	}
 
@@ -142,7 +143,8 @@ func (b *Backend) RoundTrip(req *http.Request) (*http.Response, error) {
 
 	if b.openAPIValidator != nil {
 		if err = b.openAPIValidator.ValidateResponse(beresp); err != nil {
-			return nil, errors.BackendValidation.Label(b.name).With(err).Status(http.StatusBadGateway)
+			return nil, errors.BackendValidation.Label(b.name).Kind("backend_response_validation").
+				With(err).Status(http.StatusBadGateway)
 		}
 	}
 
diff --git a/handler/validation/openapi.go b/handler/validation/openapi.go
index facbf574..74ed42a1 100644
--- a/handler/validation/openapi.go
+++ b/handler/validation/openapi.go
@@ -6,14 +6,18 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
+	"net/url"
+	"sync"
 
-	"github.com/avenga/couper/config/request"
-
+	"github.com/getkin/kin-openapi/openapi3"
 	"github.com/getkin/kin-openapi/openapi3filter"
 
+	"github.com/avenga/couper/config/request"
 	"github.com/avenga/couper/eval"
 )
 
+var swaggers sync.Map
+
 type OpenAPI struct {
 	options                *OpenAPIOptions
 	requestValidationInput *openapi3filter.RequestValidationInput
@@ -28,10 +32,63 @@ func NewOpenAPI(opts *OpenAPIOptions) *OpenAPI {
 	}
 }
 
-func (v *OpenAPI) ValidateRequest(req *http.Request) error {
-	route, pathParams, err := v.options.router.FindRoute(req.Method, req.URL)
+func (v *OpenAPI) getModifiedSwagger(key, origin string) (*openapi3.Swagger, error) {
+	swagger, exists := swaggers.Load(key)
+	if !exists {
+		clonedSwagger := cloneSwagger(v.options.swagger)
+
+		var newServers []string
+		for _, s := range clonedSwagger.Servers {
+			su, err := url.Parse(s.URL)
+			if err != nil {
+				return nil, err
+			}
+			if !su.IsAbs() {
+				newServers = append(newServers, origin+s.URL)
+			}
+		}
+		for _, ns := range newServers {
+			clonedSwagger.AddServer(&openapi3.Server{URL: ns})
+		}
+
+		swaggers.Store(key, clonedSwagger)
+		swagger = clonedSwagger
+	}
+
+	if s, ok := swagger.(*openapi3.Swagger); ok {
+		return s, nil
+	}
+
+	return nil, fmt.Errorf("swagger wrong type: %v", swagger)
+}
+
+func cloneSwagger(s *openapi3.Swagger) *openapi3.Swagger {
+	sw := *s
+	// this is not a deep clone; we only want to add servers
+	sw.Servers = s.Servers[:]
+	return &sw
+}
+
+func (v *OpenAPI) ValidateRequest(req *http.Request, key, origin string) error {
+	swagger, err := v.getModifiedSwagger(key, origin)
+	if err != nil {
+		if ctx, ok := req.Context().Value(request.OpenAPI).(*OpenAPIContext); ok {
+			ctx.errors = append(ctx.errors, err)
+		}
+		if !v.options.ignoreRequestViolations {
+			return err
+		}
+		return nil
+	}
+
+	router := openapi3filter.NewRouter()
+	if err = router.AddSwagger(swagger); err != nil {
+		return err
+	}
+
+	route, pathParams, err := router.FindRoute(req.Method, req.URL)
 	if err != nil {
-		err = fmt.Errorf("request validation: '%s %s': %w", req.Method, req.URL.Path, err)
+		err = fmt.Errorf("'%s %s': %w", req.Method, req.URL.Path, err)
 		if ctx, ok := req.Context().Value(request.OpenAPI).(*OpenAPIContext); ok {
 			ctx.errors = append(ctx.errors, err)
 		}
@@ -50,10 +107,7 @@ func (v *OpenAPI) ValidateRequest(req *http.Request) error {
 	}
 
 	// openapi3filter.ValidateRequestBody also handles resetting the req body after reading until EOF.
-	err = openapi3filter.ValidateRequest(req.Context(), v.requestValidationInput)
-
-	if err != nil {
-		err = fmt.Errorf("request validation: %w", err)
+	if err = openapi3filter.ValidateRequest(req.Context(), v.requestValidationInput); err != nil {
 		if ctx, ok := req.Context().Value(request.OpenAPI).(*OpenAPIContext); ok {
 			ctx.errors = append(ctx.errors, err)
 		}
@@ -68,7 +122,7 @@ func (v *OpenAPI) ValidateRequest(req *http.Request) error {
 func (v *OpenAPI) ValidateResponse(beresp *http.Response) error {
 	// since a request validation could fail and ignored due to user options, the input route MAY be nil
 	if v.requestValidationInput == nil || v.requestValidationInput.Route == nil {
-		err := fmt.Errorf("response validation: '%s %s': invalid route", beresp.Request.Method, beresp.Request.URL.Path)
+		err := fmt.Errorf("'%s %s': invalid route", beresp.Request.Method, beresp.Request.URL.Path)
 		if beresp.Request != nil {
 			if ctx, ok := beresp.Request.Context().Value(request.OpenAPI).(*OpenAPIContext); ok {
 				ctx.errors = append(ctx.errors, err)
@@ -103,7 +157,6 @@ func (v *OpenAPI) ValidateResponse(beresp *http.Response) error {
 	}
 
 	if err := openapi3filter.ValidateResponse(beresp.Request.Context(), responseValidationInput); err != nil {
-		err = fmt.Errorf("response validation: %w", err)
 		if beresp.Request != nil {
 			if ctx, ok := beresp.Request.Context().Value(request.OpenAPI).(*OpenAPIContext); ok {
 				ctx.errors = append(ctx.errors, err)
diff --git a/handler/validation/openapi_options.go b/handler/validation/openapi_options.go
index 74805275..0ffbdad5 100644
--- a/handler/validation/openapi_options.go
+++ b/handler/validation/openapi_options.go
@@ -3,6 +3,7 @@ package validation
 import (
 	"fmt"
 	"io/ioutil"
+	"net/url"
 	"path/filepath"
 
 	"github.com/getkin/kin-openapi/openapi3"
@@ -17,7 +18,7 @@ type OpenAPIOptions struct {
 	ignoreRequestViolations  bool
 	ignoreResponseViolations bool
 	filterOptions            *openapi3filter.Options
-	router                   *openapi3filter.Router
+	swagger                  *openapi3.Swagger
 }
 
 // NewOpenAPIOptions takes a list of openAPI configuration due to merging configurations.
@@ -40,6 +41,26 @@ func NewOpenAPIOptions(openapi *config.OpenAPI) (*OpenAPIOptions, error) {
 	return NewOpenAPIOptionsFromBytes(openapi, b)
 }
 
+func canonicalizeServerURLs(swagger *openapi3.Swagger) error {
+	for _, server := range swagger.Servers {
+		su, err := url.Parse(server.URL)
+		if err != nil {
+			return err
+		}
+
+		if su.IsAbs() && su.Port() == "" && (su.Scheme == "https" || su.Scheme == "http") {
+			su.Host = su.Hostname() + ":"
+			if su.Scheme == "https" {
+				su.Host += "443"
+			} else {
+				su.Host += "80"
+			}
+			server.URL = su.String()
+		}
+	}
+	return nil
+}
+
 func NewOpenAPIOptionsFromBytes(openapi *config.OpenAPI, bytes []byte) (*OpenAPIOptions, error) {
 	if openapi == nil || bytes == nil {
 		return nil, nil
@@ -50,8 +71,7 @@ func NewOpenAPIOptionsFromBytes(openapi *config.OpenAPI, bytes []byte) (*OpenAPI
 		return nil, fmt.Errorf("error loading openapi file: %w", err)
 	}
 
-	router := openapi3filter.NewRouter()
-	if err = router.AddSwagger(swagger); err != nil {
+	if err = canonicalizeServerURLs(swagger); err != nil {
 		return nil, err
 	}
 
@@ -68,6 +88,6 @@ func NewOpenAPIOptionsFromBytes(openapi *config.OpenAPI, bytes []byte) (*OpenAPI
 		},
 		ignoreRequestViolations:  openapi.IgnoreRequestViolations,
 		ignoreResponseViolations: openapi.IgnoreResponseViolations,
-		router:                   router,
+		swagger:                  swagger,
 	}, nil
 }


Test patch:
diff --git a/handler/transport/backend_test.go b/handler/transport/backend_test.go
index 5ec70d5c..0017d2be 100644
--- a/handler/transport/backend_test.go
+++ b/handler/transport/backend_test.go
@@ -197,7 +197,7 @@ func TestBackend_RoundTrip_Validation(t *testing.T) {
 			http.MethodPost,
 			"/get",
 			"backend validation error",
-			"request validation: 'POST /get': Path doesn't support the HTTP method",
+			"'POST /get': Path doesn't support the HTTP method",
 		},
 		{
 			"invalid request, IgnoreRequestViolations",
@@ -205,7 +205,7 @@ func TestBackend_RoundTrip_Validation(t *testing.T) {
 			http.MethodPost,
 			"/get",
 			"",
-			"request validation: 'POST /get': Path doesn't support the HTTP method",
+			"'POST /get': Path doesn't support the HTTP method",
 		},
 		{
 			"invalid response",
@@ -213,7 +213,7 @@ func TestBackend_RoundTrip_Validation(t *testing.T) {
 			http.MethodGet,
 			"/get?404",
 			"backend validation error",
-			"response validation: status is not supported",
+			"status is not supported",
 		},
 		{
 			"invalid response, IgnoreResponseViolations",
@@ -221,11 +221,11 @@ func TestBackend_RoundTrip_Validation(t *testing.T) {
 			http.MethodGet,
 			"/get?404",
 			"",
-			"response validation: status is not supported",
+			"status is not supported",
 		},
 	}
 
-	logger, hook := logrustest.NewNullLogger()
+	logger, hook := test.NewLogger()
 	log := logger.WithContext(context.Background())
 
 	for _, tt := range tests {
@@ -260,13 +260,15 @@ func TestBackend_RoundTrip_Validation(t *testing.T) {
 			entry := hook.LastEntry()
 			if tt.expectedLogMessage != "" {
 				if data, ok := entry.Data["validation"]; ok {
-					for _, err := range data.([]string) {
-						if err == tt.expectedLogMessage {
+					for _, errStr := range data.([]string) {
+						if errStr != tt.expectedLogMessage {
+							subT.Errorf("\nwant:\t%s\ngot:\t%v", tt.expectedLogMessage, errStr)
 							return
 						}
+						return
 					}
-					for _, err := range data.([]string) {
-						subT.Log(err)
+					for _, errStr := range data.([]string) {
+						subT.Log(errStr)
 					}
 				}
 				subT.Errorf("expected matching validation error logs:\n\t%s\n\tgot: nothing", tt.expectedLogMessage)
diff --git a/handler/validation/openapi_test.go b/handler/validation/openapi_test.go
index ead7d2e1..9540d9e7 100644
--- a/handler/validation/openapi_test.go
+++ b/handler/validation/openapi_test.go
@@ -43,7 +43,7 @@ func TestOpenAPIValidator_ValidateRequest(t *testing.T) {
 		}
 	}))
 
-	log, hook := logrustest.NewNullLogger()
+	log, hook := test.NewLogger()
 	logger := log.WithContext(context.Background())
 	beConf := &config.Backend{
 		Remain: body.New(&hcl.BodyContent{Attributes: hcl.Attributes{
@@ -69,10 +69,10 @@ func TestOpenAPIValidator_ValidateRequest(t *testing.T) {
 		wantBody   bool
 		wantErrLog string
 	}{
-		{"GET without required query", "/a?b", nil, false, "request validation: Parameter 'b' in query has an error: must have a value: must have a value"},
+		{"GET without required query", "/a?b", nil, false, "backend validation error: Parameter 'b' in query has an error: must have a value: must have a value"},
 		{"GET with required query", "/a?b=value", nil, false, ""},
 		{"GET with required path", "/a/value", nil, false, ""},
-		{"GET with required path missing", "/a//", nil, false, "request validation: Parameter 'b' in query has an error: must have a value: must have a value"},
+		{"GET with required path missing", "/a//", nil, false, "backend validation error: Parameter 'b' in query has an error: must have a value: must have a value"},
 		{"GET with optional query", "/b", nil, false, ""},
 		{"GET with optional path param", "/b/a", nil, false, ""},
 		{"GET with required json body", "/json", strings.NewReader(`["hans", "wurst"]`), true, ""},
@@ -87,32 +87,23 @@ func TestOpenAPIValidator_ValidateRequest(t *testing.T) {
 			}
 
 			hook.Reset()
-			res, err := backend.RoundTrip(req)
+			var res *http.Response
+			res, err = backend.RoundTrip(req)
 			if err != nil && tt.wantErrLog == "" {
-				helper.Must(err)
+				t.Error(err)
+				return
 			}
 
 			if tt.wantErrLog != "" {
-				var found bool
-				for _, entry := range hook.Entries {
-					if valEntry, ok := entry.Data["validation"]; ok {
-						if list, ok := valEntry.([]string); ok {
-							for _, valMsg := range list {
-								if valMsg == tt.wantErrLog {
-									found = true
-									break
-								}
-							}
-						}
-					}
-				}
-				if !found {
-					t.Errorf("Expected error log: %q", tt.wantErrLog)
+				entry := hook.LastEntry()
+				if entry.Message != tt.wantErrLog {
+					t.Errorf("Expected error log:\nwant:\t%q\ngot:\t%s", tt.wantErrLog, entry.Message)
 				}
 			}
 
 			if tt.wantBody {
-				n, err := io.Copy(ioutil.Discard, res.Body)
+				var n int64
+				n, err = io.Copy(ioutil.Discard, res.Body)
 				if err != nil {
 					t.Error(err)
 				}
@@ -129,3 +120,92 @@ func TestOpenAPIValidator_ValidateRequest(t *testing.T) {
 		})
 	}
 }
+
+func TestOpenAPIValidator_RelativeServerURL(t *testing.T) {
+	helper := test.New(t)
+
+	log, hook := logrustest.NewNullLogger()
+	logger := log.WithContext(context.Background())
+	beConf := &config.Backend{
+		Remain: body.New(&hcl.BodyContent{Attributes: hcl.Attributes{
+			"origin": &hcl.Attribute{
+				Name: "origin",
+				Expr: hcltest.MockExprLiteral(cty.StringVal("https://httpbin.org")),
+			},
+		}}),
+		OpenAPI: &config.OpenAPI{
+			File: filepath.Join("testdata/backend_02_openapi.yaml"),
+		},
+	}
+	openAPI, err := validation.NewOpenAPIOptions(beConf.OpenAPI)
+	helper.Must(err)
+
+	backend := transport.NewBackend(beConf.Remain, &transport.Config{}, &transport.BackendOptions{
+		OpenAPI: openAPI,
+	}, logger)
+
+	t.Run("relative server URL", func(t *testing.T) {
+		req := httptest.NewRequest(http.MethodGet, "https://httpbin.org/anything", nil)
+
+		hook.Reset()
+		_, err := backend.RoundTrip(req)
+		if err != nil {
+			helper.Must(err)
+		}
+
+		if t.Failed() {
+			for _, entry := range hook.Entries {
+				t.Log(entry.String())
+			}
+		}
+	})
+}
+
+func TestOpenAPIValidator_NonCanonicalServerURL(t *testing.T) {
+	helper := test.New(t)
+
+	log, hook := logrustest.NewNullLogger()
+	logger := log.WithContext(context.Background())
+	beConf := &config.Backend{
+		Remain: body.New(&hcl.BodyContent{Attributes: hcl.Attributes{
+			"origin": &hcl.Attribute{
+				Name: "origin",
+				Expr: hcltest.MockExprLiteral(cty.StringVal("https://httpbin.org")),
+			},
+		}}),
+		OpenAPI: &config.OpenAPI{
+			File: filepath.Join("testdata/backend_03_openapi.yaml"),
+		},
+	}
+	openAPI, err := validation.NewOpenAPIOptions(beConf.OpenAPI)
+	helper.Must(err)
+
+	backend := transport.NewBackend(beConf.Remain, &transport.Config{}, &transport.BackendOptions{
+		OpenAPI: openAPI,
+	}, logger)
+
+	tests := []struct {
+		name, url string
+	}{
+		{"http", "http://httpbin.org/anything"},
+		{"https", "https://httpbin.org/anything"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodGet, tt.url, nil)
+
+			hook.Reset()
+			_, err := backend.RoundTrip(req)
+			if err != nil {
+				helper.Must(err)
+			}
+
+			if t.Failed() {
+				for _, entry := range hook.Entries {
+					t.Log(entry.String())
+				}
+			}
+		})
+	}
+}
diff --git a/handler/validation/testdata/backend_02_openapi.yaml b/handler/validation/testdata/backend_02_openapi.yaml
new file mode 100644
index 00000000..7dd24e6e
--- /dev/null
+++ b/handler/validation/testdata/backend_02_openapi.yaml
@@ -0,0 +1,12 @@
+openapi: '3'
+info:
+  title: 'Couper backend validation test: relative server URL'
+  version: 'v1.2.3'
+servers:
+  - url: /
+paths:
+  /anything:
+    get:
+      responses:
+        200:
+          description: OK
diff --git a/handler/validation/testdata/backend_03_openapi.yaml b/handler/validation/testdata/backend_03_openapi.yaml
new file mode 100644
index 00000000..94f8a419
--- /dev/null
+++ b/handler/validation/testdata/backend_03_openapi.yaml
@@ -0,0 +1,13 @@
+openapi: '3'
+info:
+  title: 'Couper backend validation test: non-canonical server URL'
+  version: 'v1.2.3'
+servers:
+  - url: https://httpbin.org/
+  - url: http://httpbin.org/
+paths:
+  /anything:
+    get:
+      responses:
+        200:
+          description: OK