From f7b9830b64a9928bb9e93bb203efe2adb9e438d9 Mon Sep 17 00:00:00 2001 From: Daniel Ortiz Date: Wed, 14 Feb 2024 17:28:22 +0000 Subject: [PATCH 1/5] Add the encoding to the HTTPResponseError so, when returning an error from the backend, the content-type matches. This change only applies when using "return_error_code" or "return_error_details". Signed-off-by: Daniel Ortiz --- proxy/http_test.go | 2 +- transport/http/client/status.go | 7 +++++++ transport/http/client/status_test.go | 11 +++++++++-- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/proxy/http_test.go b/proxy/http_test.go index 1d67a10e0..9fd1f3cfc 100644 --- a/proxy/http_test.go +++ b/proxy/http_test.go @@ -240,7 +240,7 @@ func TestNewHTTPProxy_badStatusCode_detailed(t *testing.T) { t.Errorf("unexpected error code: %d", response.Metadata.StatusCode) } b, _ := json.Marshal(response.Data) - if string(b) != `{"error_some":{"http_status_code":500,"http_body":"booom\n"}}` { + if string(b) != `{"error_some":{"http_status_code":500,"http_body":"booom\n","http_body_encoding":"text/plain; charset=utf-8"}}` { t.Errorf("unexpected response content: %s", string(b)) } select { diff --git a/transport/http/client/status.go b/transport/http/client/status.go index 25d846162..893a77dc8 100644 --- a/transport/http/client/status.go +++ b/transport/http/client/status.go @@ -87,6 +87,7 @@ func newHTTPResponseError(resp *http.Response) HTTPResponseError { return HTTPResponseError{ Code: resp.StatusCode, Msg: string(body), + Enc: resp.Header.Get("Content-Type"), } } @@ -94,6 +95,7 @@ func newHTTPResponseError(resp *http.Response) HTTPResponseError { type HTTPResponseError struct { Code int `json:"http_status_code"` Msg string `json:"http_body,omitempty"` + Enc string `json:"http_body_encoding,omitempty"` } // Error returns the error message @@ -106,6 +108,11 @@ func (r HTTPResponseError) StatusCode() int { return r.Code } +// Encoding returns the content type returned by the backend +func (r HTTPResponseError) Encoding() string { + return r.Enc +} + // NamedHTTPResponseError is the error to be returned by the DetailedHTTPStatusHandler type NamedHTTPResponseError struct { HTTPResponseError diff --git a/transport/http/client/status_test.go b/transport/http/client/status_test.go index 82912f281..9ebc326f6 100644 --- a/transport/http/client/status_test.go +++ b/transport/http/client/status_test.go @@ -5,6 +5,7 @@ package client import ( "bytes" "context" + "fmt" "io" "net/http" "testing" @@ -14,6 +15,7 @@ import ( func TestDetailedHTTPStatusHandler(t *testing.T) { expectedErrName := "some" + expectedEncoding := "application/json; charset=utf-8" cfg := &config.Backend{ ExtraConfig: config.ExtraConfig{ Namespace: map[string]interface{}{ @@ -47,7 +49,8 @@ func TestDetailedHTTPStatusHandler(t *testing.T) { resp := &http.Response{ StatusCode: code, - Body: io.NopCloser(bytes.NewBufferString(msg)), + Body: io.NopCloser(bytes.NewBufferString(fmt.Sprintf(`{"msg":"%s"}`, msg))), + Header: http.Header{"Content-Type": []string{expectedEncoding}}, } r, err := sh(context.Background(), resp) @@ -68,7 +71,7 @@ func TestDetailedHTTPStatusHandler(t *testing.T) { return } - if e.Error() != msg { + if e.Error() != fmt.Sprintf(`{"msg":"%s"}`, msg) { t.Errorf("#%d unexpected message: %s", i, e.Msg) return } @@ -77,6 +80,10 @@ func TestDetailedHTTPStatusHandler(t *testing.T) { t.Errorf("#%d unexpected error name: %s", i, e.name) return } + + if e.Encoding() != expectedEncoding { + t.Errorf("#%d unexpected encoding: %s", i, e.Enc) + } } } From a17f86fe9803c7ab44705e7fd649b3149d5e6658 Mon Sep 17 00:00:00 2001 From: Daniel Ortiz Date: Thu, 15 Feb 2024 15:25:43 +0000 Subject: [PATCH 2/5] Fix tests and add mux support. Signed-off-by: Daniel Ortiz --- router/mux/endpoint.go | 21 +++++++++++++++++++-- router/mux/router.go | 8 ++++++++ router/mux/router_test.go | 8 -------- test/integration_test.go | 4 ++-- test/lura.json | 3 +++ 5 files changed, 32 insertions(+), 12 deletions(-) diff --git a/router/mux/endpoint.go b/router/mux/endpoint.go index 66b21e936..ae8ffa6ca 100644 --- a/router/mux/endpoint.go +++ b/router/mux/endpoint.go @@ -82,9 +82,12 @@ func CustomEndpointHandlerWithHTTPError(rb RequestBuilder, errF server.ToHTTPErr w.Header().Set(server.CompleteResponseHeaderName, server.HeaderIncompleteResponseValue) if err != nil { if t, ok := err.(responseError); ok { - http.Error(w, err.Error(), t.StatusCode()) + w.WriteHeader(t.StatusCode()) } else { - http.Error(w, err.Error(), errF(err)) + w.WriteHeader(errF(err)) + } + if returnErrorMsg { + ErrorResponseWriter(w, err) } cancel() return @@ -169,6 +172,11 @@ type responseError interface { StatusCode() int } +type encodedResponseError interface { + responseError + Encoding() string +} + // clientIP implements a best effort algorithm to return the real client IP, it parses // X-Real-IP and X-Forwarded-For in order to work properly with reverse-proxies such us: nginx or haproxy. // Use X-Forwarded-For before X-Real-Ip as nginx uses X-Real-Ip with the proxy's IP. @@ -192,3 +200,12 @@ func clientIP(r *http.Request) string { return "" } + +// ErrorResponseWriter writes the string representation of an error into the response body +// and sets a Content-Type header for errors that implement the encodedResponseError interface. +var ErrorResponseWriter = func(rw http.ResponseWriter, err error) { + if te, ok := err.(encodedResponseError); ok && te.Encoding() != "" { + rw.Header().Set("Content-Type", te.Encoding()) + } + rw.Write([]byte(err.Error())) +} diff --git a/router/mux/router.go b/router/mux/router.go index bd4b3abd8..44a1137d2 100644 --- a/router/mux/router.go +++ b/router/mux/router.go @@ -22,6 +22,7 @@ const ( DefaultDebugPattern = "/__debug/" DefaultEchoPattern = "/__echo/" logPrefix = "[SERVICE: Mux]" + Namespace = "github_com/luraproject/lura/router/mux" ) // RunServerFunc is a func that will run the http Server with the given params. @@ -130,6 +131,11 @@ func (r httpRouter) Run(cfg config.ServiceConfig) { } } + if v, ok := cfg.ExtraConfig[Namespace].(map[string]interface{}); ok { + b, ok := v["return_error_msg"].(bool) + returnErrorMsg = b && ok + } + r.cfg.Engine.Handle("/__health", "GET", http.HandlerFunc(HealthHandler)) server.InitHTTPDefaultTransport(cfg) @@ -187,3 +193,5 @@ func (r httpRouter) handler() http.Handler { } return handler } + +var returnErrorMsg bool diff --git a/router/mux/router_test.go b/router/mux/router_test.go index 955084212..f7f0b58a8 100644 --- a/router/mux/router_test.go +++ b/router/mux/router_test.go @@ -174,14 +174,6 @@ func TestDefaultFactory_ko(t *testing.T) { Method: "GETTT", Backend: []*config.Backend{}, }, - { - Endpoint: "/also-ignored", - Method: "PUT", - Backend: []*config.Backend{ - {}, - {}, - }, - }, }, } diff --git a/test/integration_test.go b/test/integration_test.go index 7f5d96130..b41bc00dc 100644 --- a/test/integration_test.go +++ b/test/integration_test.go @@ -73,6 +73,7 @@ func TestKrakenD_ginRouter(t *testing.T) { "trusted_proxies": []interface{}{"127.0.0.1/32", "::1"}, "remote_ip_headers": []interface{}{"x-forwarded-for"}, "forwarded_by_client_ip": true, + "return_error_msg": true, } ignoredChan := make(chan string) @@ -238,7 +239,7 @@ func testKrakenD(t *testing.T, runRouter func(logging.Logger, *config.ServiceCon url: "/detail_error", headers: map[string]string{}, expHeaders: incompleteHeader, - expBody: `{"email":"some@email.com","error_backend_a":{"http_status_code":429,"http_body":"sad panda\n"},"name":"a"}`, + expBody: `{"email":"some@email.com","error_backend_a":{"http_status_code":429,"http_body":"sad panda\n","http_body_encoding":"text/plain; charset=utf-8"},"name":"a"}`, }, { name: "querystring-params-no-params", @@ -454,7 +455,6 @@ func testKrakenD(t *testing.T, runRouter func(logging.Logger, *config.ServiceCon } }) } - } func setupBackend(t *testing.T) (*config.ServiceConfig, error) { diff --git a/test/lura.json b/test/lura.json index 6ee7da48f..ee74c76a1 100644 --- a/test/lura.json +++ b/test/lura.json @@ -8,6 +8,9 @@ "extra_config":{ "github_com/luraproject/lura/router/gin":{ "return_error_msg":true + }, + "github_com/luraproject/lura/router/mux":{ + "return_error_msg":true } }, "endpoints": [ From 5f792f33adb914311e405f2b270a11387610adea Mon Sep 17 00:00:00 2001 From: Daniel Ortiz Date: Thu, 15 Feb 2024 21:40:41 +0000 Subject: [PATCH 3/5] Remove mux support since we can't write headers after the first write. Signed-off-by: Daniel Ortiz --- router/mux/endpoint.go | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/router/mux/endpoint.go b/router/mux/endpoint.go index ae8ffa6ca..66b21e936 100644 --- a/router/mux/endpoint.go +++ b/router/mux/endpoint.go @@ -82,12 +82,9 @@ func CustomEndpointHandlerWithHTTPError(rb RequestBuilder, errF server.ToHTTPErr w.Header().Set(server.CompleteResponseHeaderName, server.HeaderIncompleteResponseValue) if err != nil { if t, ok := err.(responseError); ok { - w.WriteHeader(t.StatusCode()) + http.Error(w, err.Error(), t.StatusCode()) } else { - w.WriteHeader(errF(err)) - } - if returnErrorMsg { - ErrorResponseWriter(w, err) + http.Error(w, err.Error(), errF(err)) } cancel() return @@ -172,11 +169,6 @@ type responseError interface { StatusCode() int } -type encodedResponseError interface { - responseError - Encoding() string -} - // clientIP implements a best effort algorithm to return the real client IP, it parses // X-Real-IP and X-Forwarded-For in order to work properly with reverse-proxies such us: nginx or haproxy. // Use X-Forwarded-For before X-Real-Ip as nginx uses X-Real-Ip with the proxy's IP. @@ -200,12 +192,3 @@ func clientIP(r *http.Request) string { return "" } - -// ErrorResponseWriter writes the string representation of an error into the response body -// and sets a Content-Type header for errors that implement the encodedResponseError interface. -var ErrorResponseWriter = func(rw http.ResponseWriter, err error) { - if te, ok := err.(encodedResponseError); ok && te.Encoding() != "" { - rw.Header().Set("Content-Type", te.Encoding()) - } - rw.Write([]byte(err.Error())) -} From 035759cf522afe728fbef9ba3819a2ad7534b430 Mon Sep 17 00:00:00 2001 From: "deepsource-autofix[bot]" <62050782+deepsource-autofix[bot]@users.noreply.github.com> Date: Thu, 15 Feb 2024 21:47:34 +0000 Subject: [PATCH 4/5] refactor: autofix issues in 1 file Resolved issues in transport/http/client/status_test.go with DeepSource Autofix --- transport/http/client/status_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/transport/http/client/status_test.go b/transport/http/client/status_test.go index 9ebc326f6..45b1ae898 100644 --- a/transport/http/client/status_test.go +++ b/transport/http/client/status_test.go @@ -49,7 +49,7 @@ func TestDetailedHTTPStatusHandler(t *testing.T) { resp := &http.Response{ StatusCode: code, - Body: io.NopCloser(bytes.NewBufferString(fmt.Sprintf(`{"msg":"%s"}`, msg))), + Body: io.NopCloser(bytes.NewBufferString(fmt.Sprintf(`{"msg":%q}`, msg))), Header: http.Header{"Content-Type": []string{expectedEncoding}}, } @@ -71,7 +71,7 @@ func TestDetailedHTTPStatusHandler(t *testing.T) { return } - if e.Error() != fmt.Sprintf(`{"msg":"%s"}`, msg) { + if e.Error() != fmt.Sprintf(`{"msg":%q}`, msg) { t.Errorf("#%d unexpected message: %s", i, e.Msg) return } From f91f22ee5a4de2324d5454fa2cb5d7f1ee9aaabe Mon Sep 17 00:00:00 2001 From: Daniel Ortiz Date: Thu, 15 Feb 2024 21:49:32 +0000 Subject: [PATCH 5/5] Remove unused code. Signed-off-by: Daniel Ortiz --- router/mux/router.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/router/mux/router.go b/router/mux/router.go index 44a1137d2..bd4b3abd8 100644 --- a/router/mux/router.go +++ b/router/mux/router.go @@ -22,7 +22,6 @@ const ( DefaultDebugPattern = "/__debug/" DefaultEchoPattern = "/__echo/" logPrefix = "[SERVICE: Mux]" - Namespace = "github_com/luraproject/lura/router/mux" ) // RunServerFunc is a func that will run the http Server with the given params. @@ -131,11 +130,6 @@ func (r httpRouter) Run(cfg config.ServiceConfig) { } } - if v, ok := cfg.ExtraConfig[Namespace].(map[string]interface{}); ok { - b, ok := v["return_error_msg"].(bool) - returnErrorMsg = b && ok - } - r.cfg.Engine.Handle("/__health", "GET", http.HandlerFunc(HealthHandler)) server.InitHTTPDefaultTransport(cfg) @@ -193,5 +187,3 @@ func (r httpRouter) handler() http.Handler { } return handler } - -var returnErrorMsg bool