From e2a3ec4c3dbc537d10ed872b31c2ed693740b3fa Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 31 Dec 2015 12:12:16 -0700 Subject: [PATCH 1/2] Replacer supports case-insensitive header placeholders (fixes #476) --- middleware/replacer.go | 43 ++++++++----- middleware/replacer_test.go | 120 ++++++++++++++++++------------------ 2 files changed, 90 insertions(+), 73 deletions(-) diff --git a/middleware/replacer.go b/middleware/replacer.go index 193fd4fcd..10f0e35af 100644 --- a/middleware/replacer.go +++ b/middleware/replacer.go @@ -86,9 +86,9 @@ func NewReplacer(r *http.Request, rr *responseRecorder, emptyValue string) Repla rep.replacements["{latency}"] = time.Since(rr.start).String() } - // Header placeholders - for header, val := range r.Header { - rep.replacements[headerReplacer+header+"}"] = strings.Join(val, ",") + // Header placeholders (case-insensitive) + for header, values := range r.Header { + rep.replacements[headerReplacer+strings.ToLower(header)+"}"] = strings.Join(values, ",") } return rep @@ -97,6 +97,32 @@ func NewReplacer(r *http.Request, rr *responseRecorder, emptyValue string) Repla // Replace performs a replacement of values on s and returns // the string with the replaced values. func (r replacer) Replace(s string) string { + // Header replacements - these are case-insensitive, so we can't just use strings.Replace() + startPos := strings.Index(s, headerReplacer) + for startPos > -1 { + // carefully find end of placeholder + endOffset := strings.Index(s[startPos+1:], "}") + if endOffset == -1 { + startPos = strings.Index(s[startPos+len(headerReplacer):], headerReplacer) + continue + } + endPos := startPos + len(headerReplacer) + endOffset + + // look for replacement, case-insensitive + placeholder := strings.ToLower(s[startPos:endPos]) + replacement := r.replacements[placeholder] + if replacement == "" { + replacement = r.emptyValue + } + + // do the replacement manually + s = s[:startPos] + replacement + s[endPos:] + + // move to next one + startPos = strings.Index(s[endOffset:], headerReplacer) + } + + // Regular replacements - these are easier because they're case-sensitive for placeholder, replacement := range r.replacements { if replacement == "" { replacement = r.emptyValue @@ -104,17 +130,6 @@ func (r replacer) Replace(s string) string { s = strings.Replace(s, placeholder, replacement, -1) } - // Replace any header placeholders that weren't found - for strings.Contains(s, headerReplacer) { - idxStart := strings.Index(s, headerReplacer) - endOffset := idxStart + len(headerReplacer) - idxEnd := strings.Index(s[endOffset:], "}") - if idxEnd > -1 { - s = s[:idxStart] + r.emptyValue + s[endOffset+idxEnd+1:] - } else { - break - } - } return s } diff --git a/middleware/replacer_test.go b/middleware/replacer_test.go index 8f1147dea..46d7b8f90 100644 --- a/middleware/replacer_test.go +++ b/middleware/replacer_test.go @@ -10,102 +10,104 @@ import ( func TestNewReplacer(t *testing.T) { w := httptest.NewRecorder() recordRequest := NewResponseRecorder(w) - userJSON := `{"username": "dennis"}` + reader := strings.NewReader(`{"username": "dennis"}`) - reader := strings.NewReader(userJSON) //Convert string to reader - - request, err := http.NewRequest("POST", "http://caddyserver.com", reader) //Create request with JSON body + request, err := http.NewRequest("POST", "http://localhost", reader) if err != nil { - t.Fatalf("Request Formation Failed \n") + t.Fatal("Request Formation Failed\n") } replaceValues := NewReplacer(request, recordRequest, "") switch v := replaceValues.(type) { case replacer: - if v.replacements["{host}"] != "caddyserver.com" { - t.Errorf("Expected host to be caddyserver.com") + if v.replacements["{host}"] != "localhost" { + t.Error("Expected host to be localhost") } if v.replacements["{method}"] != "POST" { - t.Errorf("Expected request method to be POST") + t.Error("Expected request method to be POST") } if v.replacements["{status}"] != "200" { - t.Errorf("Expected status to be 200") + t.Error("Expected status to be 200") } default: - t.Fatalf("Return Value from New Replacer expected pass type assertion into a replacer type \n") + t.Fatal("Return Value from New Replacer expected pass type assertion into a replacer type\n") } } func TestReplace(t *testing.T) { w := httptest.NewRecorder() recordRequest := NewResponseRecorder(w) - userJSON := `{"username": "dennis"}` + reader := strings.NewReader(`{"username": "dennis"}`) - reader := strings.NewReader(userJSON) //Convert string to reader - - request, err := http.NewRequest("POST", "http://caddyserver.com", reader) //Create request with JSON body + request, err := http.NewRequest("POST", "http://localhost", reader) if err != nil { - t.Fatalf("Request Formation Failed \n") + t.Fatal("Request Formation Failed\n") } - replaceValues := NewReplacer(request, recordRequest, "") + request.Header.Set("Custom", "fooBar") + repl := NewReplacer(request, recordRequest, "-") - switch v := replaceValues.(type) { - case replacer: - - if v.Replace("This host is {host}") != "This host is caddyserver.com" { - t.Errorf("Expected host replacement failed") - } - if v.Replace("This request method is {method}") != "This request method is POST" { - t.Errorf("Expected method replacement failed") - } - if v.Replace("The response status is {status}") != "The response status is 200" { - t.Errorf("Expected status replacement failed") - } - - default: - t.Fatalf("Return Value from New Replacer expected pass type assertion into a replacer type \n") + if expected, actual := "This host is localhost.", repl.Replace("This host is {host}."); expected != actual { + t.Errorf("{host} replacement: expected '%s', got '%s'", expected, actual) + } + if expected, actual := "This request method is POST.", repl.Replace("This request method is {method}."); expected != actual { + t.Errorf("{method} replacement: expected '%s', got '%s'", expected, actual) + } + if expected, actual := "The response status is 200.", repl.Replace("The response status is {status}."); expected != actual { + t.Errorf("{status} replacement: expected '%s', got '%s'", expected, actual) + } + if expected, actual := "The Custom header is fooBar.", repl.Replace("The Custom header is {>Custom}."); expected != actual { + t.Errorf("{>Custom} replacement: expected '%s', got '%s'", expected, actual) } + // Test header case-insensitivity + if expected, actual := "The cUsToM header is fooBar...", repl.Replace("The cUsToM header is {>cUsToM}..."); expected != actual { + t.Errorf("{>cUsToM} replacement: expected '%s', got '%s'", expected, actual) + } + + // Test non-existent header/value + if expected, actual := "The Non-Existent header is -.", repl.Replace("The Non-Existent header is {>Non-Existent}."); expected != actual { + t.Errorf("{>Non-Existent} replacement: expected '%s', got '%s'", expected, actual) + } + + // Test bad placeholder + if expected, actual := "Bad {host placeholder...", repl.Replace("Bad {host placeholder..."); expected != actual { + t.Errorf("bad placeholder: expected '%s', got '%s'", expected, actual) + } + + // Test bad header placeholder + if expected, actual := "Bad {>Custom placeholder", repl.Replace("Bad {>Custom placeholder"); expected != actual { + t.Errorf("bad header placeholder: expected '%s', got '%s'", expected, actual) + } } func TestSet(t *testing.T) { w := httptest.NewRecorder() recordRequest := NewResponseRecorder(w) - userJSON := `{"username": "dennis"}` + reader := strings.NewReader(`{"username": "dennis"}`) - reader := strings.NewReader(userJSON) //Convert string to reader - - request, err := http.NewRequest("POST", "http://caddyserver.com", reader) //Create request with JSON body + request, err := http.NewRequest("POST", "http://localhost", reader) if err != nil { t.Fatalf("Request Formation Failed \n") } - replaceValues := NewReplacer(request, recordRequest, "") + repl := NewReplacer(request, recordRequest, "") - replaceValues.Set("host", "getcaddy.com") - replaceValues.Set("method", "GET") - replaceValues.Set("status", "201") - replaceValues.Set("variable", "value") + repl.Set("host", "getcaddy.com") + repl.Set("method", "GET") + repl.Set("status", "201") + repl.Set("variable", "value") - switch v := replaceValues.(type) { - case replacer: - - if v.Replace("This host is {host}") != "This host is getcaddy.com" { - t.Errorf("Expected host replacement failed") - } - if v.Replace("This request method is {method}") != "This request method is GET" { - t.Errorf("Expected method replacement failed") - } - if v.Replace("The response status is {status}") != "The response status is 201" { - t.Errorf("Expected status replacement failed") - } - if v.Replace("The value of variable is {variable}") != "The value of variable is value" { - t.Errorf("Expected status replacement failed") - } - - default: - t.Fatalf("Return Value from New Replacer expected pass type assertion into a replacer type \n") + if repl.Replace("This host is {host}") != "This host is getcaddy.com" { + t.Error("Expected host replacement failed") + } + if repl.Replace("This request method is {method}") != "This request method is GET" { + t.Error("Expected method replacement failed") + } + if repl.Replace("The response status is {status}") != "The response status is 201" { + t.Error("Expected status replacement failed") + } + if repl.Replace("The value of variable is {variable}") != "The value of variable is value" { + t.Error("Expected variable replacement failed") } - } From b6326d402d3025084cce6b28b1299506aaaa838c Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 31 Dec 2015 12:31:30 -0700 Subject: [PATCH 2/2] Fix for case-insensitive header replacements (#476) --- middleware/replacer.go | 34 +++++++++++++--------------------- middleware/replacer_test.go | 17 ++++++++++++++--- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/middleware/replacer.go b/middleware/replacer.go index 10f0e35af..8a3d202a2 100644 --- a/middleware/replacer.go +++ b/middleware/replacer.go @@ -98,28 +98,20 @@ func NewReplacer(r *http.Request, rr *responseRecorder, emptyValue string) Repla // the string with the replaced values. func (r replacer) Replace(s string) string { // Header replacements - these are case-insensitive, so we can't just use strings.Replace() - startPos := strings.Index(s, headerReplacer) - for startPos > -1 { - // carefully find end of placeholder - endOffset := strings.Index(s[startPos+1:], "}") - if endOffset == -1 { - startPos = strings.Index(s[startPos+len(headerReplacer):], headerReplacer) - continue + for strings.Contains(s, headerReplacer) { + idxStart := strings.Index(s, headerReplacer) + endOffset := idxStart + len(headerReplacer) + idxEnd := strings.Index(s[endOffset:], "}") + if idxEnd > -1 { + placeholder := strings.ToLower(s[idxStart : endOffset+idxEnd+1]) + replacement := r.replacements[placeholder] + if replacement == "" { + replacement = r.emptyValue + } + s = s[:idxStart] + replacement + s[endOffset+idxEnd+1:] + } else { + break } - endPos := startPos + len(headerReplacer) + endOffset - - // look for replacement, case-insensitive - placeholder := strings.ToLower(s[startPos:endPos]) - replacement := r.replacements[placeholder] - if replacement == "" { - replacement = r.emptyValue - } - - // do the replacement manually - s = s[:startPos] + replacement + s[endPos:] - - // move to next one - startPos = strings.Index(s[endOffset:], headerReplacer) } // Regular replacements - these are easier because they're case-sensitive diff --git a/middleware/replacer_test.go b/middleware/replacer_test.go index 46d7b8f90..d98bd2de1 100644 --- a/middleware/replacer_test.go +++ b/middleware/replacer_test.go @@ -45,7 +45,8 @@ func TestReplace(t *testing.T) { if err != nil { t.Fatal("Request Formation Failed\n") } - request.Header.Set("Custom", "fooBar") + request.Header.Set("Custom", "foobarbaz") + request.Header.Set("ShorterVal", "1") repl := NewReplacer(request, recordRequest, "-") if expected, actual := "This host is localhost.", repl.Replace("This host is {host}."); expected != actual { @@ -57,12 +58,12 @@ func TestReplace(t *testing.T) { if expected, actual := "The response status is 200.", repl.Replace("The response status is {status}."); expected != actual { t.Errorf("{status} replacement: expected '%s', got '%s'", expected, actual) } - if expected, actual := "The Custom header is fooBar.", repl.Replace("The Custom header is {>Custom}."); expected != actual { + if expected, actual := "The Custom header is foobarbaz.", repl.Replace("The Custom header is {>Custom}."); expected != actual { t.Errorf("{>Custom} replacement: expected '%s', got '%s'", expected, actual) } // Test header case-insensitivity - if expected, actual := "The cUsToM header is fooBar...", repl.Replace("The cUsToM header is {>cUsToM}..."); expected != actual { + if expected, actual := "The cUsToM header is foobarbaz...", repl.Replace("The cUsToM header is {>cUsToM}..."); expected != actual { t.Errorf("{>cUsToM} replacement: expected '%s', got '%s'", expected, actual) } @@ -80,6 +81,16 @@ func TestReplace(t *testing.T) { if expected, actual := "Bad {>Custom placeholder", repl.Replace("Bad {>Custom placeholder"); expected != actual { t.Errorf("bad header placeholder: expected '%s', got '%s'", expected, actual) } + + // Test bad header placeholder with valid one later + if expected, actual := "Bad -", repl.Replace("Bad {>Custom placeholder {>ShorterVal}"); expected != actual { + t.Errorf("bad header placeholders: expected '%s', got '%s'", expected, actual) + } + + // Test shorter header value with multiple placeholders + if expected, actual := "Short value 1 then foobarbaz.", repl.Replace("Short value {>ShorterVal} then {>Custom}."); expected != actual { + t.Errorf("short value: expected '%s', got '%s'", expected, actual) + } } func TestSet(t *testing.T) {