From 7dd58742ae5ef00778ec1184e1924d7ef05c6a8d Mon Sep 17 00:00:00 2001 From: Leonid Bugaev Date: Tue, 7 Nov 2023 10:43:56 +0300 Subject: [PATCH] Merging to release-4-lts: TT-8934 Fix chunked response analytics (#5495) (#5746) TT-8934 Fix chunked response analytics (#5495) ## Description When transfer-encoding is chunked on the upstream response, analytics records raw responses also contain the chunked characters (example https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Transfer-Encoding#examples). This is not expected. The proposed solution is to delete the transfer-encoding header before reading and writing the response body to the rawResponse field. Without this header, Go will process the header as it should. ## Related Issue https://tyktech.atlassian.net/browse/TT-8934 ## Motivation and Context https://tyktech.atlassian.net/browse/TT-8934 ## How This Has Been Tested Added tests ## Screenshots (if appropriate) ## Types of changes - [X] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Refactoring or add test (improvements in base code or adds test coverage to functionality) ## Checklist - [ ] I ensured that the documentation is up to date - [ ] I explained why this PR updates go.mod in detail with reasoning why it's required - [ ] I would like a code coverage CI quality gate exception and have explained why --------- Co-authored-by: Tomas Buchaillot Co-authored-by: Tit Petric --- gateway/handler_success.go | 5 +++ internal/httputil/response.go | 33 ++++++++++++++ internal/httputil/response_test.go | 72 ++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+) create mode 100644 internal/httputil/response.go create mode 100644 internal/httputil/response_test.go diff --git a/gateway/handler_success.go b/gateway/handler_success.go index 1e6d6b44fb8..6dfadbb68bf 100644 --- a/gateway/handler_success.go +++ b/gateway/handler_success.go @@ -11,6 +11,8 @@ import ( "strings" "time" + "github.com/TykTechnologies/tyk/internal/httputil" + "github.com/TykTechnologies/tyk/config" "github.com/TykTechnologies/tyk/ctx" "github.com/TykTechnologies/tyk/headers" @@ -170,6 +172,9 @@ func (s *SuccessHandler) RecordHit(r *http.Request, timing Latency, code int, re // mw_redis_cache instead? is there a reason not // to include that in the analytics? if responseCopy != nil { + // we need to delete the chunked transfer encoding header to avoid malformed body in our rawResponse + httputil.RemoveResponseTransferEncoding(responseCopy, "chunked") + contents, err := ioutil.ReadAll(responseCopy.Body) if err != nil { log.Error("Couldn't read response body", err) diff --git a/internal/httputil/response.go b/internal/httputil/response.go new file mode 100644 index 00000000000..26e7fff1466 --- /dev/null +++ b/internal/httputil/response.go @@ -0,0 +1,33 @@ +package httputil + +import ( + "net/http" +) + +// EntityTooLarge responds with HTTP 413 Request Entity Too Large. +// The function is used for a response when blocking requests by size. +func EntityTooLarge(w http.ResponseWriter, _ *http.Request) { + status := http.StatusRequestEntityTooLarge + http.Error(w, http.StatusText(status), status) +} + +// LengthRequired responds with HTTP 411 Length Required. +// The function is used in places where Content-Length is required. +func LengthRequired(w http.ResponseWriter, _ *http.Request) { + status := http.StatusLengthRequired + http.Error(w, http.StatusText(status), status) +} + +// InternalServerError responds with HTTP 503 Internal Server Error. +func InternalServerError(w http.ResponseWriter, _ *http.Request) { + status := http.StatusInternalServerError + http.Error(w, http.StatusText(status), status) +} + +func RemoveResponseTransferEncoding(response *http.Response, encoding string) { + for i, value := range response.TransferEncoding { + if value == encoding { + response.TransferEncoding = append(response.TransferEncoding[:i], response.TransferEncoding[i+1:]...) + } + } +} diff --git a/internal/httputil/response_test.go b/internal/httputil/response_test.go new file mode 100644 index 00000000000..6efea65b025 --- /dev/null +++ b/internal/httputil/response_test.go @@ -0,0 +1,72 @@ +package httputil + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestRequestUtilities(t *testing.T) { + w := httptest.NewRecorder() + EntityTooLarge(w, nil) + assert.Equal(t, http.StatusRequestEntityTooLarge, w.Result().StatusCode) + + w = httptest.NewRecorder() + LengthRequired(w, nil) + assert.Equal(t, http.StatusLengthRequired, w.Result().StatusCode) + + w = httptest.NewRecorder() + InternalServerError(w, nil) + assert.Equal(t, http.StatusInternalServerError, w.Result().StatusCode) +} + +func TestRemoveResponseTransferEncoding(t *testing.T) { + tests := []struct { + name string + response *http.Response + encoding string + expectedOutput []string + }{ + { + name: "Remove chunked encoding", + response: &http.Response{ + TransferEncoding: []string{"chunked", "gzip"}, + }, + encoding: "chunked", + expectedOutput: []string{"gzip"}, + }, + { + name: "Remove gzip encoding", + response: &http.Response{ + TransferEncoding: []string{"chunked", "gzip"}, + }, + encoding: "gzip", + expectedOutput: []string{"chunked"}, + }, + { + name: "Remove non-existent encoding", + response: &http.Response{ + TransferEncoding: []string{"chunked", "gzip"}, + }, + encoding: "deflate", + expectedOutput: []string{"chunked", "gzip"}, + }, + { + name: "Remove from empty slice", + response: &http.Response{ + TransferEncoding: []string{}, + }, + encoding: "gzip", + expectedOutput: []string{}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + RemoveResponseTransferEncoding(tc.response, tc.encoding) + assert.Equal(t, tc.expectedOutput, tc.response.TransferEncoding) + }) + } +}