From 1bf9e44bda5c8cd1fd72622cffce8ec291db79c5 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Wed, 29 Apr 2020 07:34:59 -0400 Subject: Fix sanitizer config - multiple rules (#11133) In #9888, it was reported that my earlier pull request #9075 didn't quite function as expected. I was quite hopeful the `ValuesWithShadow()` worked as expected (and, I thought my testing showed it did) but I guess not. @zeripath proposed an alternative syntax which I like: ```ini [markup.sanitizer.1] ELEMENT=a ALLOW_ATTR=target REGEXP=something [markup.sanitizer.2] ELEMENT=a ALLOW_ATTR=target REGEXP=something ``` This was quite easy to adopt into the existing code. I've done so in a semi-backwards-compatible manner: - The value from `.Value()` is used for each element. - We parse `[markup.sanitizer]` and all `[markup.sanitizer.*]` sections and add them as rules. This means that existing configs will load one rule (not all rules). It also means people can use string identifiers (`[markup.sanitiser.KaTeX]`) if they prefer, instead of numbered ones. Co-authored-by: Andrew Thornton Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com> --- modules/setting/markup.go | 58 ++++++++++++++++++++--------------------------- 1 file changed, 25 insertions(+), 33 deletions(-) (limited to 'modules') diff --git a/modules/setting/markup.go b/modules/setting/markup.go index 75e6d651bd..1dd76243e6 100644 --- a/modules/setting/markup.go +++ b/modules/setting/markup.go @@ -44,7 +44,7 @@ func newMarkup() { continue } - if name == "sanitizer" { + if name == "sanitizer" || strings.HasPrefix(name, "sanitizer.") { newMarkupSanitizer(name, sec) } else { newMarkupRenderer(name, sec) @@ -67,44 +67,36 @@ func newMarkupSanitizer(name string, sec *ini.Section) { return } - elements := sec.Key("ELEMENT").ValueWithShadows() - allowAttrs := sec.Key("ALLOW_ATTR").ValueWithShadows() - regexps := sec.Key("REGEXP").ValueWithShadows() + elements := sec.Key("ELEMENT").Value() + allowAttrs := sec.Key("ALLOW_ATTR").Value() + regexpStr := sec.Key("REGEXP").Value() - if len(elements) != len(allowAttrs) || - len(elements) != len(regexps) { - log.Error("All three keys in markup.%s (ELEMENT, ALLOW_ATTR, REGEXP) must be defined the same number of times! Got %d, %d, and %d respectively.", name, len(elements), len(allowAttrs), len(regexps)) + if regexpStr == "" { + rule := MarkupSanitizerRule{ + Element: elements, + AllowAttr: allowAttrs, + Regexp: nil, + } + + ExternalSanitizerRules = append(ExternalSanitizerRules, rule) return } - ExternalSanitizerRules = make([]MarkupSanitizerRule, 0, len(elements)) - - for index, pattern := range regexps { - if pattern == "" { - rule := MarkupSanitizerRule{ - Element: elements[index], - AllowAttr: allowAttrs[index], - Regexp: nil, - } - ExternalSanitizerRules = append(ExternalSanitizerRules, rule) - continue - } - - // Validate when parsing the config that this is a valid regular - // expression. Then we can use regexp.MustCompile(...) later. - compiled, err := regexp.Compile(pattern) - if err != nil { - log.Error("In module.%s: REGEXP at definition %d failed to compile: %v", name, index+1, err) - continue - } + // Validate when parsing the config that this is a valid regular + // expression. Then we can use regexp.MustCompile(...) later. + compiled, err := regexp.Compile(regexpStr) + if err != nil { + log.Error("In module.%s: REGEXP (%s) at definition %d failed to compile: %v", regexpStr, name, err) + return + } - rule := MarkupSanitizerRule{ - Element: elements[index], - AllowAttr: allowAttrs[index], - Regexp: compiled, - } - ExternalSanitizerRules = append(ExternalSanitizerRules, rule) + rule := MarkupSanitizerRule{ + Element: elements, + AllowAttr: allowAttrs, + Regexp: compiled, } + + ExternalSanitizerRules = append(ExternalSanitizerRules, rule) } func newMarkupRenderer(name string, sec *ini.Section) { -- cgit v1.2.3