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

New Issue

GiteaMirror · 2025-11-02T05:59:28-06:00

GiteaMirror commented

2025-11-02 05:59:28 -06:00

Originally created by @albertjin on GitHub (Jan 20, 2020).

ValueWithShadows() called in function newMarkupSanitizer() (file modules/setting/markup.go) always returns one element.

For example, the following section in app.ini means "rel" attribute is allowed for "a" element while "target" attribute is still ignored.

[markup.sanitizer]
ELEMENT = a
ALLOW_ATTR = target
REGEXP =
ELEMENT = a
ALLOW_ATTR = rel
REGEXP =

Originally created by @albertjin on GitHub (Jan 20, 2020). `ValueWithShadows()` called in function `newMarkupSanitizer()` (file `modules/setting/markup.go`) always returns one element. For example, the following section in `app.ini` means "rel" attribute is allowed for "a" element while "target" attribute is still ignored. ``` [markup.sanitizer] ELEMENT = a ALLOW_ATTR = target REGEXP = ELEMENT = a ALLOW_ATTR = rel REGEXP = ```

GiteaMirror closed this issue

2025-11-02 05:59:28 -06:00

GiteaMirror commented

2025-11-02 05:59:29 -06:00

@albertjin commented on GitHub (Jan 20, 2020):

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.

@albertjin commented on GitHub (Jan 20, 2020): 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. ```go 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. ```

GiteaMirror commented

2025-11-02 05:59:29 -06:00

@zeripath commented on GitHub (Jan 20, 2020):

I suspect shadowload here isn't right. We still need something that is going to append but with shadows

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

GiteaMirror commented

2025-11-02 05:59:29 -06:00

@zeripath commented on GitHub (Jan 20, 2020):

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

7d7ab1eeae/modules/setting/setting.go (L520)

With:

ini.ShadowLoad([]byte{})

@zeripath commented on GitHub (Jan 20, 2020): I think We need to replace the Empty() here: https://github.com/go-gitea/gitea/blob/7d7ab1eeae43d99fe329878ac9c8db5e45e2dee5/modules/setting/setting.go#L520 With: ```go ini.ShadowLoad([]byte{}) ```

GiteaMirror commented

2025-11-02 05:59:29 -06:00

@albertjin commented on GitHub (Jan 20, 2020):

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

7d7ab1eeae/modules/setting/setting.go (L520)

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 =

@albertjin commented on GitHub (Jan 20, 2020): > I think We need to replace the Empty() here: > > https://github.com/go-gitea/gitea/blob/7d7ab1eeae43d99fe329878ac9c8db5e45e2dee5/modules/setting/setting.go#L520 > > With: > > ```go > 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 = ```

GiteaMirror commented

2025-11-02 05:59:30 -06:00

@zeripath commented on GitHub (Jan 20, 2020):

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.

@zeripath commented on GitHub (Jan 20, 2020): 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.

GiteaMirror commented

2025-11-02 05:59:30 -06:00

@cipherboy commented on GitHub (Mar 9, 2020):

@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.

@cipherboy commented on GitHub (Mar 9, 2020): @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: ```ini [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.

GiteaMirror commented

2025-11-02 05:59:30 -06:00

@zeripath commented on GitHub (Mar 28, 2020):

@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

@zeripath commented on GitHub (Mar 28, 2020): @cipherboy sorry it's taken me so long to reply to this. I think the best option would actually be: ```ini [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

GiteaMirror commented

2025-11-02 05:59:30 -06:00

@zeripath commented on GitHub (May 12, 2020):

Fixed by #11133

@zeripath commented on GitHub (May 12, 2020): Fixed by #11133

GiteaMirror referenced this issue

2025-11-02 13:33:11 -06:00

[PR #4686] [MERGED] Backport: Hide org/create menu item in Dashboard if user has no rights (#4678) #17464

Sign in to join this conversation.