diff --git a/css/handlers.go b/css/handlers.go index 1c2d6f0..6550d10 100644 --- a/css/handlers.go +++ b/css/handlers.go @@ -337,32 +337,16 @@ func multiSplit(value string, seps ...string) []string { return curArray } -// recursiveCheck reports whether value can be split, in order, into groups -// of tokens each accepted by one of funcs. The outcome depends only on how -// many tokens remain, so memoizing on that length keeps the search -// polynomial without changing which values are accepted. func recursiveCheck(value []string, funcs []func(string) bool) bool { - memo := make(map[int]bool, len(value)) - return recursiveCheckMemo(value, funcs, memo) -} - -func recursiveCheckMemo(value []string, funcs []func(string) bool, memo map[int]bool) bool { - if cached, ok := memo[len(value)]; ok { - return cached - } - result := false -outer: for i := 0; i < len(value); i++ { tempVal := strings.Join(value[:i+1], " ") for _, j := range funcs { - if j(tempVal) && (len(value[i+1:]) == 0 || recursiveCheckMemo(value[i+1:], funcs, memo)) { - result = true - break outer + if j(tempVal) && (len(value[i+1:]) == 0 || recursiveCheck(value[i+1:], funcs)) { + return true } } } - memo[len(value)] = result - return result + return false } func in(value []string, arr []string) bool { diff --git a/css/handlers_test.go b/css/handlers_test.go deleted file mode 100644 index 295e83f..0000000 --- a/css/handlers_test.go +++ /dev/null @@ -1,89 +0,0 @@ -// Copyright (c) 2019, David Kitchen -// -// All rights reserved. -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are met: -// -// * Redistributions of source code must retain the above copyright notice, this -// list of conditions and the following disclaimer. -// -// * Redistributions in binary form must reproduce the above copyright notice, -// this list of conditions and the following disclaimer in the documentation -// and/or other materials provided with the distribution. -// -// * Neither the name of the organisation (Microcosm) nor the names of its -// contributors may be used to endorse or promote products derived from -// this software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" -// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE -// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE -// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE -// FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL -// DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR -// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER -// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, -// OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -package css - -import ( - "strings" - "testing" - "time" -) - -// Regression test for https://github.com/microcosm-cc/bluemonday/issues/225. -// Whitespace-only tokens give recursiveCheck many equally valid split points; -// unmemoized it re-explored each one, taking ~4s for the value below. The -// verdict is unchanged (rejected either way), only the time to reach it. -func TestIssue225(t *testing.T) { - // 20 blank tokens, as the issue #225 payload produced them. - pathological := "normal\n" + strings.Repeat(" ", 20) + "16px\nroboto" - - const deadline = 3 * time.Second - const want = false - - result := make(chan bool, 1) - go func() { - result <- FontHandler(pathological) - }() - - select { - case got := <-result: - if got != want { - t.Errorf("FontHandler(%q) = %v, want %v", pathological, got, want) - } - case <-time.After(deadline): - t.Fatalf("FontHandler did not return within %v for a 20-blank-token "+ - "value; recursiveCheck may have regressed to exponential-time "+ - "behaviour (see issue #225)", deadline) - } -} - -// Memoization must not change which values FontHandler accepts, only how -// fast it decides. Expectations captured from the pre-fix behaviour. -func TestIssue225AcceptanceUnchanged(t *testing.T) { - tests := []struct { - value string - want bool - }{ - // Valid "font" shorthand values. - {"italic bold 12px/30px Georgia, serif", true}, - {"icon", true}, - {"caption", true}, - {"normal 16px Roboto", true}, - {"bold 16px/1.5 Arial, sans-serif", true}, - // Invalid/unsafe values that must stay rejected. - {"expression(alert(1))", false}, - {"javascript:alert(1)", false}, - {"url(evil.com)", false}, - } - for _, tt := range tests { - if got := FontHandler(tt.value); got != tt.want { - t.Errorf("FontHandler(%q) = %v, want %v", tt.value, got, tt.want) - } - } -}