WIP: Allow all characters in title attribute when rendering #10371
Conversation
According to the common mark spec: https://spec.commonmark.org/0.29/#link-title You should be able to stick abut anything in there. Right now we're limited by the default setting of bluemonday which restricts what can go in a title tag and makes our rendering not compliant with the spec. This should fix go-gitea#10326
|
Could also perhaps only be for render-content class ? Would that cover all cases where we render something? |
GiteaBot
added
the
lgtm/need 2
GiteaBot
added
lgtm/need 1
e-mails? HTML parsers of certain Outloo***ehem mail user agents is buggy to say the least. |
There was a problem hiding this comment.
From my testing this seemed to work fine as well.
My only other concern is having to remember this if we ever use a jQuery/CSS selector based on a title.
Though I don't know of a reason we would.
Would like someone with more experience in security to look at this before approval.
goldmark seems to do a good job of escaping bad strings, though. I wasn't able to escape a markdown link or explicit anchor HTML via the title attribute.
|
This is fixed by #10527 |
According to the common mark spec:
https://spec.commonmark.org/0.29/#link-title
You should be able to stick abut anything in there. Right now we're
limited by the default setting of bluemonday which restricts what can go
in a title tag and makes our rendering not compliant with the spec.
This should fix #10326
Marking WIP to discuss here. According to the commonmark spec just about anything should be allowed in a title attribute. Goldmark will escape things properly when generating links as in the example issue but of course you can just stick raw HTML in as well, try to abuse other links, etc...
Not able to cause problems in limited testing but others should try! FWIW Github seems to let you stick almost anything in there as well (also shown in the example).