Fix some issues with special chars in branch names #3767
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,8 +13,8 @@ | |
| {{else if eq .GetOpType 2}} | ||
| {{$.i18n.Tr "action.rename_repo" .GetContent .GetRepoLink .ShortRepoPath | Str2html}} | ||
| {{else if eq .GetOpType 5}} | ||
| {{ $branchLink := (printf "branch/%s" .GetBranch) | EscapePound | Escape}} | ||
|
There was a problem hiding this comment. I think this should go into the translation file that already holds a part of the url. There was a problem hiding this comment. Is it really a good idea to put even more code there that is redundant across all translations? If so, am I supposed to edit the en-US file so that all translations have to be adapted via crowdin? There was a problem hiding this comment. I just don't like that one part is in the locale file and another one in code. It should be either or and not both. There was a problem hiding this comment. Ok moved it to the translations file. Maybe it would be a good idea to pass the entire links as parameters: |
||
| {{$.i18n.Tr "action.commit_repo" .GetRepoLink $branchLink (Escape .GetBranch) .ShortRepoPath | Str2html}} | ||
|
There was a problem hiding this comment. Why not use There was a problem hiding this comment. I think I chose to do that because EscapePound does url escaping and in general you would not want the branch names to be displayed like that (unless you want to punish users for using chars like |
||
| {{else if eq .GetOpType 6}} | ||
| {{ $index := index .GetIssueInfos 0}} | ||
| {{$.i18n.Tr "action.create_issue" .GetRepoLink $index .ShortRepoPath | Str2html}} | ||
|
|
@@ -24,7 +24,8 @@ | |
| {{else if eq .GetOpType 8}} | ||
| {{$.i18n.Tr "action.transfer_repo" .GetContent .GetRepoLink .ShortRepoPath | Str2html}} | ||
| {{else if eq .GetOpType 9}} | ||
| {{ $branchLink := .GetBranch | EscapePound | Escape}} | ||
|
There was a problem hiding this comment. Shouldn't this be also the non-legacy path like at https://github.com/go-gitea/gitea/pull/3767/files#diff-06961515e08a5e52a731517765266c80R16 ? There was a problem hiding this comment. Comment from above also applies here. |
||
| {{$.i18n.Tr "action.push_tag" .GetRepoLink $branchLink .ShortRepoPath | Str2html}} | ||
| {{else if eq .GetOpType 10}} | ||
| {{ $index := index .GetIssueInfos 0}} | ||
| {{$.i18n.Tr "action.comment_issue" .GetRepoLink $index .ShortRepoPath | Str2html}} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we just use
url.PathEscapehere instead? Just a thought. Didn't try it myself.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I tried that the last time, I got back into the endless loop that I was trying to fix in the first place. I have no idea what exactly the difference is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into that again:
String():
branch/s%3Cscript%3Ealert%28%27XSS%27%29;%3C/script%3EsPathEscape():
branch%2Fs%3Cscript%3Ealert%28%27XSS%27%29%3B%3C%2Fscript%3EsPathEscape also escapes slashes, which makes sense but is not wanted here because BranchNameSubURL already contains the branch/ prefix.