Merge pull request #47 from rollbar/force-content-type-single-valued

Disallow multiple values for Content-Type header

rollbar_test.go

@@ -220,6 +220,46 @@ func TestErrorRequest(t *testing.T) {
 	}
 }
 
+func TestErrorRequestHeaders(t *testing.T) {
+	r, _ := http.NewRequest("GET", "http://foo.com/somethere?param1=true", nil)
+	r.RemoteAddr = "1.1.1.1:123"
+	r.Header.Add("Content-Type", "application/json")
+	r.Header.Add("Content-Type", "application/x-json")
+	r.Header.Add("X-Foo-Bar", "baz")
+	r.Header.Add("X-Mult", "a")
+	r.Header.Add("X-Mult", "b")
+
+	object := std.requestDetails(r)
+
+	if object["url"] != "http://foo.com/somethere?param1=true" {
+		t.Errorf("wrong url, got %v", object["url"])
+	}
+
+	if object["method"] != "GET" {
+		t.Errorf("wrong method, got %v", object["method"])
+	}
+
+	if object["query_string"] != "param1=true" {
+		t.Errorf("wrong id, got %v", object["query_string"])
+	}
+
+	headers := object["headers"].(map[string]interface{})
+	if headers["Content-Type"].(string) != "application/json" {
+		t.Errorf("expected single string value for Content-Type, got %v", headers["Content-Type"])
+	}
+	if headers["X-Foo-Bar"].(string) != "baz" {
+		t.Errorf("expected single string value for X-Foo-Bar, got %v", headers["X-Foo-Bar"])
+	}
+	if len(headers["X-Mult"].([]string)) != 2 {
+		t.Errorf("expected X-Mult to have two string values, got %v", headers["X-Mult"])
+	}
+
+	multHeaders := headers["X-Mult"].([]string)
+	if multHeaders[0] != "a" || multHeaders[1] != "b" {
+		t.Errorf("expected multiple string values for X-Mult, got %v", headers["X-Mult"])
+	}
+}
+
 func TestFilterParams(t *testing.T) {
 	values := map[string][]string{
 		"password":     {"one"},
@@ -257,6 +297,67 @@ func TestFlattenValues(t *testing.T) {
 	}
 }
 
+func TestFilterFlatten(t *testing.T) {
+	values := map[string][]string{
+		"password":     {"one"},
+		"ok":           {"one", "two"},
+		"access_token": {"one", "two"},
+		"thing":        {"foo", "bar"},
+		"a":            {"single"},
+		"b":            {"more", "than", "one"},
+	}
+
+	clean := filterFlatten(std.configuration.scrubFields, values, nil)
+	if clean["password"] != FILTERED {
+		t.Error("should filter password parameter")
+	}
+
+	if clean["ok"] == FILTERED {
+		t.Error("should keep ok parameter")
+	}
+
+	if len(clean["ok"].([]string)) != 2 {
+		t.Error("should not flatten ok parameter")
+	}
+
+	if clean["access_token"] != FILTERED {
+		t.Error("should filter access_token parameter")
+	}
+
+	special := map[string]struct{}{
+		"thing": struct{}{},
+	}
+
+	clean2 := filterFlatten(std.configuration.scrubFields, values, special)
+	if clean2["password"] != FILTERED {
+		t.Error("should filter password parameter")
+	}
+
+	if clean2["ok"] == FILTERED {
+		t.Error("should keep ok parameter")
+	}
+
+	if len(clean2["ok"].([]string)) != 2 {
+		t.Error("should not flatten ok parameter")
+	}
+
+	if clean2["access_token"] != FILTERED {
+		t.Error("should filter access_token parameter")
+	}
+
+	if clean2["thing"] != "foo" {
+		t.Error("should force flatten a special key")
+	}
+
+	if clean2["a"].(string) != "single" {
+		t.Error("should flatten single parameter to string")
+	}
+
+	if len(clean2["b"].([]string)) != 3 {
+		t.Error("should leave multiple parametres as []string")
+	}
+}
+
 type cs struct {
 	error
 	cause error

transforms.go

@@ -84,22 +84,51 @@ func addErrorToBody(configuration configuration, body map[string]interface{}, er
 
 func requestDetails(configuration configuration, r *http.Request) map[string]interface{} {
 	cleanQuery := filterParams(configuration.scrubFields, r.URL.Query())
+	specialHeaders := map[string]struct{}{
+		"Content-Type": struct{}{},
+	}
 
 	return map[string]interface{}{
 		"url":     r.URL.String(),
 		"method":  r.Method,
-		"headers": flattenValues(filterParams(configuration.scrubHeaders, r.Header)),
+		"headers": filterFlatten(configuration.scrubHeaders, r.Header, specialHeaders),
 
 		// GET params
 		"query_string": url.Values(cleanQuery).Encode(),
 		"GET":          flattenValues(cleanQuery),
 
 		// POST / PUT params
-		"POST":    flattenValues(filterParams(configuration.scrubFields, r.Form)),
+		"POST":    filterFlatten(configuration.scrubFields, r.Form, nil),
 		"user_ip": filterIp(r.RemoteAddr, configuration.captureIp),
 	}
 }
 
+// filterFlatten filters sensitive information like passwords from being sent to Rollbar, and
+// also lifts any values with length one up to be a standalone string. The optional specialKeys map
+// will force strings that exist in that map and also in values to have a single string value in the
+// resulting map by taking the first element in the list of strings if there are more than one.
+// This is essentially the same as the composition of filterParams and filterValues, plus the bit
+// extra about the special keys. The composition would range of the values twice when we really only
+// need to do it once, so I decided to combine them as the result is still quite easy to follow.
+// We keep the other two so that we can use url.Values.Encode on the filtered query params and not
+// run the filtering twice for the query.
+func filterFlatten(pattern *regexp.Regexp, values map[string][]string, specialKeys map[string]struct{}) map[string]interface{} {
+	result := make(map[string]interface{})
+
+	for k, v := range values {
+		switch _, special := specialKeys[k]; {
+		case pattern.Match([]byte(k)):
+			result[k] = FILTERED
+		case special || len(v) == 1:
+			result[k] = v[0]
+		default:
+			result[k] = v
+		}
+	}
+
+	return result
+}
+
 // filterParams filters sensitive information like passwords from being sent to
 // Rollbar.
 func filterParams(pattern *regexp.Regexp, values map[string][]string) map[string][]string {