markup.sanitizer in app.ini does not support multiple rule entries as expected #9888

Activity

1. albertjin commented on Jan 20, 2020

   

   albertjin

   on Jan 20, 2020

   Last edited by albertjin

   Author

   More actions

   This is related to pull request #9075. After a bit investigation, the problem is related multiple value support in gopkg.in/ini.v1.

   For the function NewContext() in file modules/setting/setting.go at around line 519, change line Cfg.Append(...) to ini.ShadowLoad(...) as follows.

   func NewContext() {
   	Cfg = ini.Empty()
   
   	if len(CustomPID) > 0 {
   		createPIDFile(CustomPID)
   	}
   
   	if com.IsFile(CustomConf) {
   		var err error
   		if Cfg, err = ini.ShadowLoad(CustomConf); err != nil {
   		//if err := Cfg.Append(CustomConf); err != nil {
   			log.Fatal("Failed to load custom conf '%s': %v", CustomConf, err)
   		}
   	} else {

   We'll have error as follows using the example above.

   [E] All three keys in markup.sanitizer (ELEMENT, ALLOW_ATTR, REGEXP) must be defined the same number of times! Got 1, 2, and 1 respectively.
   
2. zeripath commented on Jan 20, 2020

   

   zeripath

   on Jan 20, 2020

   Contributor

   More actions

   I suspect shadowload here isn't right. We still need something that is going to append but with shadows
3. zeripath commented on Jan 20, 2020

   

   zeripath

   on Jan 20, 2020

   Contributor

   More actions

   I think We need to replace the Empty() here:

   gitea/modules/setting/setting.go

   Line 520 in 7d7ab1e

   Cfg = ini.Empty()

   With:

   ini.ShadowLoad([]byte{})
4. albertjin commented on Jan 20, 2020

   

   albertjin

   on Jan 20, 2020

   Author

   More actions

   I think We need to replace the Empty() here:

   gitea/modules/setting/setting.go

   Line 520 in 7d7ab1e

   Cfg = ini.Empty()

   With:

   ini.ShadowLoad([]byte{})

   The same error

   [E] All three keys in markup.sanitizer (ELEMENT, ALLOW_ATTR, REGEXP) must be defined the same number of times! Got 1, 2, and 1 respectively.
   

   for this one,

   [markup.sanitizer]
   ELEMENT = a
   ALLOW_ATTR = target
   REGEXP =
   ELEMENT = a
   ALLOW_ATTR = rel
   REGEXP =
   
5. zeripath commented on Jan 20, 2020

   

   zeripath

   on Jan 20, 2020

   Contributor

   More actions

   Sorry I hadn't read your next issue - I was sorting out the shadow.

   This is never gonna work.

   We need to represent a matched group of values. These just need to be child sections.

   I don't think we can workaround this it needs an actual code change.
6. cipherboy commented on Mar 9, 2020

   

   cipherboy

   on Mar 9, 2020

   Contributor

   More actions

   @albertjin / @zeripath:

   Ah sorry -- I just saw this. Feel free to @mention me next time or drop me an email.

   One idea would be a count parameter:

   [markup.sanitizer]
   RULE_COUNT = 2
   ELEMENT_1 = a
   ALLOW_ATTR_1 = target
   REGEXP_1 = something
   ELEMENT_2 = a
   ALLOW_ATTR_2 = rel
   REGEXP_2 = something

   That'd also solve some of the clarity problems (and omitting a regex would be a blanket whitelist) and keys would now be unique.

   Thoughts? Happy to implement whatever gets decided.
7. zeripath commented on Mar 28, 2020

   

   zeripath

   on Mar 28, 2020

   Contributor

   More actions

   @cipherboy sorry it's taken me so long to reply to this.

   I think the best option would actually be:

   [markup.sanitizer.1]
   ELEMENT=a
   ALLOW_ATTR=target
   REGEXP=something
   [markup.sanitizer.2]
   ELEMENT=a
   ALLOW_ATTR=target
   REGEXP=something

   We can grab all these child sections quite easily from go-ini and even map them quite nicely to objects to result in a simple []*SanitizerRules array
8. zeripath

   added a commit that references this issue on Apr 29, 2020

   Fix sanitizer config - multiple rules (#11133)

   ...

   1bf9e44
9. zeripath commented on May 12, 2020

   

   zeripath

   on May 12, 2020

   Contributor

   More actions

   Fixed by #11133
10. zeripath

    closed this as completedon May 12, 2020
11. ydelafollye

    added a commit that references this issue on Jul 31, 2020

    Fix sanitizer config - multiple rules (go-gitea#11133)

    ...

    7208a2f
12. go-gitea

    locked and limited conversation to collaborators on Nov 24, 2020