From f1eaae9b0d4a6e37882ba2cd38cc842513c502cf Mon Sep 17 00:00:00 2001 From: Matthew Fay Date: Mon, 19 Mar 2018 12:42:43 +1000 Subject: [PATCH] httpserver: Rework Replacer loop to ignore escaped braces (#2075) * httpserver.Replacer: Rework loop to ignore escaped placeholder braces * Fix typo and ineffectual assignment to ret * Remove redundant idxOffset declaration, simplify escape check * Add benchmark tests for new Replacer code --- caddyhttp/httpserver/replacer.go | 67 ++++++++++++++++++++------- caddyhttp/httpserver/replacer_test.go | 65 ++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 16 deletions(-) diff --git a/caddyhttp/httpserver/replacer.go b/caddyhttp/httpserver/replacer.go index f8c4f8d1..a43b4318 100644 --- a/caddyhttp/httpserver/replacer.go +++ b/caddyhttp/httpserver/replacer.go @@ -141,6 +141,14 @@ func canLogRequest(r *http.Request) bool { return false } +// unescapeBraces finds escaped braces in s and returns +// a string with those braces unescaped. +func unescapeBraces(s string) string { + s = strings.Replace(s, "\\{", "{", -1) + s = strings.Replace(s, "\\}", "}", -1) + return s +} + // Replace performs a replacement of values on s and returns // the string with the replaced values. func (r *replacer) Replace(s string) string { @@ -150,32 +158,59 @@ func (r *replacer) Replace(s string) string { } result := "" +Placeholders: // process each placeholder in sequence for { - idxStart := strings.Index(s, "{") - if idxStart == -1 { - // no placeholder anymore - break - } - idxEnd := strings.Index(s[idxStart:], "}") - if idxEnd == -1 { - // unpaired placeholder - break - } - idxEnd += idxStart + var idxStart, idxEnd int - // get a replacement - placeholder := s[idxStart : idxEnd+1] + idxOffset := 0 + for { // find first unescaped opening brace + searchSpace := s[idxOffset:] + idxStart = strings.Index(searchSpace, "{") + if idxStart == -1 { + // no more placeholders + break Placeholders + } + if idxStart == 0 || searchSpace[idxStart-1] != '\\' { + // preceding character is not an escape + idxStart += idxOffset + break + } + // the brace we found was escaped + // search the rest of the string next + idxOffset += idxStart + 1 + } + + idxOffset = 0 + for { // find first unescaped closing brace + searchSpace := s[idxStart+idxOffset:] + idxEnd = strings.Index(searchSpace, "}") + if idxEnd == -1 { + // unpaired placeholder + break Placeholders + } + if idxEnd == 0 || searchSpace[idxEnd-1] != '\\' { + // preceding character is not an escape + idxEnd += idxOffset + idxStart + break + } + // the brace we found was escaped + // search the rest of the string next + idxOffset += idxEnd + 1 + } + + // get a replacement for the unescaped placeholder + placeholder := unescapeBraces(s[idxStart : idxEnd+1]) replacement := r.getSubstitution(placeholder) - // append prefix + replacement - result += s[:idxStart] + replacement + // append unescaped prefix + replacement + result += strings.TrimPrefix(unescapeBraces(s[:idxStart]), "\\") + replacement // strip out scanned parts s = s[idxEnd+1:] } // append unscanned parts - return result + s + return result + unescapeBraces(s) } func roundDuration(d time.Duration) time.Duration { diff --git a/caddyhttp/httpserver/replacer_test.go b/caddyhttp/httpserver/replacer_test.go index a1127524..a33d4298 100644 --- a/caddyhttp/httpserver/replacer_test.go +++ b/caddyhttp/httpserver/replacer_test.go @@ -112,6 +112,7 @@ func TestReplace(t *testing.T) { {"Query string is {query}", "Query string is foo=bar"}, {"Query string value for foo is {?foo}", "Query string value for foo is bar"}, {"Missing query string argument is {?missing}", "Missing query string argument is "}, + {"\\{ 'hostname': '{hostname}' \\}", "{ 'hostname': '" + hostname + "' }"}, } for _, c := range testCases { @@ -144,6 +145,70 @@ func TestReplace(t *testing.T) { } } +func BenchmarkReplace(b *testing.B) { + w := httptest.NewRecorder() + recordRequest := NewResponseRecorder(w) + reader := strings.NewReader(`{"username": "dennis"}`) + + request, err := http.NewRequest("POST", "http://localhost/?foo=bar", reader) + if err != nil { + b.Fatalf("Failed to make request: %v", err) + } + ctx := context.WithValue(request.Context(), OriginalURLCtxKey, *request.URL) + request = request.WithContext(ctx) + + request.Header.Set("Custom", "foobarbaz") + request.Header.Set("ShorterVal", "1") + repl := NewReplacer(request, recordRequest, "-") + // add some headers after creating replacer + request.Header.Set("CustomAdd", "caddy") + request.Header.Set("Cookie", "foo=bar; taste=delicious") + + // add some respons headers + recordRequest.Header().Set("Custom", "CustomResponseHeader") + + now = func() time.Time { + return time.Date(2006, 1, 2, 15, 4, 5, 02, time.FixedZone("hardcoded", -7)) + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + repl.Replace("This hostname is {hostname}") + } +} + +func BenchmarkReplaceEscaped(b *testing.B) { + w := httptest.NewRecorder() + recordRequest := NewResponseRecorder(w) + reader := strings.NewReader(`{"username": "dennis"}`) + + request, err := http.NewRequest("POST", "http://localhost/?foo=bar", reader) + if err != nil { + b.Fatalf("Failed to make request: %v", err) + } + ctx := context.WithValue(request.Context(), OriginalURLCtxKey, *request.URL) + request = request.WithContext(ctx) + + request.Header.Set("Custom", "foobarbaz") + request.Header.Set("ShorterVal", "1") + repl := NewReplacer(request, recordRequest, "-") + // add some headers after creating replacer + request.Header.Set("CustomAdd", "caddy") + request.Header.Set("Cookie", "foo=bar; taste=delicious") + + // add some respons headers + recordRequest.Header().Set("Custom", "CustomResponseHeader") + + now = func() time.Time { + return time.Date(2006, 1, 2, 15, 4, 5, 02, time.FixedZone("hardcoded", -7)) + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + repl.Replace("\\{ 'hostname': '{hostname}' \\}") + } +} + func TestResponseRecorderNil(t *testing.T) { reader := strings.NewReader(`{"username": "dennis"}`)