Merging to release-4-lts: TT-8934 Fix chunked response analytics (#5495…

…) (#5746)

TT-8934 Fix chunked response analytics (#5495)

<!-- Provide a general summary of your changes in the Title above -->

## Description

<!-- Describe your changes in detail -->

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

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->
https://tyktech.atlassian.net/browse/TT-8934
## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->
https://tyktech.atlassian.net/browse/TT-8934
## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

Added tests

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [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

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the 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 <tombuchaillot89@gmail.com>
Co-authored-by: Tit Petric <tit@tyk.io>

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)

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:]...)
+		}
+	}
+}

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)
+		})
+	}
+}