Remove sanitize HTTP body for knativeerrordata extension (knative#6902

) (#203) Sanitizing the HTTP body was added as a measure to try to handle as many HTTP body as possible, now that we're base64 encoding the error body [1], it is not necessary anymore, it just breaks any consumer trying to use such info. [1] knative#6542 Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
openshift-knative · May 5, 2023 · 7c2eaa8 · 7c2eaa8
1 parent 71cbea9
commit 7c2eaa8
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 75 deletions.
diff --git a/pkg/channel/message_dispatcher.go b/pkg/channel/message_dispatcher.go
@@ -34,13 +34,14 @@ import (
 	"go.uber.org/zap"
 	"k8s.io/apimachinery/pkg/util/sets"
 
+	"knative.dev/pkg/network"
+	"knative.dev/pkg/system"
+
 	"knative.dev/eventing/pkg/broker"
 	"knative.dev/eventing/pkg/channel/attributes"
 	"knative.dev/eventing/pkg/kncloudevents"
 	"knative.dev/eventing/pkg/tracing"
 	"knative.dev/eventing/pkg/utils"
-	"knative.dev/pkg/network"
-	"knative.dev/pkg/system"
 )
 
 const (
@@ -309,55 +310,19 @@ func (d *MessageDispatcherImpl) dispatchExecutionInfoTransformers(destination *u
 	}
 
 	destination = d.sanitizeURL(destination)
-	// Unprintable control characters are not allowed in header values
-	// and cause HTTP requests to fail if not removed.
-	// https://pkg.go.dev/golang.org/x/net/http/httpguts#ValidHeaderFieldValue
-	httpBody := sanitizeHTTPBody(httpResponseBody)
 
 	// Encodes response body as base64 for the resulting length.
-	bodyLen := len(httpBody)
+	bodyLen := len(httpResponseBody)
 	encodedLen := base64.StdEncoding.EncodedLen(bodyLen)
 	if encodedLen > attributes.KnativeErrorDataExtensionMaxLength {
 		encodedLen = attributes.KnativeErrorDataExtensionMaxLength
 	}
 	encodedBuf := make([]byte, encodedLen)
-	base64.StdEncoding.Encode(encodedBuf, []byte(httpBody))
+	base64.StdEncoding.Encode(encodedBuf, httpResponseBody)
 
 	return attributes.KnativeErrorTransformers(*destination, dispatchExecutionInfo.ResponseCode, string(encodedBuf[:encodedLen]))
 }
 
-func sanitizeHTTPBody(body []byte) string {
-	if !hasControlChars(body) {
-		return string(body)
-	}
-
-	sanitizedResponse := make([]byte, 0, len(body))
-	for _, v := range body {
-		if !isControl(v) {
-			sanitizedResponse = append(sanitizedResponse, v)
-		}
-	}
-	return string(sanitizedResponse)
-}
-
-func hasControlChars(data []byte) bool {
-	for _, v := range data {
-		if isControl(v) {
-			return true
-		}
-	}
-	return false
-}
-
-func isControl(c byte) bool {
-	// US ASCII codes range for printable graphic characters and a space.
-	// http://www.columbia.edu/kermit/ascii.html
-	const asciiUnitSeparator = 31
-	const asciiRubout = 127
-
-	return int(c) < asciiUnitSeparator || int(c) > asciiRubout
-}
-
 // isFailure returns true if the status code is not a successful HTTP status.
 func isFailure(statusCode int) bool {
 	return statusCode < nethttp.StatusOK /* 200 */ ||

diff --git a/pkg/channel/message_dispatcher_test.go b/pkg/channel/message_dispatcher_test.go
@@ -693,7 +693,7 @@ func TestDispatchMessage(t *testing.T) {
 					"traceparent":         {"ignored-value-header"},
 					"ce-abc":              {`"ce-abc-value"`},
 					"ce-knativeerrorcode": {strconv.Itoa(http.StatusBadRequest)},
-					"ce-knativeerrordata": {base64.StdEncoding.EncodeToString([]byte("destination multi-line response"))},
+					"ce-knativeerrordata": {base64.StdEncoding.EncodeToString([]byte("destination\n multi-line\n response"))},
 					"ce-id":               {"ignored-value-header"},
 					"ce-time":             {"2002-10-02T15:00:00Z"},
 					"ce-source":           {testCeSource},

diff --git a/test/rekt/features/channel/features.go b/test/rekt/features/channel/features.go
@@ -427,7 +427,6 @@ func channelSubscriberReturnedErrorWithData(createSubscriberFn func(ref *duckv1.
 	f.Setup("install sink", eventshub.Install(sink, eventshub.StartReceiver))
 
 	errorData := "<!doctype html>\n<html>\n<head>\n    <title>Error Page(tm)</title>\n</head>\n<body>\n<p>Quoth the server, 404!\n</body></html>"
-	sanitizeBodyData := sanitizeHTTPBody([]byte(errorData))
 	f.Setup("install failing receiver", eventshub.Install(failer,
 		eventshub.StartReceiver,
 		eventshub.DropFirstN(1),
@@ -461,7 +460,7 @@ func channelSubscriberReturnedErrorWithData(createSubscriberFn func(ref *duckv1.
 			return test.HasExtension("knativeerrorcode", "422")
 		},
 		func(ctx context.Context) test.EventMatcher {
-			return test.HasExtension("knativeerrordata", base64.StdEncoding.EncodeToString([]byte(sanitizeBodyData)))
+			return test.HasExtension("knativeerrordata", base64.StdEncoding.EncodeToString([]byte(errorData)))
 		},
 	))
 
@@ -482,35 +481,3 @@ func assertEnhancedWithKnativeErrorExtensions(sinkName string, matcherfns ...fun
 		)
 	}
 }
-
-func sanitizeHTTPBody(body []byte) string {
-	if !hasControlChars(body) {
-		return string(body)
-	}
-
-	sanitizedResponse := make([]byte, 0, len(body))
-	for _, v := range body {
-		if !isControl(v) {
-			sanitizedResponse = append(sanitizedResponse, v)
-		}
-	}
-	return string(sanitizedResponse)
-}
-
-func isControl(c byte) bool {
-	// US ASCII codes range for printable graphic characters and a space.
-	// http://www.columbia.edu/kermit/ascii.html
-	const asciiUnitSeparator = 31
-	const asciiRubout = 127
-
-	return int(c) < asciiUnitSeparator || int(c) > asciiRubout
-}
-
-func hasControlChars(data []byte) bool {
-	for _, v := range data {
-		if isControl(v) {
-			return true
-		}
-	}
-	return false
-}