New template URL handling design #11811

New Issue

GiteaMirror · 2025-11-02T09:48:30-06:00

GiteaMirror commented

2025-11-02 09:48:30 -06:00

Originally created by @wxiaoguang on GitHub (Oct 8, 2023).

Many PRs don't handle the URL escaping in templates correctly. To make the template system more friendly, there could be a new design:

{{UrlPath "%s/foo/bar/{%s}" "the#/seg" "the#/slash"}}
output: "the%23/seg/foo/bar/the%23%2Fslash"

{{$q = dict "a" 1 "b" 2}}
{{UrlQuery $q "a" 100 "c" 3}}
output: "a=100&b=2&c=3"

{{UrlQuery ctx "mode" "off"}}
suppose the current URL is: "/path?foo=bar&mode=on"
then the output is: "foo=bar&mode=off"

Then most printf / PathEscape / PathEscapeSegments in templates could be removed, and there won't be low-level mistakes.

Originally created by @wxiaoguang on GitHub (Oct 8, 2023). Many PRs don't handle the URL escaping in templates correctly. To make the template system more friendly, there could be a new design: ``` {{UrlPath "%s/foo/bar/{%s}" "the#/seg" "the#/slash"}} output: "the%23/seg/foo/bar/the%23%2Fslash" ``` ``` {{$q = dict "a" 1 "b" 2}} {{UrlQuery $q "a" 100 "c" 3}} output: "a=100&b=2&c=3" {{UrlQuery ctx "mode" "off"}} suppose the current URL is: "/path?foo=bar&mode=on" then the output is: "foo=bar&mode=off" ``` Then most `printf` / `PathEscape` / `PathEscapeSegments` in templates could be removed, and there won't be low-level mistakes.

GiteaMirror added the type/proposal label 2025-11-02 09:48:30 -06:00

GiteaMirror closed this issue

2025-11-02 09:48:30 -06:00

GiteaMirror commented

2025-11-02 09:48:31 -06:00

@delvh commented on GitHub (Oct 8, 2023):

Generally, I agree.
However, I'd suggest switching around the escaping in UrlPath: %s is escaped, while {%s} is unescaped.
That way, we prevent accidents.

@delvh commented on GitHub (Oct 8, 2023): Generally, I agree. However, I'd suggest switching around the escaping in `UrlPath`: `%s` is escaped, while `{%s}` is unescaped. That way, we prevent accidents.

GiteaMirror commented

2025-11-02 09:48:31 -06:00

@wxiaoguang commented on GitHub (Oct 8, 2023):

However, I'd suggest switching around the escaping in UrlPath: %s is escaped, while {%s} is unescaped.

Both of them should be escaped. %s will use PathEscapeSegments and {%s} will use PathEscape (updated the description)

@wxiaoguang commented on GitHub (Oct 8, 2023): > However, I'd suggest switching around the escaping in `UrlPath`: `%s` is escaped, while `{%s}` is unescaped. Both of them should be escaped. `%s` will use `PathEscapeSegments` and `{%s}` will use `PathEscape` (updated the description)

GiteaMirror commented

2025-11-02 09:48:31 -06:00

@delvh commented on GitHub (Oct 8, 2023):

Still, which of the two is more common? The more common one should be shorter.
I'm not sure which of the two we need more often.

@delvh commented on GitHub (Oct 8, 2023): Still, which of the two is more common? The more common one should be shorter. I'm not sure which of the two we need more often.

GiteaMirror commented

2025-11-02 09:48:31 -06:00

@wxiaoguang commented on GitHub (Oct 9, 2023):

Still, which of the two is more common? The more common one should be shorter. I'm not sure which of the two we need more often.

In most cases, PathEscapeSegments is good enough. Although there are a lot of (UserName | PathEscape) or (CommitID | PathEscape), actually it's still safe to use PathEscapeSegments , and in many cases (in backend code) these values are even not escaped (because they are safe for URL path)

But there is still a problem of this design: actually there are 3 kinds of string values:

Already escaped (like AppSubURL / RepoLink)
Need PathEscapeSegments (for most cases)
Need PathEscape (eg: wiki path, only a few)

Case 1 is not well supported by this design ....

@wxiaoguang commented on GitHub (Oct 9, 2023): > Still, which of the two is more common? The more common one should be shorter. I'm not sure which of the two we need more often. In most cases, `PathEscapeSegments` is good enough. Although there are a lot of `(UserName | PathEscape)` or `(CommitID | PathEscape)`, actually it's still safe to use `PathEscapeSegments` , and in many cases (in backend code) these values are even not escaped (because they are safe for URL path) But there is still a problem of this design: actually there are 3 kinds of string values: 1. Already escaped (like AppSubURL / RepoLink) 2. Need PathEscapeSegments (for most cases) 3. Need PathEscape (eg: wiki path, only a few) Case 1 is not well supported by this design ....

GiteaMirror commented

2025-11-02 09:48:32 -06:00

@delvh commented on GitHub (Oct 9, 2023):

Unless we add a third type, i.e. [%s] that does not escape data

@delvh commented on GitHub (Oct 9, 2023): Unless we add a third type, i.e. `[%s]` that does not escape data

GiteaMirror commented

2025-11-02 09:48:32 -06:00

@wxiaoguang commented on GitHub (Oct 10, 2023):

Unless we add a third type, i.e. [%s] that does not escape data

Well, that's why I haven't started the work ..... it brings unclear part to the framework. If a developer sees this {{UrlPath "%s/[%s]/{%s}" ...}}, they would go crazy ....

@wxiaoguang commented on GitHub (Oct 10, 2023): > Unless we add a third type, i.e. `[%s]` that does not escape data Well, that's why I haven't started the work ..... it brings unclear part to the framework. If a developer sees this `{{UrlPath "%s/[%s]/{%s}" ...}}`, they would go crazy ....

GiteaMirror commented

2025-11-02 09:48:32 -06:00

@wxiaoguang commented on GitHub (Apr 23, 2025):

Some have been implemented. Others won't get progress any time soon

@wxiaoguang commented on GitHub (Apr 23, 2025): Some have been implemented. Others won't get progress any time soon

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/gitea#11811