diff --git a/modules/templates/helper_test.go b/modules/templates/helper_test.go index 27020e6e68..f04724bfe6 100644 --- a/modules/templates/helper_test.go +++ b/modules/templates/helper_test.go @@ -92,9 +92,12 @@ func TestTemplateEscape(t *testing.T) { } t.Run("Golang URL Escape", func(t *testing.T) { - // Golang template considers "href", "*src*", "*uri*", "*url*" (and more) ... attributes as contentTypeURL and does auto-escaping + // HINT: GOLANG-HTML-TEMPLATE-URL-ESCAPING: demo cases (html/template/attr.go): + // Golang template considers "href", "data-href", "*src*", "*uri*", "*url*" (and more) ... attributes as contentTypeURL and does auto-escaping actual := execTmpl(``) assert.Equal(t, ``, actual) + actual = execTmpl(``) + assert.Equal(t, ``, actual) actual = execTmpl(``) assert.Equal(t, ``, actual) }) @@ -102,6 +105,10 @@ func TestTemplateEscape(t *testing.T) { // non-URL content isn't auto-escaped actual := execTmpl(``) assert.Equal(t, ``, actual) + // the attr names like "data-href" and "data-action" are treated as URL (as the "data-" prefix is stripped) + // but "data-xxx-href" and "data-xxx-action" are not, so no escaping. + actual = execTmpl(``) + assert.Equal(t, ``, actual) }) t.Run("QueryBuild", func(t *testing.T) { actual := execTmpl(``) diff --git a/templates/admin/packages/list.tmpl b/templates/admin/packages/list.tmpl index 800f8dac5b..bf71a12d1e 100644 --- a/templates/admin/packages/list.tmpl +++ b/templates/admin/packages/list.tmpl @@ -73,7 +73,7 @@ {{DateUtils.AbsoluteShort .Version.CreatedUnix}} {{svg "octicon-trash"}} diff --git a/templates/admin/repo/list.tmpl b/templates/admin/repo/list.tmpl index d366b8cb2d..02205793c4 100644 --- a/templates/admin/repo/list.tmpl +++ b/templates/admin/repo/list.tmpl @@ -86,7 +86,7 @@ {{DateUtils.AbsoluteShort .CreatedUnix}} {{svg "octicon-trash"}} diff --git a/templates/org/team/members.tmpl b/templates/org/team/members.tmpl index c652d9c481..2b51764cb4 100644 --- a/templates/org/team/members.tmpl +++ b/templates/org/team/members.tmpl @@ -35,7 +35,7 @@
{{if and $.IsOrganizationOwner (not (and ($.Team.IsOwnerTeam) (eq (len $.Team.Members) 1)))}} {{end}} diff --git a/templates/org/team/sidebar.tmpl b/templates/org/team/sidebar.tmpl index a0b936285e..d2c092c1da 100644 --- a/templates/org/team/sidebar.tmpl +++ b/templates/org/team/sidebar.tmpl @@ -13,7 +13,7 @@
{{if .Team.IsMember ctx $.SignedUser.ID}} {{else if .IsOrganizationOwner}} diff --git a/templates/org/team/teams.tmpl b/templates/org/team/teams.tmpl index 33524e2cca..9f8030f28c 100644 --- a/templates/org/team/teams.tmpl +++ b/templates/org/team/teams.tmpl @@ -37,7 +37,7 @@ {{.NumRepos}} {{ctx.Locale.Tr "org.lower_repositories"}} {{if .IsMember ctx $.SignedUser.ID}} {{end}} diff --git a/templates/repo/branch/list.tmpl b/templates/repo/branch/list.tmpl index 74e38bfdc0..59c4b5bc89 100644 --- a/templates/repo/branch/list.tmpl +++ b/templates/repo/branch/list.tmpl @@ -200,7 +200,7 @@ {{else}} - {{end}} diff --git a/templates/repo/issue/view_content/context_menu.tmpl b/templates/repo/issue/view_content/context_menu.tmpl index d3c2f8932e..a2341e6097 100644 --- a/templates/repo/issue/view_content/context_menu.tmpl +++ b/templates/repo/issue/view_content/context_menu.tmpl @@ -34,10 +34,10 @@
{{end}} {{if $canUserBlock}} -
{{ctx.Locale.Tr "user.block.block.user"}}
+
{{ctx.Locale.Tr "user.block.block.user"}}
{{end}} {{if $canOrgBlock}} -
{{ctx.Locale.Tr "user.block.block.org"}}
+
{{ctx.Locale.Tr "user.block.block.org"}}
{{end}} {{end}} {{end}} diff --git a/templates/shared/secrets/add_list.tmpl b/templates/shared/secrets/add_list.tmpl index d99ec279dd..f16bd5e9aa 100644 --- a/templates/shared/secrets/add_list.tmpl +++ b/templates/shared/secrets/add_list.tmpl @@ -3,7 +3,7 @@
diff --git a/tests/integration/branches_test.go b/tests/integration/branches_test.go index a37f77986a..1a647001d9 100644 --- a/tests/integration/branches_test.go +++ b/tests/integration/branches_test.go @@ -58,7 +58,7 @@ func TestUndoDeleteBranch(t *testing.T) { } onGiteaRun(t, func(t *testing.T, u *url.URL) { - htmlDoc, name := branchAction(t, ".delete-branch-button", "data-modal-form.action") + htmlDoc, name := branchAction(t, ".delete-branch-button", "data-modal-form.url") assert.Contains(t, htmlDoc.doc.Find(".ui.positive.message").Text(), translation.NewLocale("en-US").TrString("repo.branch.deletion_success", name), diff --git a/web_src/js/features/common-button.test.ts b/web_src/js/features/common-button.test.ts index 4ae1f74897..0ca4d9eef9 100644 --- a/web_src/js/features/common-button.test.ts +++ b/web_src/js/features/common-button.test.ts @@ -2,9 +2,10 @@ import {assignElementProperty, type ElementWithAssignableProperties} from './com test('assignElementProperty', () => { const elForm = document.createElement('form'); - assignElementProperty(elForm, 'action', '/test-link'); - expect(elForm.action).contains('/test-link'); // the DOM always returns absolute URL - expect(elForm.getAttribute('action')).eq('/test-link'); + expect(() => assignElementProperty(elForm, 'action', '/test-link')).toThrow(); + assignElementProperty(elForm, 'url', '/test-url'); + expect(elForm.action).contains('/test-url'); // the DOM always returns absolute URL + expect(elForm.getAttribute('action')).eq('/test-url'); assignElementProperty(elForm, 'text-content', 'dummy'); expect(elForm.textContent).toBe('dummy'); @@ -14,8 +15,9 @@ test('assignElementProperty', () => { _attrs: Record = {}; setAttribute(name: string, value: string) { this._attrs[name] = value } getAttribute(name: string): string | null { return this._attrs[name] } + nodeName = 'FORM'; }(); - assignElementProperty(elFormWithAction, 'action', '/bar'); + assignElementProperty(elFormWithAction, 'url', '/bar'); expect(elFormWithAction.getAttribute('action')).eq('/bar'); const elInput = document.createElement('input'); diff --git a/web_src/js/features/common-button.ts b/web_src/js/features/common-button.ts index a00ed4ee8b..fd7dc47383 100644 --- a/web_src/js/features/common-button.ts +++ b/web_src/js/features/common-button.ts @@ -42,11 +42,20 @@ function onHidePanelClick(el: HTMLElement, e: MouseEvent) { } export type ElementWithAssignableProperties = { + nodeName: string; getAttribute: (name: string) => string | null; setAttribute: (name: string, value: string) => void; } & Record; export function assignElementProperty(el: ElementWithAssignableProperties, kebabName: string, val: string) { + if (el.nodeName === 'FORM') { + // HINT: GOLANG-HTML-TEMPLATE-URL-ESCAPING: a special case for Golang HTML template escaping. + // Golang HTML template only handles some "known" attribute names as URL (e.g.: when the name is "action" or contains "url") + // To prevent template developers from making mistakes like `data-modal-form.action="?k={{ValueWithSpecialChars}}" (no escaping), + // here we use `data-modal-form.url="?k={{ValueWithSpecialChars}}", then the value gets correctly escaped by Golang HTML template. + if (kebabName === 'action') throw new Error(`don't assign element property "action" by value, use "data-modal-form.url" instead`); + if (kebabName === 'url') kebabName = 'action'; + } const camelizedName = camelize(kebabName); const old = el[camelizedName]; if (typeof old === 'boolean') { @@ -59,7 +68,7 @@ export function assignElementProperty(el: ElementWithAssignableProperties, kebab // "form" has an edge case: its "" element overwrites the "action" property, we can only set attribute el.setAttribute(kebabName, val); } else { - // in the future, we could introduce a better typing system like `data-modal-form.action:string="..."` + // in the future, maybe we could introduce a better typing system if it is really needed throw new Error(`cannot assign element property "${camelizedName}" by value "${val}"`); } } @@ -71,7 +80,11 @@ function onShowModalClick(el: HTMLElement, e: MouseEvent) { // * Then, try to query '[name=target]' // * Then, try to query '.target' // * Then, try to query 'target' as HTML tag - // If there is a ".{prop-name}" part like "data-modal-form.action", the "form" element's "action" property will be set, the "prop-name" will be camel-cased to "propName". + // If there's a ".{prop-name}" part like "data-modal-input.value", the "input" element's "value" property will be set, + // the "prop-name" will be camel-cased to "propName" (e.g.: "data-modal-input.read-only" for "readOnly" property). + // + // HINT: GOLANG-HTML-TEMPLATE-URL-ESCAPING: Form element's "action" property must be set by "data-modal-form.url" + // to make the template variables get correctly escaped in the URL. e.preventDefault(); const modalSelector = el.getAttribute('data-modal')!; const elModal = document.querySelector(modalSelector);