[receiver/otlpreceiver] improve request's content-type check

content-type header could contain optional parameters after the mime-type, do not reject requests in this case
open-telemetry · Mar 30, 2023 · 0df234c · 0df234c
1 parent e9b7929
commit 0df234c
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 21 deletions.
diff --git a/receiver/otlpreceiver/otlp.go b/receiver/otlpreceiver/otlp.go
@@ -205,7 +205,7 @@ func (r *otlpReceiver) registerTraceConsumer(tc consumer.Traces) error {
 				handleUnmatchedMethod(resp)
 				return
 			}
-			switch req.Header.Get("Content-Type") {
+			switch getMimeTypeFromContentType(req.Header.Get("Content-Type")) {
 			case pbContentType:
 				handleTraces(resp, req, httpTracesReceiver, pbEncoder)
 			case jsonContentType:
@@ -230,7 +230,7 @@ func (r *otlpReceiver) registerMetricsConsumer(mc consumer.Metrics) error {
 				handleUnmatchedMethod(resp)
 				return
 			}
-			switch req.Header.Get("Content-Type") {
+			switch getMimeTypeFromContentType(req.Header.Get("Content-Type")) {
 			case pbContentType:
 				handleMetrics(resp, req, httpMetricsReceiver, pbEncoder)
 			case jsonContentType:
@@ -255,7 +255,7 @@ func (r *otlpReceiver) registerLogsConsumer(lc consumer.Logs) error {
 				handleUnmatchedMethod(resp)
 				return
 			}
-			switch req.Header.Get("Content-Type") {
+			switch getMimeTypeFromContentType(req.Header.Get("Content-Type")) {
 			case pbContentType:
 				handleLogs(resp, req, httpLogsReceiver, pbEncoder)
 			case jsonContentType:

diff --git a/receiver/otlpreceiver/otlp_test.go b/receiver/otlpreceiver/otlp_test.go
@@ -139,27 +139,42 @@ var traceOtlp = func() ptrace.Traces {
 
 func TestJsonHttp(t *testing.T) {
 	tests := []struct {
-		name     string
-		encoding string
-		err      error
+		name        string
+		encoding    string
+		contentType string
+		err         error
 	}{
 		{
-			name:     "JSONUncompressed",
-			encoding: "",
+			name:        "JSONUncompressed",
+			encoding:    "",
+			contentType: "application/json",
 		},
 		{
-			name:     "JSONGzipCompressed",
-			encoding: "gzip",
+			name:        "JSONUncompressedUTF8",
+			encoding:    "",
+			contentType: "application/json; charset=utf-8",
 		},
 		{
-			name:     "NotGRPCError",
-			encoding: "",
-			err:      errors.New("my error"),
+			name:        "JSONUncompressedUppercase",
+			encoding:    "",
+			contentType: "APPLICATION/JSON",
 		},
 		{
-			name:     "GRPCError",
-			encoding: "",
-			err:      status.New(codes.Internal, "").Err(),
+			name:        "JSONGzipCompressed",
+			encoding:    "gzip",
+			contentType: "application/json",
+		},
+		{
+			name:        "NotGRPCError",
+			encoding:    "",
+			contentType: "application/json",
+			err:         errors.New("my error"),
+		},
+		{
+			name:        "GRPCError",
+			encoding:    "",
+			contentType: "application/json",
+			err:         status.New(codes.Internal, "").Err(),
 		},
 	}
 	addr := testutil.GetAvailableLocalAddress(t)
@@ -179,7 +194,7 @@ func TestJsonHttp(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			url := fmt.Sprintf("http://%s/v1/traces", addr)
 			sink.Reset()
-			testHTTPJSONRequest(t, url, sink, test.encoding, test.err)
+			testHTTPJSONRequest(t, url, sink, test.encoding, test.contentType, test.err)
 		})
 	}
 }
@@ -238,6 +253,15 @@ func TestHandleInvalidRequests(t *testing.T) {
 			expectedStatus:       http.StatusUnsupportedMediaType,
 			expectedResponseBody: "415 unsupported media type, supported: [application/json, application/x-protobuf]",
 		},
+		{
+			name:        "POST /v1/traces, invalid content type",
+			uri:         "/v1/traces",
+			method:      http.MethodPost,
+			contentType: "invalid",
+
+			expectedStatus:       http.StatusUnsupportedMediaType,
+			expectedResponseBody: "415 unsupported media type, supported: [application/json, application/x-protobuf]",
+		},
 		{
 			name:        "PATCH /v1/traces",
 			uri:         "/v1/traces",
@@ -336,7 +360,7 @@ func TestHandleInvalidRequests(t *testing.T) {
 	require.NoError(t, err)
 }
 
-func testHTTPJSONRequest(t *testing.T, url string, sink *errOrSinkConsumer, encoding string, expectedErr error) {
+func testHTTPJSONRequest(t *testing.T, url string, sink *errOrSinkConsumer, encoding string, contentType string, expectedErr error) {
 	var buf *bytes.Buffer
 	var err error
 	switch encoding {
@@ -349,7 +373,7 @@ func testHTTPJSONRequest(t *testing.T, url string, sink *errOrSinkConsumer, enco
 	sink.SetConsumeError(expectedErr)
 	req, err := http.NewRequest(http.MethodPost, url, buf)
 	require.NoError(t, err, "Error creating trace POST request: %v", err)
-	req.Header.Set("Content-Type", "application/json")
+	req.Header.Set("Content-Type", contentType)
 	req.Header.Set("Content-Encoding", encoding)
 
 	client := &http.Client{}

diff --git a/receiver/otlpreceiver/otlphttp.go b/receiver/otlpreceiver/otlphttp.go
@@ -16,6 +16,7 @@ package otlpreceiver // import "go.opentelemetry.io/collector/receiver/otlprecei
 
 import (
 	"io"
+	"mime"
 	"net/http"
 
 	spb "google.golang.org/genproto/googleapis/rpc/status"
@@ -136,8 +137,7 @@ func writeError(w http.ResponseWriter, encoder encoder, err error, statusCode in
 // by the OTLP protocol.
 func errorHandler(w http.ResponseWriter, r *http.Request, errMsg string, statusCode int) {
 	s := errorMsgToStatus(errMsg, statusCode)
-	contentType := r.Header.Get("Content-Type")
-	switch contentType {
+	switch getMimeTypeFromContentType(r.Header.Get("Content-Type")) {
 	case pbContentType:
 		writeStatusResponse(w, pbEncoder, statusCode, s.Proto())
 		return
@@ -171,3 +171,11 @@ func errorMsgToStatus(errMsg string, statusCode int) *status.Status {
 	}
 	return status.New(codes.Unknown, errMsg)
 }
+
+func getMimeTypeFromContentType(contentType string) string {
+	mediatype, _, err := mime.ParseMediaType(contentType)
+	if err != nil {
+		return ""
+	}
+	return mediatype
+}