Parse media types in the Content-Type header correctly (#964)

* Parse media types in the Content-Type header correctly

Ensures that Content-Type calculation is based on the extracted Content-Type
from the media type, rather than the raw Media Type value. Changes behavior
in the Thrift HTTP endpoint and Zipkin V1 and V2 HTTP endpoints.

Signed-off-by: Chris Zelenak <chris@zelenak.me>

* Ensure Content-Type parse errors are visible

Make sure that callers are informed of errors if they provide an
incorrect media type.

Signed-off-by: Chris Zelenak <chris@zelenak.me>

Author: netshade

cmd/collector/app/http_handler.go

@@ -17,6 +17,7 @@ package app
 import (
 	"fmt"
 	"io/ioutil"
+	"mime"
 	"net/http"
 	"time"
 
@@ -66,7 +67,13 @@ func (aH *APIHandler) saveSpan(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	contentType := r.Header.Get("Content-Type")
+	contentType, _, err := mime.ParseMediaType(r.Header.Get("Content-Type"))
+
+	if err != nil {
+		http.Error(w, fmt.Sprintf("Cannot parse content type: %v", err), http.StatusBadRequest)
+		return
+	}
+
 	if _, ok := acceptedThriftFormats[contentType]; !ok {
 		http.Error(w, fmt.Sprintf("Unsupported content type: %v", contentType), http.StatusBadRequest)
 		return

cmd/collector/app/http_handler_test.go

@@ -83,6 +83,11 @@ func TestThriftFormat(t *testing.T) {
 	assert.EqualValues(t, http.StatusAccepted, statusCode)
 	assert.EqualValues(t, "", resBodyStr)
 
+	statusCode, resBodyStr, err = postBytes("application/x-thrift; charset=utf-8", server.URL+`/api/traces`, someBytes)
+	assert.NoError(t, err)
+	assert.EqualValues(t, http.StatusAccepted, statusCode)
+	assert.EqualValues(t, "", resBodyStr)
+
 	handler.jaegerBatchesHandler.(*mockJaegerHandler).err = fmt.Errorf("Bad times ahead")
 	statusCode, resBodyStr, err = postBytes("application/vnd.apache.thrift.binary", server.URL+`/api/traces`, someBytes)
 	assert.NoError(t, err)
@@ -143,6 +148,15 @@ func TestWrongFormat(t *testing.T) {
 	assert.EqualValues(t, "Unsupported content type: nosoupforyou\n", resBodyStr)
 }
 
+func TestMalformedFormat(t *testing.T) {
+	server, _ := initializeTestServer(nil)
+	defer server.Close()
+	statusCode, resBodyStr, err := postBytes("application/json; =iammalformed", server.URL+`/api/traces`, []byte{})
+	assert.NoError(t, err)
+	assert.EqualValues(t, http.StatusBadRequest, statusCode)
+	assert.EqualValues(t, "Cannot parse content type: mime: invalid media parameter\n", resBodyStr)
+}
+
 func TestCannotReadBodyFromRequest(t *testing.T) {
 	handler := NewAPIHandler(&mockJaegerHandler{})
 	req, err := http.NewRequest(http.MethodPost, "whatever", &errReader{})

cmd/collector/app/zipkin/http_handler.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"mime"
 	"net/http"
 	"strings"
 	"time"
@@ -79,7 +80,13 @@ func (aH *APIHandler) saveSpans(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	contentType := r.Header.Get("Content-Type")
+	contentType, _, err := mime.ParseMediaType(r.Header.Get("Content-Type"))
+
+	if err != nil {
+		http.Error(w, fmt.Sprintf("Cannot parse Content-Type: %v", err), http.StatusBadRequest)
+		return
+	}
+
 	var tSpans []*zipkincore.Span
 	if contentType == "application/x-thrift" {
 		tSpans, err = deserializeThrift(bodyBytes)
@@ -121,7 +128,13 @@ func (aH *APIHandler) saveSpansV2(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	contentType := r.Header.Get("Content-Type")
+	contentType, _, err := mime.ParseMediaType(r.Header.Get("Content-Type"))
+
+	if err != nil {
+		http.Error(w, fmt.Sprintf("Cannot parse Content-Type: %v", err), http.StatusBadRequest)
+		return
+	}
+
 	if contentType != "application/json" {
 		http.Error(w, "Unsupported Content-Type", http.StatusBadRequest)
 		return

cmd/collector/app/zipkin/http_handler_test.go

@@ -131,6 +131,11 @@ func TestJsonFormat(t *testing.T) {
 	require.NotNil(t, recdSpan.Annotations[0].Host)
 	assert.EqualValues(t, -1, recdSpan.Annotations[0].Host.Port, "Port 65535 must be represented as -1 in zipkin.thrift")
 
+	statusCode, resBodyStr, err = postBytes(server.URL+`/api/v1/spans`, []byte(spanJSON), createHeader("application/json; charset=utf-8"))
+	assert.NoError(t, err)
+	assert.EqualValues(t, http.StatusAccepted, statusCode)
+	assert.EqualValues(t, "", resBodyStr)
+
 	endpErrJSON := createEndpoint("", "127.0.0.A", "", 80)
 
 	// error zipkinSpanHandler
@@ -193,6 +198,15 @@ func TestGzipBadBody(t *testing.T) {
 	assert.EqualValues(t, "Unable to process request body: unexpected EOF\n", resBodyStr)
 }
 
+func TestMalformedContentType(t *testing.T) {
+	server, _ := initializeTestServer(nil)
+	defer server.Close()
+	statusCode, resBodyStr, err := postBytes(server.URL+`/api/v1/spans`, []byte{}, createHeader("application/json; =iammalformed;"))
+	assert.NoError(t, err)
+	assert.EqualValues(t, http.StatusBadRequest, statusCode)
+	assert.EqualValues(t, "Cannot parse Content-Type: mime: invalid media parameter\n", resBodyStr)
+}
+
 func TestUnsupportedContentType(t *testing.T) {
 	server, _ := initializeTestServer(nil)
 	defer server.Close()
@@ -245,8 +259,10 @@ func TestSaveSpansV2(t *testing.T) {
 		headers map[string]string
 	}{
 		{body: []byte("[]"), code: http.StatusAccepted},
+		{body: []byte("[]"), code: http.StatusAccepted, headers: map[string]string{"Content-Type": "application/json; charset=utf-8"}},
 		{body: gzipEncode([]byte("[]")), code: http.StatusAccepted, headers: map[string]string{"Content-Encoding": "gzip"}},
 		{body: []byte("[]"), code: http.StatusBadRequest, headers: map[string]string{"Content-Type": "text/html"}, resBody: "Unsupported Content-Type\n"},
+		{body: []byte("[]"), code: http.StatusBadRequest, headers: map[string]string{"Content-Type": "application/json; =iammalformed;"}, resBody: "Cannot parse Content-Type: mime: invalid media parameter\n"},
 		{body: []byte("[]"), code: http.StatusBadRequest, headers: map[string]string{"Content-Encoding": "gzip"}, resBody: "Unable to process request body: unexpected EOF\n"},
 		{body: []byte("not good"), code: http.StatusBadRequest, resBody: "Unable to process request body: invalid character 'o' in literal null (expecting 'u')\n"},
 		{body: []byte("[{}]"), code: http.StatusBadRequest, resBody: "Unable to process request body: validation failure list:\nid in body is required\ntraceId in body is required\n"},