From 2d9a50b97f762637627436b6ed153242e43874fb Mon Sep 17 00:00:00 2001 From: Didier Spezia Date: Fri, 8 May 2015 16:38:08 +0000 Subject: [PATCH] html: simplify and optimize escape/unescape The html package uses some specific code to escape special characters. Actually, the strings.Replacer can be used instead, and is much more efficient. The converse operation is more complex but can still be slightly optimized. Credits to Ken Bloom (kabloom@google.com), who first submitted a similar patch at https://codereview.appspot.com/141930043 Added benchmarks and slightly optimized UnescapeString. benchmark old ns/op new ns/op delta BenchmarkEscape-4 118713 19825 -83.30% BenchmarkEscapeNone-4 87653 3784 -95.68% BenchmarkUnescape-4 24888 23417 -5.91% BenchmarkUnescapeNone-4 14423 157 -98.91% benchmark old allocs new allocs delta BenchmarkEscape-4 9 2 -77.78% BenchmarkEscapeNone-4 0 0 +0.00% BenchmarkUnescape-4 2 2 +0.00% BenchmarkUnescapeNone-4 0 0 +0.00% benchmark old bytes new bytes delta BenchmarkEscape-4 24800 12288 -50.45% BenchmarkEscapeNone-4 0 0 +0.00% BenchmarkUnescape-4 10240 10240 +0.00% BenchmarkUnescapeNone-4 0 0 +0.00% Fixes #8697 Change-Id: I208261ed7cbe9b3dee6317851f8c0cf15528bce4 Reviewed-on: https://go-review.googlesource.com/9808 Run-TryBot: Brad Fitzpatrick Reviewed-by: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/html/escape.go | 57 ++++++++--------------------------------- src/html/escape_test.go | 40 ++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 47 deletions(-) diff --git a/src/html/escape.go b/src/html/escape.go index dd5dfa7cd7..f50a4b937a 100644 --- a/src/html/escape.go +++ b/src/html/escape.go @@ -6,7 +6,6 @@ package html import ( - "bytes" "strings" "unicode/utf8" ) @@ -187,52 +186,20 @@ func unescape(b []byte) []byte { return b } -const escapedChars = `&'<>"` - -func escape(w writer, s string) error { - i := strings.IndexAny(s, escapedChars) - for i != -1 { - if _, err := w.WriteString(s[:i]); err != nil { - return err - } - var esc string - switch s[i] { - case '&': - esc = "&" - case '\'': - // "'" is shorter than "'" and apos was not in HTML until HTML5. - esc = "'" - case '<': - esc = "<" - case '>': - esc = ">" - case '"': - // """ is shorter than """. - esc = """ - default: - panic("unrecognized escape character") - } - s = s[i+1:] - if _, err := w.WriteString(esc); err != nil { - return err - } - i = strings.IndexAny(s, escapedChars) - } - _, err := w.WriteString(s) - return err -} +var htmlEscaper = strings.NewReplacer( + `&`, "&", + `'`, "'", // "'" is shorter than "'" and apos was not in HTML until HTML5. + `<`, "<", + `>`, ">", + `"`, """, // """ is shorter than """. +) // EscapeString escapes special characters like "<" to become "<". It // escapes only five such characters: <, >, &, ' and ". // UnescapeString(EscapeString(s)) == s always holds, but the converse isn't // always true. func EscapeString(s string) string { - if strings.IndexAny(s, escapedChars) == -1 { - return s - } - var buf bytes.Buffer - escape(&buf, s) - return buf.String() + return htmlEscaper.Replace(s) } // UnescapeString unescapes entities like "<" to become "<". It unescapes a @@ -241,10 +208,8 @@ func EscapeString(s string) string { // UnescapeString(EscapeString(s)) == s always holds, but the converse isn't // always true. func UnescapeString(s string) string { - for _, c := range s { - if c == '&' { - return string(unescape([]byte(s))) - } + if !strings.Contains(s, "&") { + return s } - return s + return string(unescape([]byte(s))) } diff --git a/src/html/escape_test.go b/src/html/escape_test.go index 2d7ad8ac26..3702626a3d 100644 --- a/src/html/escape_test.go +++ b/src/html/escape_test.go @@ -4,7 +4,10 @@ package html -import "testing" +import ( + "strings" + "testing" +) type unescapeTest struct { // A short description of the test case. @@ -113,3 +116,38 @@ func TestUnescapeEscape(t *testing.T) { } } } + +var ( + benchEscapeData = strings.Repeat("AAAAA < BBBBB > CCCCC & DDDDD ' EEEEE \" ", 100) + benchEscapeNone = strings.Repeat("AAAAA x BBBBB x CCCCC x DDDDD x EEEEE x ", 100) +) + +func BenchmarkEscape(b *testing.B) { + n := 0 + for i := 0; i < b.N; i++ { + n += len(EscapeString(benchEscapeData)) + } +} + +func BenchmarkEscapeNone(b *testing.B) { + n := 0 + for i := 0; i < b.N; i++ { + n += len(EscapeString(benchEscapeNone)) + } +} + +func BenchmarkUnescape(b *testing.B) { + s := EscapeString(benchEscapeData) + n := 0 + for i := 0; i < b.N; i++ { + n += len(UnescapeString(s)) + } +} + +func BenchmarkUnescapeNone(b *testing.B) { + s := EscapeString(benchEscapeNone) + n := 0 + for i := 0; i < b.N; i++ { + n += len(UnescapeString(s)) + } +}