From 6596b921409f0763f08e2cc2c1e466929a80be28 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 1 Jul 2025 15:19:03 +0800 Subject: [PATCH 01/13] Fix modal + form abuse (#34921) See the comment. And due to the abuse, there is a regression: when the modal is hidden, the form will be reset and it can't submit. This PR fixes all problems: keep the modal with form open, and add "loading" indicator. --- web_src/js/features/common-button.ts | 28 +++++++++++++-------------- web_src/js/features/comp/LabelEdit.ts | 1 + web_src/js/modules/fomantic/modal.ts | 28 +++++++++++++++++++++++++-- 3 files changed, 41 insertions(+), 16 deletions(-) diff --git a/web_src/js/features/common-button.ts b/web_src/js/features/common-button.ts index ae399e48b3..22a7890857 100644 --- a/web_src/js/features/common-button.ts +++ b/web_src/js/features/common-button.ts @@ -43,13 +43,16 @@ export function initGlobalDeleteButton(): void { fomanticQuery(modal).modal({ closable: false, - onApprove: async () => { + onApprove: () => { // if `data-type="form"` exists, then submit the form by the selector provided by `data-form="..."` if (btn.getAttribute('data-type') === 'form') { const formSelector = btn.getAttribute('data-form'); const form = document.querySelector(formSelector); if (!form) throw new Error(`no form named ${formSelector} found`); + modal.classList.add('is-loading'); // the form is not in the modal, so also add loading indicator to the modal + form.classList.add('is-loading'); form.submit(); + return false; // prevent modal from closing automatically } // prepare an AJAX form by data attributes @@ -62,12 +65,15 @@ export function initGlobalDeleteButton(): void { postData.append('id', value); } } - - const response = await POST(btn.getAttribute('data-url'), {data: postData}); - if (response.ok) { - const data = await response.json(); - window.location.href = data.redirect; - } + (async () => { + const response = await POST(btn.getAttribute('data-url'), {data: postData}); + if (response.ok) { + const data = await response.json(); + window.location.href = data.redirect; + } + })(); + modal.classList.add('is-loading'); // the request is in progress, so also add loading indicator to the modal + return false; // prevent modal from closing automatically }, }).modal('show'); }); @@ -158,13 +164,7 @@ function onShowModalClick(el: HTMLElement, e: MouseEvent) { } } - fomanticQuery(elModal).modal('setting', { - onApprove: () => { - // "form-fetch-action" can handle network errors gracefully, - // so keep the modal dialog to make users can re-submit the form if anything wrong happens. - if (elModal.querySelector('.form-fetch-action')) return false; - }, - }).modal('show'); + fomanticQuery(elModal).modal('show'); } export function initGlobalButtons(): void { diff --git a/web_src/js/features/comp/LabelEdit.ts b/web_src/js/features/comp/LabelEdit.ts index 141c5eecfe..423440129c 100644 --- a/web_src/js/features/comp/LabelEdit.ts +++ b/web_src/js/features/comp/LabelEdit.ts @@ -72,6 +72,7 @@ export function initCompLabelEdit(pageSelector: string) { return false; } submitFormFetchAction(form); + return false; }, }).modal('show'); }; diff --git a/web_src/js/modules/fomantic/modal.ts b/web_src/js/modules/fomantic/modal.ts index b07b941590..a96c7785e1 100644 --- a/web_src/js/modules/fomantic/modal.ts +++ b/web_src/js/modules/fomantic/modal.ts @@ -9,8 +9,9 @@ const fomanticModalFn = $.fn.modal; export function initAriaModalPatch() { if ($.fn.modal === ariaModalFn) throw new Error('initAriaModalPatch could only be called once'); $.fn.modal = ariaModalFn; - $.fn.fomanticExt.onModalBeforeHidden = onModalBeforeHidden; (ariaModalFn as FomanticInitFunction).settings = fomanticModalFn.settings; + $.fn.fomanticExt.onModalBeforeHidden = onModalBeforeHidden; + $.fn.modal.settings.onApprove = onModalApproveDefault; } // the patched `$.fn.modal` modal function @@ -34,6 +35,29 @@ function ariaModalFn(this: any, ...args: Parameters) { function onModalBeforeHidden(this: any) { const $modal = $(this); const elModal = $modal[0]; - queryElems(elModal, 'form', (form: HTMLFormElement) => form.reset()); hideToastsFrom(elModal.closest('.ui.dimmer') ?? document.body); + + // reset the form after the modal is hidden, after other modal events and handlers (e.g. "onApprove", form submit) + setTimeout(() => { + queryElems(elModal, 'form', (form: HTMLFormElement) => form.reset()); + }, 0); +} + +function onModalApproveDefault(this: any) { + const $modal = $(this); + const selectors = $modal.modal('setting', 'selector'); + const elModal = $modal[0]; + const elApprove = elModal.querySelector(selectors.approve); + const elForm = elApprove?.closest('form'); + if (!elForm) return true; // no form, just allow closing the modal + + // "form-fetch-action" can handle network errors gracefully, + // so keep the modal dialog to make users can re-submit the form if anything wrong happens. + if (elForm.matches('.form-fetch-action')) return false; + + // There is an abuse for the "modal" + "form" combination, the "Approve" button is a traditional form submit button in the form. + // Then "approve" and "submit" occur at the same time, the modal will be closed immediately before the form is submitted. + // So here we prevent the modal from closing automatically by returning false, add the "is-loading" class to the form element. + elForm.classList.add('is-loading'); + return false; } From 90f96c301e9e139e351b6d68995d94e6ca679133 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 1 Jul 2025 16:32:39 +0800 Subject: [PATCH 02/13] Fix PR toggle WIP (#34920) Fix #34919 --------- Signed-off-by: wxiaoguang --- routers/web/repo/issue_view.go | 4 ++++ templates/repo/issue/sidebar/wip_switch.tmpl | 2 +- .../issue/view_content/pull_merge_box.tmpl | 2 +- web_src/js/features/repo-issue.ts | 22 ++++++++----------- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index 2897652d51..d4458ed19e 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -443,6 +443,10 @@ func ViewPullMergeBox(ctx *context.Context) { preparePullViewPullInfo(ctx, issue) preparePullViewReviewAndMerge(ctx, issue) ctx.Data["PullMergeBoxReloading"] = issue.PullRequest.IsChecking() + + // TODO: it should use a dedicated struct to render the pull merge box, to make sure all data is prepared correctly + ctx.Data["IsIssuePoster"] = ctx.IsSigned && issue.IsPoster(ctx.Doer.ID) + ctx.Data["HasIssuesOrPullsWritePermission"] = ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) ctx.HTML(http.StatusOK, tplPullMergeBox) } diff --git a/templates/repo/issue/sidebar/wip_switch.tmpl b/templates/repo/issue/sidebar/wip_switch.tmpl index b007399deb..8c40908f62 100644 --- a/templates/repo/issue/sidebar/wip_switch.tmpl +++ b/templates/repo/issue/sidebar/wip_switch.tmpl @@ -1,5 +1,5 @@ {{if and (or .HasIssuesOrPullsWritePermission .IsIssuePoster) (not .HasMerged) (not .Issue.IsClosed) (not .IsPullWorkInProgress)}} - + {{ctx.Locale.Tr "repo.pulls.still_in_progress"}} {{ctx.Locale.Tr "repo.pulls.add_prefix" (index .PullRequestWorkInProgressPrefixes 0)}} {{end}} diff --git a/templates/repo/issue/view_content/pull_merge_box.tmpl b/templates/repo/issue/view_content/pull_merge_box.tmpl index 641520247d..46bcd3b8b3 100644 --- a/templates/repo/issue/view_content/pull_merge_box.tmpl +++ b/templates/repo/issue/view_content/pull_merge_box.tmpl @@ -95,7 +95,7 @@ {{ctx.Locale.Tr "repo.pulls.cannot_merge_work_in_progress"}} {{if or .HasIssuesOrPullsWritePermission .IsIssuePoster}} - {{end}} diff --git a/web_src/js/features/repo-issue.ts b/web_src/js/features/repo-issue.ts index bc7d4dee19..c7799ec415 100644 --- a/web_src/js/features/repo-issue.ts +++ b/web_src/js/features/repo-issue.ts @@ -17,6 +17,7 @@ import {showErrorToast} from '../modules/toast.ts'; import {initRepoIssueSidebar} from './repo-issue-sidebar.ts'; import {fomanticQuery} from '../modules/fomantic/base.ts'; import {ignoreAreYouSure} from '../vendor/jquery.are-you-sure.ts'; +import {registerGlobalInitFunc} from '../modules/observer.ts'; const {appSubUrl} = window.config; @@ -416,25 +417,20 @@ export function initRepoIssueWipNewTitle() { export function initRepoIssueWipToggle() { // Toggle WIP for existing PR - queryElems(document, '.toggle-wip', (el) => el.addEventListener('click', async (e) => { + registerGlobalInitFunc('initPullRequestWipToggle', (toggleWip) => toggleWip.addEventListener('click', async (e) => { e.preventDefault(); - const toggleWip = el; const title = toggleWip.getAttribute('data-title'); const wipPrefix = toggleWip.getAttribute('data-wip-prefix'); const updateUrl = toggleWip.getAttribute('data-update-url'); - try { - const params = new URLSearchParams(); - params.append('title', title?.startsWith(wipPrefix) ? title.slice(wipPrefix.length).trim() : `${wipPrefix.trim()} ${title}`); - - const response = await POST(updateUrl, {data: params}); - if (!response.ok) { - throw new Error('Failed to toggle WIP status'); - } - window.location.reload(); - } catch (error) { - console.error(error); + const params = new URLSearchParams(); + params.append('title', title?.startsWith(wipPrefix) ? title.slice(wipPrefix.length).trim() : `${wipPrefix.trim()} ${title}`); + const response = await POST(updateUrl, {data: params}); + if (!response.ok) { + showErrorToast(`Failed to toggle 'work in progress' status`); + return; } + window.location.reload(); })); } From 35f0b5a3ec5681cd04da2861c37364ec2595b82f Mon Sep 17 00:00:00 2001 From: Aaron Meese Date: Tue, 1 Jul 2025 07:14:32 -0400 Subject: [PATCH 03/13] Adds tooltip on branch commit counts (#34869) Adds a tooltip to the commit counts when comparing branches, making it easier for novice users to understand what the numbers mean. Fixes #34867. --------- Signed-off-by: wxiaoguang Co-authored-by: wxiaoguang --- options/locale/locale_en-US.ini | 2 ++ templates/repo/branch/list.tmpl | 12 +++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 6052177100..f979fc814d 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2769,6 +2769,8 @@ branch.new_branch_from = Create new branch from "%s" branch.renamed = Branch %s was renamed to %s. branch.rename_default_or_protected_branch_error = Only admins can rename default or protected branches. branch.rename_protected_branch_failed = This branch is protected by glob-based protection rules. +branch.commits_divergence_from = Commits divergence: %[1]d behind and %[2]d ahead of %[3]s +branch.commits_no_divergence = The same as branch %[1]s tag.create_tag = Create tag %s tag.create_tag_operation = Create tag diff --git a/templates/repo/branch/list.tmpl b/templates/repo/branch/list.tmpl index fffe3a08cc..9e86641c6f 100644 --- a/templates/repo/branch/list.tmpl +++ b/templates/repo/branch/list.tmpl @@ -107,8 +107,14 @@ {{end}} - {{if and (not .DBBranch.IsDeleted) $.DefaultBranchBranch}} -
+ {{if and (not .DBBranch.IsDeleted) $.DefaultBranchBranch}} + {{$tooltipDivergence := ""}} + {{if or .CommitsBehind .CommitsAhead}} + {{$tooltipDivergence = ctx.Locale.Tr "repo.branch.commits_divergence_from" .CommitsBehind .CommitsAhead $.DefaultBranchBranch.DBBranch.Name}} + {{else}} + {{$tooltipDivergence = ctx.Locale.Tr "repo.branch.commits_no_divergence" $.DefaultBranchBranch.DBBranch.Name}} + {{end}} +
{{.CommitsBehind}}
{{/* old code bears 0/0.0 = NaN output, so it might output invalid "width: NaNpx", it just works and doesn't caues any problem. */}} @@ -119,7 +125,7 @@
- {{end}} + {{end}} {{if not .LatestPullRequest}} From 1d4ad5aa2b3a321a8d759bb91fc78e0aa6a89ed9 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 1 Jul 2025 21:44:05 +0800 Subject: [PATCH 04/13] Improve html escape (#34911) drop "escape-goat" --- .eslintrc.cjs | 3 +- package-lock.json | 13 --------- package.json | 1 - web_src/js/bootstrap.ts | 3 +- web_src/js/components/ViewFileTreeStore.ts | 3 +- web_src/js/features/comp/ConfirmModal.ts | 24 ++++++++-------- web_src/js/features/comp/EditorUpload.ts | 2 +- web_src/js/features/comp/SearchUserBox.ts | 2 +- web_src/js/features/dropzone.ts | 6 ++-- web_src/js/features/emoji.ts | 6 ++-- web_src/js/features/file-view.ts | 4 +-- web_src/js/features/repo-editor.ts | 13 +++++---- web_src/js/features/repo-issue-list.ts | 8 +++--- web_src/js/features/repo-issue.ts | 5 ++-- web_src/js/features/repo-new.ts | 2 +- web_src/js/features/repo-wiki.ts | 3 +- web_src/js/features/tribute.ts | 13 +++++---- web_src/js/markup/html2markdown.ts | 8 +++--- web_src/js/markup/mermaid.ts | 3 +- web_src/js/modules/tippy.ts | 3 +- web_src/js/modules/toast.ts | 2 +- web_src/js/svg.ts | 3 +- web_src/js/utils/dom.ts | 1 + web_src/js/utils/html.test.ts | 8 ++++++ web_src/js/utils/html.ts | 32 ++++++++++++++++++++++ 25 files changed, 103 insertions(+), 68 deletions(-) create mode 100644 web_src/js/utils/html.test.ts create mode 100644 web_src/js/utils/html.ts diff --git a/.eslintrc.cjs b/.eslintrc.cjs index f9e1050240..57c6b19600 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -91,6 +91,7 @@ module.exports = { plugins: ['@vitest/eslint-plugin'], globals: vitestPlugin.environments.env.globals, rules: { + 'github/unescaped-html-literal': [0], '@vitest/consistent-test-filename': [0], '@vitest/consistent-test-it': [0], '@vitest/expect-expect': [0], @@ -423,7 +424,7 @@ module.exports = { 'github/no-useless-passive': [2], 'github/prefer-observers': [2], 'github/require-passive-events': [2], - 'github/unescaped-html-literal': [0], + 'github/unescaped-html-literal': [2], 'grouped-accessor-pairs': [2], 'guard-for-in': [0], 'id-blacklist': [0], diff --git a/package-lock.json b/package-lock.json index 132efb8635..8361199086 100644 --- a/package-lock.json +++ b/package-lock.json @@ -28,7 +28,6 @@ "dropzone": "6.0.0-beta.2", "easymde": "2.20.0", "esbuild-loader": "4.3.0", - "escape-goat": "4.0.0", "fast-glob": "3.3.3", "htmx.org": "2.0.6", "idiomorph": "0.7.3", @@ -6563,18 +6562,6 @@ "node": ">=6" } }, - "node_modules/escape-goat": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/escape-goat/-/escape-goat-4.0.0.tgz", - "integrity": "sha512-2Sd4ShcWxbx6OY1IHyla/CVNwvg7XwZVoXZHcSu9w9SReNP1EzzD5T8NWKIR38fIqEns9kDWKUQTXXAmlDrdPg==", - "license": "MIT", - "engines": { - "node": ">=12" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, "node_modules/escape-string-regexp": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/escape-string-regexp/-/escape-string-regexp-4.0.0.tgz", diff --git a/package.json b/package.json index c8a48bb5d9..fc620bc986 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,6 @@ "dropzone": "6.0.0-beta.2", "easymde": "2.20.0", "esbuild-loader": "4.3.0", - "escape-goat": "4.0.0", "fast-glob": "3.3.3", "htmx.org": "2.0.6", "idiomorph": "0.7.3", diff --git a/web_src/js/bootstrap.ts b/web_src/js/bootstrap.ts index 9e41673b86..96a2759a23 100644 --- a/web_src/js/bootstrap.ts +++ b/web_src/js/bootstrap.ts @@ -2,6 +2,7 @@ // to make sure the error handler always works, we should never import `window.config`, because // some user's custom template breaks it. import type {Intent} from './types.ts'; +import {html} from './utils/html.ts'; // This sets up the URL prefix used in webpack's chunk loading. // This file must be imported before any lazy-loading is being attempted. @@ -23,7 +24,7 @@ export function showGlobalErrorMessage(msg: string, msgType: Intent = 'error') { let msgDiv = msgContainer.querySelector(`.js-global-error[data-global-error-msg-compact="${msgCompact}"]`); if (!msgDiv) { const el = document.createElement('div'); - el.innerHTML = `
`; + el.innerHTML = html`
`; msgDiv = el.childNodes[0] as HTMLDivElement; } // merge duplicated messages into "the message (count)" format diff --git a/web_src/js/components/ViewFileTreeStore.ts b/web_src/js/components/ViewFileTreeStore.ts index 13e2753c94..e2155bd58a 100644 --- a/web_src/js/components/ViewFileTreeStore.ts +++ b/web_src/js/components/ViewFileTreeStore.ts @@ -2,6 +2,7 @@ import {reactive} from 'vue'; import {GET} from '../modules/fetch.ts'; import {pathEscapeSegments} from '../utils/url.ts'; import {createElementFromHTML} from '../utils/dom.ts'; +import {html} from '../utils/html.ts'; export function createViewFileTreeStore(props: { repoLink: string, treePath: string, currentRefNameSubURL: string}) { const store = reactive({ @@ -16,7 +17,7 @@ export function createViewFileTreeStore(props: { repoLink: string, treePath: str if (!document.querySelector(`.global-svg-icon-pool #${svgId}`)) poolSvgs.push(svgContent); } if (poolSvgs.length) { - const svgContainer = createElementFromHTML('
'); + const svgContainer = createElementFromHTML(html`
`); svgContainer.innerHTML = poolSvgs.join(''); document.body.append(svgContainer); } diff --git a/web_src/js/features/comp/ConfirmModal.ts b/web_src/js/features/comp/ConfirmModal.ts index 81ea09476b..97a73eace6 100644 --- a/web_src/js/features/comp/ConfirmModal.ts +++ b/web_src/js/features/comp/ConfirmModal.ts @@ -1,5 +1,5 @@ import {svg} from '../../svg.ts'; -import {htmlEscape} from 'escape-goat'; +import {html, htmlRaw} from '../../utils/html.ts'; import {createElementFromHTML} from '../../utils/dom.ts'; import {fomanticQuery} from '../../modules/fomantic/base.ts'; @@ -12,17 +12,17 @@ type ConfirmModalOptions = { } export function createConfirmModal({header = '', content = '', confirmButtonColor = 'primary'}:ConfirmModalOptions = {}): HTMLElement { - const headerHtml = header ? `
${htmlEscape(header)}
` : ''; - return createElementFromHTML(` - -`); + const headerHtml = header ? html`
${header}
` : ''; + return createElementFromHTML(html` + + `.trim()); } export function confirmModal(modal: HTMLElement | ConfirmModalOptions): Promise { diff --git a/web_src/js/features/comp/EditorUpload.ts b/web_src/js/features/comp/EditorUpload.ts index bf9ce9bfb1..bf78f58daf 100644 --- a/web_src/js/features/comp/EditorUpload.ts +++ b/web_src/js/features/comp/EditorUpload.ts @@ -114,7 +114,7 @@ async function handleUploadFiles(editor: CodeMirrorEditor | TextareaEditor, drop export function removeAttachmentLinksFromMarkdown(text: string, fileUuid: string) { text = text.replace(new RegExp(`!?\\[([^\\]]+)\\]\\(/?attachments/${fileUuid}\\)`, 'g'), ''); - text = text.replace(new RegExp(`]+src="/?attachments/${fileUuid}"[^>]*>`, 'g'), ''); + text = text.replace(new RegExp(`[<]img[^>]+src="/?attachments/${fileUuid}"[^>]*>`, 'g'), ''); return text; } diff --git a/web_src/js/features/comp/SearchUserBox.ts b/web_src/js/features/comp/SearchUserBox.ts index 9fedb3ed24..4b13a2141f 100644 --- a/web_src/js/features/comp/SearchUserBox.ts +++ b/web_src/js/features/comp/SearchUserBox.ts @@ -1,4 +1,4 @@ -import {htmlEscape} from 'escape-goat'; +import {htmlEscape} from '../../utils/html.ts'; import {fomanticQuery} from '../../modules/fomantic/base.ts'; const {appSubUrl} = window.config; diff --git a/web_src/js/features/dropzone.ts b/web_src/js/features/dropzone.ts index b2ba7651c4..20f7ceb6c3 100644 --- a/web_src/js/features/dropzone.ts +++ b/web_src/js/features/dropzone.ts @@ -1,5 +1,5 @@ import {svg} from '../svg.ts'; -import {htmlEscape} from 'escape-goat'; +import {html} from '../utils/html.ts'; import {clippie} from 'clippie'; import {showTemporaryTooltip} from '../modules/tippy.ts'; import {GET, POST} from '../modules/fetch.ts'; @@ -33,14 +33,14 @@ export function generateMarkdownLinkForAttachment(file: Partial tag because it's the only // method to change image size in Markdown that is supported by all implementations. // Make the image link relative to the repo path, then the final URL is "/sub-path/owner/repo/attachments/{uuid}" - fileMarkdown = `${htmlEscape(file.name)}`; + fileMarkdown = html`${file.name}`; } else { // Markdown always renders the image with a relative path, so the final URL is "/sub-path/owner/repo/attachments/{uuid}" // TODO: it should also use relative path for consistency, because absolute is ambiguous for "/sub-path/attachments" or "/attachments" fileMarkdown = `![${file.name}](/attachments/${file.uuid})`; } } else if (isVideoFile(file)) { - fileMarkdown = ``; + fileMarkdown = html``; } return fileMarkdown; } diff --git a/web_src/js/features/emoji.ts b/web_src/js/features/emoji.ts index 135620e51e..69afe491e2 100644 --- a/web_src/js/features/emoji.ts +++ b/web_src/js/features/emoji.ts @@ -1,4 +1,5 @@ import emojis from '../../../assets/emoji.json' with {type: 'json'}; +import {html} from '../utils/html.ts'; const {assetUrlPrefix, customEmojis} = window.config; @@ -24,12 +25,11 @@ for (const key of emojiKeys) { export function emojiHTML(name: string) { let inner; if (Object.hasOwn(customEmojis, name)) { - inner = `:${name}:`; + inner = html`:${name}:`; } else { inner = emojiString(name); } - - return `${inner}`; + return html`${inner}`; } // retrieve string for given emoji name diff --git a/web_src/js/features/file-view.ts b/web_src/js/features/file-view.ts index 867f946297..d803f53c0d 100644 --- a/web_src/js/features/file-view.ts +++ b/web_src/js/features/file-view.ts @@ -3,7 +3,7 @@ import {newRenderPlugin3DViewer} from '../render/plugins/3d-viewer.ts'; import {newRenderPluginPdfViewer} from '../render/plugins/pdf-viewer.ts'; import {registerGlobalInitFunc} from '../modules/observer.ts'; import {createElementFromHTML, showElem, toggleClass} from '../utils/dom.ts'; -import {htmlEscape} from 'escape-goat'; +import {html} from '../utils/html.ts'; import {basename} from '../utils.ts'; const plugins: FileRenderPlugin[] = []; @@ -54,7 +54,7 @@ async function renderRawFileToContainer(container: HTMLElement, rawFileLink: str container.replaceChildren(elViewRawPrompt); if (errorMsg) { - const elErrorMessage = createElementFromHTML(htmlEscape`
${errorMsg}
`); + const elErrorMessage = createElementFromHTML(html`
${errorMsg}
`); elViewRawPrompt.insertAdjacentElement('afterbegin', elErrorMessage); } } diff --git a/web_src/js/features/repo-editor.ts b/web_src/js/features/repo-editor.ts index c6b5cccd54..f3ca13460c 100644 --- a/web_src/js/features/repo-editor.ts +++ b/web_src/js/features/repo-editor.ts @@ -1,4 +1,4 @@ -import {htmlEscape} from 'escape-goat'; +import {html, htmlRaw} from '../utils/html.ts'; import {createCodeEditor} from './codeeditor.ts'; import {hideElem, queryElems, showElem, createElementFromHTML} from '../utils/dom.ts'; import {attachRefIssueContextPopup} from './contextpopup.ts'; @@ -87,10 +87,10 @@ export function initRepoEditor() { if (i < parts.length - 1) { if (trimValue.length) { const linkElement = createElementFromHTML( - `${htmlEscape(value)}`, + html`${value}`, ); const dividerElement = createElementFromHTML( - ``, + html``, ); links.push(linkElement); dividers.push(dividerElement); @@ -113,7 +113,7 @@ export function initRepoEditor() { if (!warningDiv) { warningDiv = document.createElement('div'); warningDiv.classList.add('ui', 'warning', 'message', 'flash-message', 'flash-warning', 'space-related'); - warningDiv.innerHTML = '

File path contains leading or trailing whitespace.

'; + warningDiv.innerHTML = html`

File path contains leading or trailing whitespace.

`; // Add display 'block' because display is set to 'none' in formantic\build\semantic.css warningDiv.style.display = 'block'; const inputContainer = document.querySelector('.repo-editor-header'); @@ -196,7 +196,8 @@ export function initRepoEditor() { })(); } -export function renderPreviewPanelContent(previewPanel: Element, content: string) { - previewPanel.innerHTML = `
${content}
`; +export function renderPreviewPanelContent(previewPanel: Element, htmlContent: string) { + // the content is from the server, so it is safe to use innerHTML + previewPanel.innerHTML = html`
${htmlRaw(htmlContent)}
`; attachRefIssueContextPopup(previewPanel.querySelectorAll('p .ref-issue')); } diff --git a/web_src/js/features/repo-issue-list.ts b/web_src/js/features/repo-issue-list.ts index 3ea5fb70c0..762fbf51bb 100644 --- a/web_src/js/features/repo-issue-list.ts +++ b/web_src/js/features/repo-issue-list.ts @@ -1,6 +1,6 @@ import {updateIssuesMeta} from './repo-common.ts'; import {toggleElem, queryElems, isElemVisible} from '../utils/dom.ts'; -import {htmlEscape} from 'escape-goat'; +import {html} from '../utils/html.ts'; import {confirmModal} from './comp/ConfirmModal.ts'; import {showErrorToast} from '../modules/toast.ts'; import {createSortable} from '../modules/sortable.ts'; @@ -138,10 +138,10 @@ function initDropdownUserRemoteSearch(el: Element) { // the content is provided by backend IssuePosters handler processedResults.length = 0; for (const item of resp.results) { - let html = `${htmlEscape(item.username)}`; - if (item.full_name) html += `${htmlEscape(item.full_name)}`; + let nameHtml = html`${item.username}`; + if (item.full_name) nameHtml += html`${item.full_name}`; if (selectedUsername.toLowerCase() === item.username.toLowerCase()) selectedUsername = item.username; - processedResults.push({value: item.username, name: html}); + processedResults.push({value: item.username, name: nameHtml}); } resp.results = processedResults; return resp; diff --git a/web_src/js/features/repo-issue.ts b/web_src/js/features/repo-issue.ts index c7799ec415..49e8fc40a2 100644 --- a/web_src/js/features/repo-issue.ts +++ b/web_src/js/features/repo-issue.ts @@ -1,4 +1,4 @@ -import {htmlEscape} from 'escape-goat'; +import {html, htmlEscape} from '../utils/html.ts'; import {createTippy, showTemporaryTooltip} from '../modules/tippy.ts'; import { addDelegatedEventListener, @@ -46,8 +46,7 @@ export function initRepoIssueSidebarDependency() { if (String(issue.id) === currIssueId) continue; filteredResponse.results.push({ value: issue.id, - name: `
#${issue.number} ${htmlEscape(issue.title)}
-
${htmlEscape(issue.repository.full_name)}
`, + name: html`
#${issue.number} ${issue.title}
${issue.repository.full_name}
`, }); } return filteredResponse; diff --git a/web_src/js/features/repo-new.ts b/web_src/js/features/repo-new.ts index 0e4d78872d..e2aa13f490 100644 --- a/web_src/js/features/repo-new.ts +++ b/web_src/js/features/repo-new.ts @@ -1,5 +1,5 @@ import {hideElem, querySingleVisibleElem, showElem, toggleElem} from '../utils/dom.ts'; -import {htmlEscape} from 'escape-goat'; +import {htmlEscape} from '../utils/html.ts'; import {fomanticQuery} from '../modules/fomantic/base.ts'; import {sanitizeRepoName} from './repo-common.ts'; diff --git a/web_src/js/features/repo-wiki.ts b/web_src/js/features/repo-wiki.ts index f94d3ef3d1..6ae0947077 100644 --- a/web_src/js/features/repo-wiki.ts +++ b/web_src/js/features/repo-wiki.ts @@ -2,6 +2,7 @@ import {validateTextareaNonEmpty, initComboMarkdownEditor} from './comp/ComboMar import {fomanticMobileScreen} from '../modules/fomantic.ts'; import {POST} from '../modules/fetch.ts'; import type {ComboMarkdownEditor} from './comp/ComboMarkdownEditor.ts'; +import {html, htmlRaw} from '../utils/html.ts'; async function initRepoWikiFormEditor() { const editArea = document.querySelector('.repository.wiki .combo-markdown-editor textarea'); @@ -30,7 +31,7 @@ async function initRepoWikiFormEditor() { const response = await POST(editor.previewUrl, {data: formData}); const data = await response.text(); lastContent = newContent; - previewTarget.innerHTML = `
${data}
`; + previewTarget.innerHTML = html`
${htmlRaw(data)}
`; } catch (error) { console.error('Error rendering preview:', error); } finally { diff --git a/web_src/js/features/tribute.ts b/web_src/js/features/tribute.ts index cf98377ae7..43c21ebe6d 100644 --- a/web_src/js/features/tribute.ts +++ b/web_src/js/features/tribute.ts @@ -1,5 +1,5 @@ import {emojiKeys, emojiHTML, emojiString} from './emoji.ts'; -import {htmlEscape} from 'escape-goat'; +import {html, htmlRaw} from '../utils/html.ts'; type TributeItem = Record; @@ -26,17 +26,18 @@ export async function attachTribute(element: HTMLElement) { return emojiString(item.original); }, menuItemTemplate: (item: TributeItem) => { - return `
${emojiHTML(item.original)}${htmlEscape(item.original)}
`; + return html`
${htmlRaw(emojiHTML(item.original))}${item.original}
`; }, }, { // mentions values: window.config.mentionValues ?? [], requireLeadingSpace: true, menuItemTemplate: (item: TributeItem) => { - return ` + const fullNameHtml = item.original.fullname && item.original.fullname !== '' ? html`${item.original.fullname}` : ''; + return html`
- - ${htmlEscape(item.original.name)} - ${item.original.fullname && item.original.fullname !== '' ? `${htmlEscape(item.original.fullname)}` : ''} + + ${item.original.name} + ${htmlRaw(fullNameHtml)}
`; }, diff --git a/web_src/js/markup/html2markdown.ts b/web_src/js/markup/html2markdown.ts index 8c2d2f8c86..5866d0d259 100644 --- a/web_src/js/markup/html2markdown.ts +++ b/web_src/js/markup/html2markdown.ts @@ -1,4 +1,4 @@ -import {htmlEscape} from 'escape-goat'; +import {html, htmlRaw} from '../utils/html.ts'; type Processor = (el: HTMLElement) => string | HTMLElement | void; @@ -38,10 +38,10 @@ function prepareProcessors(ctx:ProcessorContext): Processors { IMG(el: HTMLElement) { const alt = el.getAttribute('alt') || 'image'; const src = el.getAttribute('src'); - const widthAttr = el.hasAttribute('width') ? ` width="${htmlEscape(el.getAttribute('width') || '')}"` : ''; - const heightAttr = el.hasAttribute('height') ? ` height="${htmlEscape(el.getAttribute('height') || '')}"` : ''; + const widthAttr = el.hasAttribute('width') ? htmlRaw` width="${el.getAttribute('width') || ''}"` : ''; + const heightAttr = el.hasAttribute('height') ? htmlRaw` height="${el.getAttribute('height') || ''}"` : ''; if (widthAttr || heightAttr) { - return `${htmlEscape(alt)}`; + return html`${alt}`; } return `![${alt}](${src})`; }, diff --git a/web_src/js/markup/mermaid.ts b/web_src/js/markup/mermaid.ts index ac24b3bcba..33d9a1ed9b 100644 --- a/web_src/js/markup/mermaid.ts +++ b/web_src/js/markup/mermaid.ts @@ -2,6 +2,7 @@ import {isDarkTheme} from '../utils.ts'; import {makeCodeCopyButton} from './codecopy.ts'; import {displayError} from './common.ts'; import {queryElems} from '../utils/dom.ts'; +import {html, htmlRaw} from '../utils/html.ts'; const {mermaidMaxSourceCharacters} = window.config; @@ -46,7 +47,7 @@ export async function initMarkupCodeMermaid(elMarkup: HTMLElement): Promise${svg}`; + iframe.srcdoc = html`${htmlRaw(svg)}`; const mermaidBlock = document.createElement('div'); mermaidBlock.classList.add('mermaid-block', 'is-loading', 'tw-hidden'); diff --git a/web_src/js/modules/tippy.ts b/web_src/js/modules/tippy.ts index f7a4b3723b..2a1d998d76 100644 --- a/web_src/js/modules/tippy.ts +++ b/web_src/js/modules/tippy.ts @@ -2,6 +2,7 @@ import tippy, {followCursor} from 'tippy.js'; import {isDocumentFragmentOrElementNode} from '../utils/dom.ts'; import {formatDatetime} from '../utils/time.ts'; import type {Content, Instance, Placement, Props} from 'tippy.js'; +import {html} from '../utils/html.ts'; type TippyOpts = { role?: string, @@ -9,7 +10,7 @@ type TippyOpts = { } & Partial; const visibleInstances = new Set(); -const arrowSvg = ``; +const arrowSvg = html``; export function createTippy(target: Element, opts: TippyOpts = {}): Instance { // the callback functions should be destructured from opts, diff --git a/web_src/js/modules/toast.ts b/web_src/js/modules/toast.ts index b0afc343c3..ed807a4977 100644 --- a/web_src/js/modules/toast.ts +++ b/web_src/js/modules/toast.ts @@ -1,4 +1,4 @@ -import {htmlEscape} from 'escape-goat'; +import {htmlEscape} from '../utils/html.ts'; import {svg} from '../svg.ts'; import {animateOnce, queryElems, showElem} from '../utils/dom.ts'; import Toastify from 'toastify-js'; // don't use "async import", because when network error occurs, the "async import" also fails and nothing is shown diff --git a/web_src/js/svg.ts b/web_src/js/svg.ts index 7b377e1ab4..50c9536f37 100644 --- a/web_src/js/svg.ts +++ b/web_src/js/svg.ts @@ -1,5 +1,6 @@ import {defineComponent, h, type PropType} from 'vue'; import {parseDom, serializeXml} from './utils.ts'; +import {html, htmlRaw} from './utils/html.ts'; import giteaDoubleChevronLeft from '../../public/assets/img/svg/gitea-double-chevron-left.svg'; import giteaDoubleChevronRight from '../../public/assets/img/svg/gitea-double-chevron-right.svg'; import giteaEmptyCheckbox from '../../public/assets/img/svg/gitea-empty-checkbox.svg'; @@ -220,7 +221,7 @@ export const SvgIcon = defineComponent({ const classes = Array.from(svgOuter.classList); if (this.symbolId) { classes.push('tw-hidden', 'svg-symbol-container'); - svgInnerHtml = `${svgInnerHtml}`; + svgInnerHtml = html`${htmlRaw(svgInnerHtml)}`; } // create VNode return h('svg', { diff --git a/web_src/js/utils/dom.ts b/web_src/js/utils/dom.ts index 7ed0d73406..8b540cebb1 100644 --- a/web_src/js/utils/dom.ts +++ b/web_src/js/utils/dom.ts @@ -314,6 +314,7 @@ export function replaceTextareaSelection(textarea: HTMLTextAreaElement, text: st export function createElementFromHTML(htmlString: string): T { htmlString = htmlString.trim(); // some tags like "tr" are special, it must use a correct parent container to create + // eslint-disable-next-line github/unescaped-html-literal -- FIXME: maybe we need to use other approaches to create elements from HTML, e.g. using DOMParser if (htmlString.startsWith(' { + expect(html`${'<>&\'"'}`).toBe(`<>&'"`); + expect(html`${htmlRaw('')}`).toBe(``); + expect(html`${htmlRaw``}`).toBe(``); + expect(htmlEscape(``)).toBe(`<a></a>`); +}); diff --git a/web_src/js/utils/html.ts b/web_src/js/utils/html.ts new file mode 100644 index 0000000000..22e5703c34 --- /dev/null +++ b/web_src/js/utils/html.ts @@ -0,0 +1,32 @@ +export function htmlEscape(s: string, ...args: Array): string { + if (args.length !== 0) throw new Error('use html or htmlRaw instead of htmlEscape'); // check legacy usages + return s.replace(/&/g, '&') + .replace(/"/g, '"') + .replace(/'/g, ''') + .replace(//g, '>'); +} + +class rawObject { + private readonly value: string; + constructor(v: string) { this.value = v } + toString(): string { return this.value } +} + +export function html(tmpl: TemplateStringsArray, ...parts: Array): string { + let output = tmpl[0]; + for (let i = 0; i < parts.length; i++) { + const value = parts[i]; + const valueEscaped = (value instanceof rawObject) ? value.toString() : htmlEscape(String(parts[i])); + output = output + valueEscaped + tmpl[i + 1]; + } + return output; +} + +export function htmlRaw(s: string|TemplateStringsArray, ...tmplParts: Array): rawObject { + if (typeof s === 'string') { + if (tmplParts.length !== 0) throw new Error("either htmlRaw('str') or htmlRaw`tmpl`"); + return new rawObject(s); + } + return new rawObject(html(s, ...tmplParts)); +} From dd1fd89185103958cb504e362df91fd2f7856939 Mon Sep 17 00:00:00 2001 From: GiteaBot Date: Wed, 2 Jul 2025 00:37:55 +0000 Subject: [PATCH 05/13] [skip ci] Updated translations via Crowdin --- options/locale/locale_ga-IE.ini | 1 + options/locale/locale_pt-PT.ini | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/options/locale/locale_ga-IE.ini b/options/locale/locale_ga-IE.ini index e8c90d059b..d49da2b853 100644 --- a/options/locale/locale_ga-IE.ini +++ b/options/locale/locale_ga-IE.ini @@ -2782,6 +2782,7 @@ topic.done=Déanta topic.count_prompt=Ní féidir leat níos mó ná 25 topaicí a roghnú topic.format_prompt=Ní mór do thopaicí tosú le litir nó uimhir, is féidir daiseanna ('-') agus poncanna ('.') a áireamh, a bheith suas le 35 carachtar ar fad. Ní mór litreacha a bheith i litreacha beaga. +find_file.follow_symlink=Lean an nasc siombalach seo go dtí an áit a bhfuil sé ag pointeáil air find_file.go_to_file=Téigh go dtí an comhad find_file.no_matching=Níl aon chomhad meaitseála le fáil diff --git a/options/locale/locale_pt-PT.ini b/options/locale/locale_pt-PT.ini index 7dcdaac4c9..2b4d644ed9 100644 --- a/options/locale/locale_pt-PT.ini +++ b/options/locale/locale_pt-PT.ini @@ -1562,8 +1562,8 @@ issues.filter_project=Planeamento issues.filter_project_all=Todos os planeamentos issues.filter_project_none=Nenhum planeamento issues.filter_assignee=Encarregado -issues.filter_assignee_no_assignee=Não atribuído -issues.filter_assignee_any_assignee=Atribuído a qualquer pessoa +issues.filter_assignee_no_assignee=Não atribuída +issues.filter_assignee_any_assignee=Atribuída a alguém issues.filter_poster=Autor(a) issues.filter_user_placeholder=Procurar utilizadores issues.filter_user_no_select=Todos os utilizadores @@ -1969,6 +1969,7 @@ pulls.cmd_instruction_checkout_title=Checkout pulls.cmd_instruction_checkout_desc=A partir do seu repositório, crie um novo ramo e teste nele as modificações. pulls.cmd_instruction_merge_title=Integrar pulls.cmd_instruction_merge_desc=Integrar as modificações e enviar para o Gitea. +pulls.cmd_instruction_merge_warning=Aviso: Esta operação não pode executar pedidos de integração porque a opção "auto-identificar integração manual" não está habilitada. pulls.clear_merge_message=Apagar mensagem de integração pulls.clear_merge_message_hint=Apagar a mensagem de integração apenas remove o conteúdo da mensagem de cometimento e mantém os rodapés do git, tais como "Co-Autorado-Por …". @@ -2781,6 +2782,7 @@ topic.done=Concluído topic.count_prompt=Não pode escolher mais do que 25 tópicos topic.format_prompt=Os tópicos devem começar com uma letra ou um número, podem incluir traços ('-') ou pontos ('.') e podem ter até 35 caracteres. As letras têm que ser minúsculas. +find_file.follow_symlink=Seguir esta ligação simbólica para onde ela está apontando find_file.go_to_file=Ir para o ficheiro find_file.no_matching=Não foi encontrado qualquer ficheiro correspondente From 6fe5c4c4d983a79e24c42af61cff0c0999c7438f Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 3 Jul 2025 00:00:16 +0200 Subject: [PATCH 06/13] Exclude devtest.ts from tailwindcss (#34935) Fix this leftover from the typescript migration. --- tailwind.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tailwind.config.js b/tailwind.config.js index 01740d816b..fa233a9814 100644 --- a/tailwind.config.js +++ b/tailwind.config.js @@ -29,7 +29,7 @@ export default { important: true, // the frameworks are mixed together, so tailwind needs to override other framework's styles content: [ isProduction && '!./templates/devtest/**/*', - isProduction && '!./web_src/js/standalone/devtest.js', + isProduction && '!./web_src/js/standalone/devtest.ts', '!./templates/swagger/v1_json.tmpl', '!./templates/user/auth/oidc_wellknown.tmpl', '!**/*_test.go', From 97fc87af89a03470999a1a68e293417e36bd9b52 Mon Sep 17 00:00:00 2001 From: GiteaBot Date: Thu, 3 Jul 2025 00:37:58 +0000 Subject: [PATCH 07/13] [skip ci] Updated translations via Crowdin --- options/locale/locale_fr-FR.ini | 3 +++ options/locale/locale_ga-IE.ini | 2 ++ options/locale/locale_pt-PT.ini | 2 ++ 3 files changed, 7 insertions(+) diff --git a/options/locale/locale_fr-FR.ini b/options/locale/locale_fr-FR.ini index f09e9071fc..03614832e9 100644 --- a/options/locale/locale_fr-FR.ini +++ b/options/locale/locale_fr-FR.ini @@ -1969,6 +1969,7 @@ pulls.cmd_instruction_checkout_title=Basculer pulls.cmd_instruction_checkout_desc=Depuis votre dépôt, basculer sur une nouvelle branche et tester des modifications. pulls.cmd_instruction_merge_title=Fusionner pulls.cmd_instruction_merge_desc=Fusionner les modifications et mettre à jour sur Gitea. +pulls.cmd_instruction_merge_warning=Attention : cette opération ne peut pas fusionner la demande d’ajout car la « détection automatique de fusion manuelle » n’a pas été activée pulls.clear_merge_message=Effacer le message de fusion pulls.clear_merge_message_hint=Effacer le message de fusion ne supprimera que le message de la révision, mais pas les pieds de révision générés tels que "Co-Authored-By:". @@ -2768,6 +2769,8 @@ branch.new_branch_from=`Créer une nouvelle branche à partir de "%s"` branch.renamed=La branche %s à été renommée en %s. branch.rename_default_or_protected_branch_error=Seuls les administrateurs peuvent renommer les branches par défaut ou protégées. branch.rename_protected_branch_failed=Cette branche est protégée par des règles de protection basées sur des globs. +branch.commits_divergence_from=Divergence de révisions : %[1]d en retard et %[2]d en avance sur %[3]s +branch.commits_no_divergence=Identique à la branche %[1]s tag.create_tag=Créer l'étiquette %s tag.create_tag_operation=Créer une étiquette diff --git a/options/locale/locale_ga-IE.ini b/options/locale/locale_ga-IE.ini index d49da2b853..7c7cb58dcf 100644 --- a/options/locale/locale_ga-IE.ini +++ b/options/locale/locale_ga-IE.ini @@ -2769,6 +2769,8 @@ branch.new_branch_from=`Cruthaigh brainse nua ó "%s"` branch.renamed=Ainmníodh brainse %s go %s. branch.rename_default_or_protected_branch_error=Ní féidir ach le riarthóirí brainsí réamhshocraithe nó cosanta a athainmniú. branch.rename_protected_branch_failed=Tá an brainse seo faoi chosaint ag rialacha cosanta domhanda. +branch.commits_divergence_from=Déanann sé dialltacht a thiomnú: %[1]d taobh thiar agus %[2]d chun tosaigh ar %[3]s +branch.commits_no_divergence=Mar an gcéanna le brainse %[1]s tag.create_tag=Cruthaigh clib %s tag.create_tag_operation=Cruthaigh clib diff --git a/options/locale/locale_pt-PT.ini b/options/locale/locale_pt-PT.ini index 2b4d644ed9..740de78809 100644 --- a/options/locale/locale_pt-PT.ini +++ b/options/locale/locale_pt-PT.ini @@ -2769,6 +2769,8 @@ branch.new_branch_from=`Criar um novo ramo a partir do ramo "%s"` branch.renamed=O ramo %s foi renomeado para %s. branch.rename_default_or_protected_branch_error=Só os administradores é que podem renomear o ramo principal ou ramos protegidos. branch.rename_protected_branch_failed=Este ramo está protegido por regras de salvaguarda baseadas em padrões glob. +branch.commits_divergence_from=Divergência nos cometimentos: %[1]d atrás e %[2]d à frente de %[3]s +branch.commits_no_divergence=Idêntico ao ramo %[1]s tag.create_tag=Criar etiqueta %s tag.create_tag_operation=Criar etiqueta From 6455c8202beacf215ba96793e75b29859f904ae3 Mon Sep 17 00:00:00 2001 From: RickyMa Date: Thu, 3 Jul 2025 09:45:42 +0800 Subject: [PATCH 08/13] Support getting last commit message using contents-ext API (#34904) Fix #34870 Fix #34929 --------- Co-authored-by: wxiaoguang --- modules/structs/repo_file.go | 19 +++-- routers/api/v1/repo/file.go | 19 ++++- services/repository/files/content.go | 57 ++++++++------ services/repository/files/content_test.go | 74 +------------------ services/repository/files/file.go | 7 +- templates/swagger/v1_json.tmpl | 8 +- .../integration/api_repo_file_create_test.go | 11 +-- .../integration/api_repo_file_update_test.go | 7 +- .../api_repo_get_contents_list_test.go | 26 +++---- .../integration/api_repo_get_contents_test.go | 45 ++++++++--- tests/integration/repofiles_change_test.go | 18 ++--- 11 files changed, 141 insertions(+), 150 deletions(-) diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index 91ee060d50..5a86db868b 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -116,14 +116,17 @@ type ContentsExtResponse struct { // ContentsResponse contains information about a repo's entry's (dir, file, symlink, submodule) metadata and content type ContentsResponse struct { - Name string `json:"name"` - Path string `json:"path"` - SHA string `json:"sha"` - LastCommitSHA string `json:"last_commit_sha"` + Name string `json:"name"` + Path string `json:"path"` + SHA string `json:"sha"` + + LastCommitSHA *string `json:"last_commit_sha,omitempty"` // swagger:strfmt date-time - LastCommitterDate time.Time `json:"last_committer_date"` + LastCommitterDate *time.Time `json:"last_committer_date,omitempty"` // swagger:strfmt date-time - LastAuthorDate time.Time `json:"last_author_date"` + LastAuthorDate *time.Time `json:"last_author_date,omitempty"` + LastCommitMessage *string `json:"last_commit_message,omitempty"` + // `type` will be `file`, `dir`, `symlink`, or `submodule` Type string `json:"type"` Size int64 `json:"size"` @@ -141,8 +144,8 @@ type ContentsResponse struct { SubmoduleGitURL *string `json:"submodule_git_url"` Links *FileLinksResponse `json:"_links"` - LfsOid *string `json:"lfs_oid"` - LfsSize *int64 `json:"lfs_size"` + LfsOid *string `json:"lfs_oid,omitempty"` + LfsSize *int64 `json:"lfs_size,omitempty"` } // FileCommitResponse contains information generated from a Git commit for a repo's file. diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 8ce52c2cd4..69b5996222 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -812,7 +812,8 @@ func GetContentsExt(ctx *context.APIContext) { // required: true // - name: filepath // in: path - // description: path of the dir, file, symlink or submodule in the repo + // description: path of the dir, file, symlink or submodule in the repo. Swagger requires path parameter to be "required", + // you can leave it empty or pass a single dot (".") to get the root directory. // type: string // required: true // - name: ref @@ -823,7 +824,8 @@ func GetContentsExt(ctx *context.APIContext) { // - name: includes // in: query // description: By default this API's response only contains file's metadata. Use comma-separated "includes" options to retrieve more fields. - // Option "file_content" will try to retrieve the file content, option "lfs_metadata" will try to retrieve LFS metadata. + // Option "file_content" will try to retrieve the file content, "lfs_metadata" will try to retrieve LFS metadata, + // "commit_metadata" will try to retrieve commit metadata, and "commit_message" will try to retrieve commit message. // type: string // required: false // responses: @@ -832,6 +834,9 @@ func GetContentsExt(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" + if treePath := ctx.PathParam("*"); treePath == "." || treePath == "/" { + ctx.SetPathParam("*", "") // workaround for swagger, it requires path parameter to be "required", but we need to list root directory + } opts := files_service.GetContentsOrListOptions{TreePath: ctx.PathParam("*")} for includeOpt := range strings.SplitSeq(ctx.FormString("includes"), ",") { if includeOpt == "" { @@ -842,6 +847,10 @@ func GetContentsExt(ctx *context.APIContext) { opts.IncludeSingleFileContent = true case "lfs_metadata": opts.IncludeLfsMetadata = true + case "commit_metadata": + opts.IncludeCommitMetadata = true + case "commit_message": + opts.IncludeCommitMessage = true default: ctx.APIError(http.StatusBadRequest, fmt.Sprintf("unknown include option %q", includeOpt)) return @@ -883,7 +892,11 @@ func GetContents(ctx *context.APIContext) { // "$ref": "#/responses/ContentsResponse" // "404": // "$ref": "#/responses/notFound" - ret := getRepoContents(ctx, files_service.GetContentsOrListOptions{TreePath: ctx.PathParam("*"), IncludeSingleFileContent: true}) + ret := getRepoContents(ctx, files_service.GetContentsOrListOptions{ + TreePath: ctx.PathParam("*"), + IncludeSingleFileContent: true, + IncludeCommitMetadata: true, + }) if ctx.Written() { return } diff --git a/services/repository/files/content.go b/services/repository/files/content.go index beef381694..2c1e88bb59 100644 --- a/services/repository/files/content.go +++ b/services/repository/files/content.go @@ -39,6 +39,8 @@ type GetContentsOrListOptions struct { TreePath string IncludeSingleFileContent bool // include the file's content when the tree path is a file IncludeLfsMetadata bool + IncludeCommitMetadata bool + IncludeCommitMessage bool } // GetContentsOrList gets the metadata of a file's contents (*ContentsResponse) if treePath not a tree @@ -132,39 +134,46 @@ func getFileContentsByEntryInternal(_ context.Context, repo *repo_model.Reposito } selfURLString := selfURL.String() - err = gitRepo.AddLastCommitCache(repo.GetCommitsCountCacheKey(refCommit.InputRef, refType != git.RefTypeCommit), repo.FullName(), refCommit.CommitID) - if err != nil { - return nil, err - } - - lastCommit, err := refCommit.Commit.GetCommitByPath(opts.TreePath) - if err != nil { - return nil, err - } - // All content types have these fields in populated contentsResponse := &api.ContentsResponse{ - Name: entry.Name(), - Path: opts.TreePath, - SHA: entry.ID.String(), - LastCommitSHA: lastCommit.ID.String(), - Size: entry.Size(), - URL: &selfURLString, + Name: entry.Name(), + Path: opts.TreePath, + SHA: entry.ID.String(), + Size: entry.Size(), + URL: &selfURLString, Links: &api.FileLinksResponse{ Self: &selfURLString, }, } - // GitHub doesn't have these fields in the response, but we could follow other similar APIs to name them - // https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#list-commits - if lastCommit.Committer != nil { - contentsResponse.LastCommitterDate = lastCommit.Committer.When - } - if lastCommit.Author != nil { - contentsResponse.LastAuthorDate = lastCommit.Author.When + if opts.IncludeCommitMetadata || opts.IncludeCommitMessage { + err = gitRepo.AddLastCommitCache(repo.GetCommitsCountCacheKey(refCommit.InputRef, refType != git.RefTypeCommit), repo.FullName(), refCommit.CommitID) + if err != nil { + return nil, err + } + + lastCommit, err := refCommit.Commit.GetCommitByPath(opts.TreePath) + if err != nil { + return nil, err + } + + if opts.IncludeCommitMetadata { + contentsResponse.LastCommitSHA = util.ToPointer(lastCommit.ID.String()) + // GitHub doesn't have these fields in the response, but we could follow other similar APIs to name them + // https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#list-commits + if lastCommit.Committer != nil { + contentsResponse.LastCommitterDate = util.ToPointer(lastCommit.Committer.When) + } + if lastCommit.Author != nil { + contentsResponse.LastAuthorDate = util.ToPointer(lastCommit.Author.When) + } + } + if opts.IncludeCommitMessage { + contentsResponse.LastCommitMessage = util.ToPointer(lastCommit.Message()) + } } - // Now populate the rest of the ContentsResponse based on entry type + // Now populate the rest of the ContentsResponse based on the entry type if entry.IsRegular() || entry.IsExecutable() { contentsResponse.Type = string(ContentTypeRegular) // if it is listing the repo root dir, don't waste system resources on reading content diff --git a/services/repository/files/content_test.go b/services/repository/files/content_test.go index 9357c52ea8..d72f918074 100644 --- a/services/repository/files/content_test.go +++ b/services/repository/files/content_test.go @@ -5,56 +5,21 @@ package files import ( "testing" - "time" "code.gitea.io/gitea/models/unittest" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" - "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/services/contexttest" _ "code.gitea.io/gitea/models/actions" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestMain(m *testing.M) { unittest.MainTest(m) } -func getExpectedReadmeContentsResponse() *api.ContentsResponse { - treePath := "README.md" - sha := "4b4851ad51df6a7d9f25c979345979eaeb5b349f" - encoding := "base64" - content := "IyByZXBvMQoKRGVzY3JpcHRpb24gZm9yIHJlcG8x" - selfURL := "https://try.gitea.io/api/v1/repos/user2/repo1/contents/" + treePath + "?ref=master" - htmlURL := "https://try.gitea.io/user2/repo1/src/branch/master/" + treePath - gitURL := "https://try.gitea.io/api/v1/repos/user2/repo1/git/blobs/" + sha - downloadURL := "https://try.gitea.io/user2/repo1/raw/branch/master/" + treePath - return &api.ContentsResponse{ - Name: treePath, - Path: treePath, - SHA: "4b4851ad51df6a7d9f25c979345979eaeb5b349f", - LastCommitSHA: "65f1bf27bc3bf70f64657658635e66094edbcb4d", - LastCommitterDate: time.Date(2017, time.March, 19, 16, 47, 59, 0, time.FixedZone("", -14400)), - LastAuthorDate: time.Date(2017, time.March, 19, 16, 47, 59, 0, time.FixedZone("", -14400)), - Type: "file", - Size: 30, - Encoding: &encoding, - Content: &content, - URL: &selfURL, - HTMLURL: &htmlURL, - GitURL: &gitURL, - DownloadURL: &downloadURL, - Links: &api.FileLinksResponse{ - Self: &selfURL, - GitURL: &gitURL, - HTMLURL: &htmlURL, - }, - } -} - func TestGetContents(t *testing.T) { unittest.PrepareTestEnv(t) ctx, _ := contexttest.MockContext(t, "user2/repo1") @@ -63,45 +28,8 @@ func TestGetContents(t *testing.T) { contexttest.LoadRepoCommit(t, ctx) contexttest.LoadUser(t, ctx, 2) contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - repo, gitRepo := ctx.Repo.Repository, ctx.Repo.GitRepo - refCommit, err := utils.ResolveRefCommit(ctx, ctx.Repo.Repository, ctx.Repo.Repository.DefaultBranch) - require.NoError(t, err) - t.Run("GetContentsOrList(README.md)-MetaOnly", func(t *testing.T) { - expectedContentsResponse := getExpectedReadmeContentsResponse() - expectedContentsResponse.Encoding = nil // because will be in a list, doesn't have encoding and content - expectedContentsResponse.Content = nil - extResp, err := GetContentsOrList(ctx, repo, gitRepo, refCommit, GetContentsOrListOptions{TreePath: "README.md", IncludeSingleFileContent: false}) - assert.Equal(t, expectedContentsResponse, extResp.FileContents) - assert.NoError(t, err) - }) - - t.Run("GetContentsOrList(README.md)", func(t *testing.T) { - expectedContentsResponse := getExpectedReadmeContentsResponse() - extResp, err := GetContentsOrList(ctx, repo, gitRepo, refCommit, GetContentsOrListOptions{TreePath: "README.md", IncludeSingleFileContent: true}) - assert.Equal(t, expectedContentsResponse, extResp.FileContents) - assert.NoError(t, err) - }) - - t.Run("GetContentsOrList(RootDir)", func(t *testing.T) { - readmeContentsResponse := getExpectedReadmeContentsResponse() - readmeContentsResponse.Encoding = nil // because will be in a list, doesn't have encoding and content - readmeContentsResponse.Content = nil - expectedContentsListResponse := []*api.ContentsResponse{readmeContentsResponse} - // even if IncludeFileContent is true, it has no effect for directory listing - extResp, err := GetContentsOrList(ctx, repo, gitRepo, refCommit, GetContentsOrListOptions{TreePath: "", IncludeSingleFileContent: true}) - assert.Equal(t, expectedContentsListResponse, extResp.DirContents) - assert.NoError(t, err) - }) - - t.Run("GetContentsOrList(NoSuchTreePath)", func(t *testing.T) { - extResp, err := GetContentsOrList(ctx, repo, gitRepo, refCommit, GetContentsOrListOptions{TreePath: "no-such/file.md"}) - assert.Error(t, err) - assert.EqualError(t, err, "object does not exist [id: , rel_path: no-such]") - assert.Nil(t, extResp.DirContents) - assert.Nil(t, extResp.FileContents) - }) + // GetContentsOrList's behavior is fully tested in integration tests, so we don't need to test it here. t.Run("GetBlobBySHA", func(t *testing.T) { sha := "65f1bf27bc3bf70f64657658635e66094edbcb4d" diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 2a63a0a5b9..13d171d139 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -22,7 +22,12 @@ import ( func GetContentsListFromTreePaths(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, refCommit *utils.RefCommit, treePaths []string) (files []*api.ContentsResponse) { var size int64 for _, treePath := range treePaths { - fileContents, _ := GetFileContents(ctx, repo, gitRepo, refCommit, GetContentsOrListOptions{TreePath: treePath, IncludeSingleFileContent: true}) // ok if fails, then will be nil + // ok if fails, then will be nil + fileContents, _ := GetFileContents(ctx, repo, gitRepo, refCommit, GetContentsOrListOptions{ + TreePath: treePath, + IncludeSingleFileContent: true, + IncludeCommitMetadata: true, + }) if fileContents != nil && fileContents.Content != nil && *fileContents.Content != "" { // if content isn't empty (e.g., due to the single blob being too large), add file size to response size size += int64(len(*fileContents.Content)) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index ffac8edec2..879f59df2e 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7547,7 +7547,7 @@ }, { "type": "string", - "description": "path of the dir, file, symlink or submodule in the repo", + "description": "path of the dir, file, symlink or submodule in the repo. Swagger requires path parameter to be \"required\", you can leave it empty or pass a single dot (\".\") to get the root directory.", "name": "filepath", "in": "path", "required": true @@ -7560,7 +7560,7 @@ }, { "type": "string", - "description": "By default this API's response only contains file's metadata. Use comma-separated \"includes\" options to retrieve more fields. Option \"file_content\" will try to retrieve the file content, option \"lfs_metadata\" will try to retrieve LFS metadata.", + "description": "By default this API's response only contains file's metadata. Use comma-separated \"includes\" options to retrieve more fields. Option \"file_content\" will try to retrieve the file content, \"lfs_metadata\" will try to retrieve LFS metadata, \"commit_metadata\" will try to retrieve commit metadata, and \"commit_message\" will try to retrieve commit message.", "name": "includes", "in": "query" } @@ -22368,6 +22368,10 @@ "format": "date-time", "x-go-name": "LastAuthorDate" }, + "last_commit_message": { + "type": "string", + "x-go-name": "LastCommitMessage" + }, "last_commit_sha": { "type": "string", "x-go-name": "LastCommitSHA" diff --git a/tests/integration/api_repo_file_create_test.go b/tests/integration/api_repo_file_create_test.go index df0fc3dd05..af3bc54680 100644 --- a/tests/integration/api_repo_file_create_test.go +++ b/tests/integration/api_repo_file_create_test.go @@ -19,6 +19,7 @@ import ( "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/context" "github.com/stretchr/testify/assert" @@ -52,8 +53,8 @@ func getCreateFileOptions() api.CreateFileOptions { func normalizeFileContentResponseCommitTime(c *api.ContentsResponse) { // decoded JSON response may contain different timezone from the one parsed by git commit // so we need to normalize the time to UTC to make "assert.Equal" pass - c.LastCommitterDate = c.LastCommitterDate.UTC() - c.LastAuthorDate = c.LastAuthorDate.UTC() + c.LastCommitterDate = util.ToPointer(c.LastCommitterDate.UTC()) + c.LastAuthorDate = util.ToPointer(c.LastAuthorDate.UTC()) } type apiFileResponseInfo struct { @@ -74,9 +75,9 @@ func getExpectedFileResponseForCreate(info apiFileResponseInfo) *api.FileRespons Name: path.Base(info.treePath), Path: info.treePath, SHA: sha, - LastCommitSHA: info.lastCommitSHA, - LastCommitterDate: info.lastCommitterWhen, - LastAuthorDate: info.lastAuthorWhen, + LastCommitSHA: util.ToPointer(info.lastCommitSHA), + LastCommitterDate: util.ToPointer(info.lastCommitterWhen), + LastAuthorDate: util.ToPointer(info.lastAuthorWhen), Size: 16, Type: "file", Encoding: &encoding, diff --git a/tests/integration/api_repo_file_update_test.go b/tests/integration/api_repo_file_update_test.go index 6a7f7529a0..9a56711da6 100644 --- a/tests/integration/api_repo_file_update_test.go +++ b/tests/integration/api_repo_file_update_test.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/context" "github.com/stretchr/testify/assert" @@ -60,9 +61,9 @@ func getExpectedFileResponseForUpdate(info apiFileResponseInfo) *api.FileRespons Name: path.Base(info.treePath), Path: info.treePath, SHA: sha, - LastCommitSHA: info.lastCommitSHA, - LastCommitterDate: info.lastCommitterWhen, - LastAuthorDate: info.lastAuthorWhen, + LastCommitSHA: util.ToPointer(info.lastCommitSHA), + LastCommitterDate: util.ToPointer(info.lastCommitterWhen), + LastAuthorDate: util.ToPointer(info.lastAuthorWhen), Type: "file", Size: 20, Encoding: &encoding, diff --git a/tests/integration/api_repo_get_contents_list_test.go b/tests/integration/api_repo_get_contents_list_test.go index 9b192c6304..563d6fcc10 100644 --- a/tests/integration/api_repo_get_contents_list_test.go +++ b/tests/integration/api_repo_get_contents_list_test.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" repo_service "code.gitea.io/gitea/services/repository" "github.com/stretchr/testify/assert" @@ -35,9 +36,9 @@ func getExpectedContentsListResponseForContents(ref, refType, lastCommitSHA stri Name: path.Base(treePath), Path: treePath, SHA: sha, - LastCommitSHA: lastCommitSHA, - LastCommitterDate: time.Date(2017, time.March, 19, 16, 47, 59, 0, time.FixedZone("", -14400)), - LastAuthorDate: time.Date(2017, time.March, 19, 16, 47, 59, 0, time.FixedZone("", -14400)), + LastCommitSHA: util.ToPointer(lastCommitSHA), + LastCommitterDate: util.ToPointer(time.Date(2017, time.March, 19, 16, 47, 59, 0, time.FixedZone("", -14400))), + LastAuthorDate: util.ToPointer(time.Date(2017, time.March, 19, 16, 47, 59, 0, time.FixedZone("", -14400))), Type: "file", Size: 30, URL: &selfURL, @@ -65,7 +66,6 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) { repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) // public repo repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) // public repo repo16 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 16}) // private repo - treePath := "" // root dir // Get user2's token session := loginUser(t, user2.Name) @@ -94,7 +94,7 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) { // ref is default ref ref := repo1.DefaultBranch refType := "branch" - req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref) + req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents?ref=%s", user2.Name, repo1.Name, ref) resp := MakeRequest(t, req, http.StatusOK) var contentsListResponse []*api.ContentsResponse DecodeJSON(t, resp, &contentsListResponse) @@ -106,7 +106,7 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) { // No ref refType = "branch" - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath) + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/", user2.Name, repo1.Name) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &contentsListResponse) assert.NotNil(t, contentsListResponse) @@ -117,7 +117,7 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) { // ref is the branch we created above in setup ref = newBranch refType = "branch" - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref) + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents?ref=%s", user2.Name, repo1.Name, ref) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &contentsListResponse) assert.NotNil(t, contentsListResponse) @@ -131,7 +131,7 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) { // ref is the new tag we created above in setup ref = newTag refType = "tag" - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref) + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/?ref=%s", user2.Name, repo1.Name, ref) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &contentsListResponse) assert.NotNil(t, contentsListResponse) @@ -145,7 +145,7 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) { // ref is a commit ref = commitID refType = "commit" - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref) + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/?ref=%s", user2.Name, repo1.Name, ref) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &contentsListResponse) assert.NotNil(t, contentsListResponse) @@ -154,21 +154,21 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) { // Test file contents a file with a bad ref ref = "badref" - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref) + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/?ref=%s", user2.Name, repo1.Name, ref) MakeRequest(t, req, http.StatusNotFound) // Test accessing private ref with user token that does not have access - should fail - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s", user2.Name, repo16.Name, treePath). + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/", user2.Name, repo16.Name). AddTokenAuth(token4) MakeRequest(t, req, http.StatusNotFound) // Test access private ref of owner of token - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/readme.md", user2.Name, repo16.Name). + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/", user2.Name, repo16.Name). AddTokenAuth(token2) MakeRequest(t, req, http.StatusOK) // Test access of org org3 private repo file by owner user2 - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s", org3.Name, repo3.Name, treePath). + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/", org3.Name, repo3.Name). AddTokenAuth(token2) MakeRequest(t, req, http.StatusOK) } diff --git a/tests/integration/api_repo_get_contents_test.go b/tests/integration/api_repo_get_contents_test.go index 9517db4c87..33df74f6ee 100644 --- a/tests/integration/api_repo_get_contents_test.go +++ b/tests/integration/api_repo_get_contents_test.go @@ -35,9 +35,9 @@ func getExpectedContentsResponseForContents(ref, refType, lastCommitSHA string) Name: treePath, Path: treePath, SHA: "4b4851ad51df6a7d9f25c979345979eaeb5b349f", - LastCommitSHA: lastCommitSHA, - LastCommitterDate: time.Date(2017, time.March, 19, 16, 47, 59, 0, time.FixedZone("", -14400)), - LastAuthorDate: time.Date(2017, time.March, 19, 16, 47, 59, 0, time.FixedZone("", -14400)), + LastCommitSHA: util.ToPointer(lastCommitSHA), + LastCommitterDate: util.ToPointer(time.Date(2017, time.March, 19, 16, 47, 59, 0, time.FixedZone("", -14400))), + LastAuthorDate: util.ToPointer(time.Date(2017, time.March, 19, 16, 47, 59, 0, time.FixedZone("", -14400))), Type: "file", Size: 30, Encoding: util.ToPointer("base64"), @@ -97,11 +97,16 @@ func testAPIGetContents(t *testing.T, u *url.URL) { require.NoError(t, err) /*** END SETUP ***/ + // not found + req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/no-such/file.md", user2.Name, repo1.Name) + resp := MakeRequest(t, req, http.StatusNotFound) + assert.Contains(t, resp.Body.String(), "object does not exist [id: , rel_path: no-such]") + // ref is default ref ref := repo1.DefaultBranch refType := "branch" - req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref) - resp := MakeRequest(t, req, http.StatusOK) + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref) + resp = MakeRequest(t, req, http.StatusOK) var contentsResponse api.ContentsResponse DecodeJSON(t, resp, &contentsResponse) lastCommit, _ := gitRepo.GetCommitByPath("README.md") @@ -116,7 +121,7 @@ func testAPIGetContents(t *testing.T, u *url.URL) { expectedContentsResponse = getExpectedContentsResponseForContents(repo1.DefaultBranch, refType, lastCommit.ID.String()) assert.Equal(t, *expectedContentsResponse, contentsResponse) - // ref is the branch we created above in setup + // ref is the branch we created above in setup ref = newBranch refType = "branch" req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref) @@ -206,14 +211,30 @@ func testAPIGetContentsExt(t *testing.T) { session := loginUser(t, "user2") token2 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) t.Run("DirContents", func(t *testing.T) { - req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo1/contents-ext/docs?ref=sub-home-md-img-check") + req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo1/contents-ext?ref=sub-home-md-img-check") resp := MakeRequest(t, req, http.StatusOK) var contentsResponse api.ContentsExtResponse DecodeJSON(t, resp, &contentsResponse) assert.Nil(t, contentsResponse.FileContents) + assert.NotNil(t, contentsResponse.DirContents) + + req = NewRequestf(t, "GET", "/api/v1/repos/user2/repo1/contents-ext/.?ref=sub-home-md-img-check") + resp = MakeRequest(t, req, http.StatusOK) + contentsResponse = api.ContentsExtResponse{} + DecodeJSON(t, resp, &contentsResponse) + assert.Nil(t, contentsResponse.FileContents) + assert.NotNil(t, contentsResponse.DirContents) + + req = NewRequestf(t, "GET", "/api/v1/repos/user2/repo1/contents-ext/docs?ref=sub-home-md-img-check") + resp = MakeRequest(t, req, http.StatusOK) + contentsResponse = api.ContentsExtResponse{} + DecodeJSON(t, resp, &contentsResponse) + assert.Nil(t, contentsResponse.FileContents) assert.Equal(t, "README.md", contentsResponse.DirContents[0].Name) assert.Nil(t, contentsResponse.DirContents[0].Encoding) assert.Nil(t, contentsResponse.DirContents[0].Content) + assert.Nil(t, contentsResponse.DirContents[0].LastCommitSHA) + assert.Nil(t, contentsResponse.DirContents[0].LastCommitMessage) // "includes=file_content" shouldn't affect directory listing req = NewRequestf(t, "GET", "/api/v1/repos/user2/repo1/contents-ext/docs?ref=sub-home-md-img-check&includes=file_content") @@ -240,7 +261,7 @@ func testAPIGetContentsExt(t *testing.T) { assert.Equal(t, util.ToPointer("0b8d8b5f15046343fd32f451df93acc2bdd9e6373be478b968e4cad6b6647351"), respFile.LfsOid) }) t.Run("FileContents", func(t *testing.T) { - // by default, no file content is returned + // by default, no file content or commit info is returned req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo1/contents-ext/docs/README.md?ref=sub-home-md-img-check") resp := MakeRequest(t, req, http.StatusOK) var contentsResponse api.ContentsExtResponse @@ -249,9 +270,11 @@ func testAPIGetContentsExt(t *testing.T) { assert.Equal(t, "README.md", contentsResponse.FileContents.Name) assert.Nil(t, contentsResponse.FileContents.Encoding) assert.Nil(t, contentsResponse.FileContents.Content) + assert.Nil(t, contentsResponse.FileContents.LastCommitSHA) + assert.Nil(t, contentsResponse.FileContents.LastCommitMessage) // file content is only returned when `includes=file_content` - req = NewRequestf(t, "GET", "/api/v1/repos/user2/repo1/contents-ext/docs/README.md?ref=sub-home-md-img-check&includes=file_content") + req = NewRequestf(t, "GET", "/api/v1/repos/user2/repo1/contents-ext/docs/README.md?ref=sub-home-md-img-check&includes=file_content,commit_metadata,commit_message") resp = MakeRequest(t, req, http.StatusOK) contentsResponse = api.ContentsExtResponse{} DecodeJSON(t, resp, &contentsResponse) @@ -259,6 +282,8 @@ func testAPIGetContentsExt(t *testing.T) { assert.Equal(t, "README.md", contentsResponse.FileContents.Name) assert.NotNil(t, contentsResponse.FileContents.Encoding) assert.NotNil(t, contentsResponse.FileContents.Content) + assert.Equal(t, "4649299398e4d39a5c09eb4f534df6f1e1eb87cc", *contentsResponse.FileContents.LastCommitSHA) + assert.Equal(t, "Test how READMEs render images when found in a subfolder\n", *contentsResponse.FileContents.LastCommitMessage) req = NewRequestf(t, "GET", "/api/v1/repos/user2/lfs/contents-ext/jpeg.jpg?includes=file_content").AddTokenAuth(token2) resp = session.MakeRequest(t, req, http.StatusOK) @@ -270,6 +295,8 @@ func testAPIGetContentsExt(t *testing.T) { assert.Equal(t, "jpeg.jpg", respFile.Name) assert.NotNil(t, respFile.Encoding) assert.NotNil(t, respFile.Content) + assert.Nil(t, contentsResponse.FileContents.LastCommitSHA) + assert.Nil(t, contentsResponse.FileContents.LastCommitMessage) assert.Equal(t, util.ToPointer(int64(107)), respFile.LfsSize) assert.Equal(t, util.ToPointer("0b8d8b5f15046343fd32f451df93acc2bdd9e6373be478b968e4cad6b6647351"), respFile.LfsOid) }) diff --git a/tests/integration/repofiles_change_test.go b/tests/integration/repofiles_change_test.go index b63d06a866..dc389f5680 100644 --- a/tests/integration/repofiles_change_test.go +++ b/tests/integration/repofiles_change_test.go @@ -155,9 +155,9 @@ func getExpectedFileResponseForRepoFilesCreate(commitID string, lastCommit *git. Name: path.Base(treePath), Path: treePath, SHA: "103ff9234cefeee5ec5361d22b49fbb04d385885", - LastCommitSHA: lastCommit.ID.String(), - LastCommitterDate: lastCommit.Committer.When, - LastAuthorDate: lastCommit.Author.When, + LastCommitSHA: util.ToPointer(lastCommit.ID.String()), + LastCommitterDate: util.ToPointer(lastCommit.Committer.When), + LastAuthorDate: util.ToPointer(lastCommit.Author.When), Type: "file", Size: 18, Encoding: &encoding, @@ -198,7 +198,7 @@ func getExpectedFileResponseForRepoFilesCreate(commitID string, lastCommit *git. SHA: "65f1bf27bc3bf70f64657658635e66094edbcb4d", }, }, - Message: "Updates README.md\n", + Message: "Creates new/file.txt\n", Tree: &api.CommitMeta{ URL: setting.AppURL + "api/v1/repos/user2/repo1/git/trees/f93e3a1a1525fb5b91020da86e44810c87a2d7bc", SHA: "f93e3a1a1525fb5b91020git dda86e44810c87a2d7bc", @@ -225,9 +225,9 @@ func getExpectedFileResponseForRepoFilesUpdate(commitID, filename, lastCommitSHA Name: filename, Path: filename, SHA: "dbf8d00e022e05b7e5cf7e535de857de57925647", - LastCommitSHA: lastCommitSHA, - LastCommitterDate: lastCommitterWhen, - LastAuthorDate: lastAuthorWhen, + LastCommitSHA: util.ToPointer(lastCommitSHA), + LastCommitterDate: util.ToPointer(lastCommitterWhen), + LastAuthorDate: util.ToPointer(lastAuthorWhen), Type: "file", Size: 43, Encoding: &encoding, @@ -331,7 +331,7 @@ func getExpectedFileResponseForRepoFilesUpdateRename(commitID, lastCommitSHA str Name: detail.filename, Path: detail.filename, SHA: detail.sha, - LastCommitSHA: lastCommitSHA, + LastCommitSHA: util.ToPointer(lastCommitSHA), Type: "file", Size: detail.size, Encoding: util.ToPointer("base64"), @@ -537,7 +537,7 @@ func TestChangeRepoFilesForUpdateWithFileRename(t *testing.T) { lastCommit, _ := commit.GetCommitByPath(opts.Files[0].TreePath) expectedFileResponse := getExpectedFileResponseForRepoFilesUpdateRename(commit.ID.String(), lastCommit.ID.String()) for _, file := range filesResponse.Files { - file.LastCommitterDate, file.LastAuthorDate = time.Time{}, time.Time{} // there might be different time in one operation, so we ignore them + file.LastCommitterDate, file.LastAuthorDate = nil, nil // there might be different time in one operation, so we ignore them } assert.Len(t, filesResponse.Files, 4) assert.Equal(t, expectedFileResponse.Files, filesResponse.Files) From 8cbec63cc70f9ebbe1558123d95cbe63b2f31782 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 3 Jul 2025 10:35:45 +0800 Subject: [PATCH 09/13] Don't send trigger for a pending review's comment create/update/delete (#34928) Fix #18846 Fix #34924 --------- Co-authored-by: wxiaoguang --- models/issues/comment.go | 10 +++------- services/notify/notify.go | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index 9bef96d0dd..4fdb0c1808 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -715,7 +715,8 @@ func (c *Comment) LoadReactions(ctx context.Context, repo *repo_model.Repository return nil } -func (c *Comment) loadReview(ctx context.Context) (err error) { +// LoadReview loads the associated review +func (c *Comment) LoadReview(ctx context.Context) (err error) { if c.ReviewID == 0 { return nil } @@ -732,11 +733,6 @@ func (c *Comment) loadReview(ctx context.Context) (err error) { return nil } -// LoadReview loads the associated review -func (c *Comment) LoadReview(ctx context.Context) error { - return c.loadReview(ctx) -} - // DiffSide returns "previous" if Comment.Line is a LOC of the previous changes and "proposed" if it is a LOC of the proposed changes. func (c *Comment) DiffSide() string { if c.Line < 0 { @@ -856,7 +852,7 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment } if comment.ReviewID != 0 { if comment.Review == nil { - if err := comment.loadReview(ctx); err != nil { + if err := comment.LoadReview(ctx); err != nil { return err } } diff --git a/services/notify/notify.go b/services/notify/notify.go index 0c6fdf9cef..2416cbd2e0 100644 --- a/services/notify/notify.go +++ b/services/notify/notify.go @@ -46,10 +46,25 @@ func DeleteWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model } } +func shouldSendCommentChangeNotification(ctx context.Context, comment *issues_model.Comment) bool { + if err := comment.LoadReview(ctx); err != nil { + log.Error("LoadReview: %v", err) + return false + } else if comment.Review != nil && comment.Review.Type == issues_model.ReviewTypePending { + // Pending review comments updating should not triggered + return false + } + return true +} + // CreateIssueComment notifies issue comment related message to notifiers func CreateIssueComment(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, comment *issues_model.Comment, mentions []*user_model.User, ) { + if !shouldSendCommentChangeNotification(ctx, comment) { + return + } + for _, notifier := range notifiers { notifier.CreateIssueComment(ctx, doer, repo, issue, comment, mentions) } @@ -156,6 +171,10 @@ func PullReviewDismiss(ctx context.Context, doer *user_model.User, review *issue // UpdateComment notifies update comment to notifiers func UpdateComment(ctx context.Context, doer *user_model.User, c *issues_model.Comment, oldContent string) { + if !shouldSendCommentChangeNotification(ctx, c) { + return + } + for _, notifier := range notifiers { notifier.UpdateComment(ctx, doer, c, oldContent) } @@ -163,6 +182,10 @@ func UpdateComment(ctx context.Context, doer *user_model.User, c *issues_model.C // DeleteComment notifies delete comment to notifiers func DeleteComment(ctx context.Context, doer *user_model.User, c *issues_model.Comment) { + if !shouldSendCommentChangeNotification(ctx, c) { + return + } + for _, notifier := range notifiers { notifier.DeleteComment(ctx, doer, c) } From d6d643fe86f125ee7fdda82264602b7e8db2a36b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 3 Jul 2025 11:02:38 +0800 Subject: [PATCH 10/13] Fix http auth header parsing (#34936) Using `strings.EqualFold` is wrong in many cases. --- modules/auth/httpauth/httpauth.go | 47 ++++++++++++++++++++++++++ modules/auth/httpauth/httpauth_test.go | 43 +++++++++++++++++++++++ modules/base/tool.go | 16 --------- modules/base/tool_test.go | 19 ----------- modules/util/string.go | 21 ++++++++++++ routers/web/auth/oauth2_provider.go | 29 ++++++---------- services/auth/basic.go | 15 ++++---- services/auth/oauth2.go | 7 ++-- services/lfs/server.go | 17 +++------- 9 files changed, 136 insertions(+), 78 deletions(-) create mode 100644 modules/auth/httpauth/httpauth.go create mode 100644 modules/auth/httpauth/httpauth_test.go diff --git a/modules/auth/httpauth/httpauth.go b/modules/auth/httpauth/httpauth.go new file mode 100644 index 0000000000..7f1f1ee152 --- /dev/null +++ b/modules/auth/httpauth/httpauth.go @@ -0,0 +1,47 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package httpauth + +import ( + "encoding/base64" + "strings" + + "code.gitea.io/gitea/modules/util" +) + +type BasicAuth struct { + Username, Password string +} + +type BearerToken struct { + Token string +} + +type ParsedAuthorizationHeader struct { + BasicAuth *BasicAuth + BearerToken *BearerToken +} + +func ParseAuthorizationHeader(header string) (ret ParsedAuthorizationHeader, _ bool) { + parts := strings.Fields(header) + if len(parts) != 2 { + return ret, false + } + if util.AsciiEqualFold(parts[0], "basic") { + s, err := base64.StdEncoding.DecodeString(parts[1]) + if err != nil { + return ret, false + } + u, p, ok := strings.Cut(string(s), ":") + if !ok { + return ret, false + } + ret.BasicAuth = &BasicAuth{Username: u, Password: p} + return ret, true + } else if util.AsciiEqualFold(parts[0], "token") || util.AsciiEqualFold(parts[0], "bearer") { + ret.BearerToken = &BearerToken{Token: parts[1]} + return ret, true + } + return ret, false +} diff --git a/modules/auth/httpauth/httpauth_test.go b/modules/auth/httpauth/httpauth_test.go new file mode 100644 index 0000000000..087b86917f --- /dev/null +++ b/modules/auth/httpauth/httpauth_test.go @@ -0,0 +1,43 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package httpauth + +import ( + "encoding/base64" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseAuthorizationHeader(t *testing.T) { + type parsed = ParsedAuthorizationHeader + type basic = BasicAuth + type bearer = BearerToken + cases := []struct { + headerValue string + expected parsed + ok bool + }{ + {"", parsed{}, false}, + {"?", parsed{}, false}, + {"foo", parsed{}, false}, + {"any value", parsed{}, false}, + + {"Basic ?", parsed{}, false}, + {"Basic " + base64.StdEncoding.EncodeToString([]byte("foo")), parsed{}, false}, + {"Basic " + base64.StdEncoding.EncodeToString([]byte("foo:bar")), parsed{BasicAuth: &basic{"foo", "bar"}}, true}, + {"basic " + base64.StdEncoding.EncodeToString([]byte("foo:bar")), parsed{BasicAuth: &basic{"foo", "bar"}}, true}, + + {"token value", parsed{BearerToken: &bearer{"value"}}, true}, + {"Token value", parsed{BearerToken: &bearer{"value"}}, true}, + {"bearer value", parsed{BearerToken: &bearer{"value"}}, true}, + {"Bearer value", parsed{BearerToken: &bearer{"value"}}, true}, + {"Bearer wrong value", parsed{}, false}, + } + for _, c := range cases { + ret, ok := ParseAuthorizationHeader(c.headerValue) + assert.Equal(t, c.ok, ok, "header %q", c.headerValue) + assert.Equal(t, c.expected, ret, "header %q", c.headerValue) + } +} diff --git a/modules/base/tool.go b/modules/base/tool.go index 02ca85569e..ed94575e74 100644 --- a/modules/base/tool.go +++ b/modules/base/tool.go @@ -8,13 +8,10 @@ import ( "crypto/sha1" "crypto/sha256" "crypto/subtle" - "encoding/base64" "encoding/hex" - "errors" "fmt" "hash" "strconv" - "strings" "time" "code.gitea.io/gitea/modules/setting" @@ -36,19 +33,6 @@ func ShortSha(sha1 string) string { return util.TruncateRunes(sha1, 10) } -// BasicAuthDecode decode basic auth string -func BasicAuthDecode(encoded string) (string, string, error) { - s, err := base64.StdEncoding.DecodeString(encoded) - if err != nil { - return "", "", err - } - - if username, password, ok := strings.Cut(string(s), ":"); ok { - return username, password, nil - } - return "", "", errors.New("invalid basic authentication") -} - // VerifyTimeLimitCode verify time limit code func VerifyTimeLimitCode(now time.Time, data string, minutes int, code string) bool { if len(code) <= 18 { diff --git a/modules/base/tool_test.go b/modules/base/tool_test.go index 7cebedb073..b7365e40c4 100644 --- a/modules/base/tool_test.go +++ b/modules/base/tool_test.go @@ -26,25 +26,6 @@ func TestShortSha(t *testing.T) { assert.Equal(t, "veryverylo", ShortSha("veryverylong")) } -func TestBasicAuthDecode(t *testing.T) { - _, _, err := BasicAuthDecode("?") - assert.Equal(t, "illegal base64 data at input byte 0", err.Error()) - - user, pass, err := BasicAuthDecode("Zm9vOmJhcg==") - assert.NoError(t, err) - assert.Equal(t, "foo", user) - assert.Equal(t, "bar", pass) - - _, _, err = BasicAuthDecode("aW52YWxpZA==") - assert.Error(t, err) - - _, _, err = BasicAuthDecode("invalid") - assert.Error(t, err) - - _, _, err = BasicAuthDecode("YWxpY2U=") // "alice", no colon - assert.Error(t, err) -} - func TestVerifyTimeLimitCode(t *testing.T) { defer test.MockVariableValue(&setting.InstallLock, true)() initGeneralSecret := func(secret string) { diff --git a/modules/util/string.go b/modules/util/string.go index 03c0df96a3..b9b59df3ef 100644 --- a/modules/util/string.go +++ b/modules/util/string.go @@ -110,3 +110,24 @@ func SplitTrimSpace(input, sep string) []string { } return stringList } + +func asciiLower(b byte) byte { + if 'A' <= b && b <= 'Z' { + return b + ('a' - 'A') + } + return b +} + +// AsciiEqualFold is from Golang https://cs.opensource.google/go/go/+/refs/tags/go1.24.4:src/net/http/internal/ascii/print.go +// ASCII only. In most cases for protocols, we should only use this but not [strings.EqualFold] +func AsciiEqualFold(s, t string) bool { //nolint:revive // PascalCase + if len(s) != len(t) { + return false + } + for i := 0; i < len(s); i++ { + if asciiLower(s[i]) != asciiLower(t[i]) { + return false + } + } + return true +} diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index 1804b5b193..dc9f34fd44 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/routers/web/auth/oauth2_provider.go @@ -4,18 +4,16 @@ package auth import ( - "errors" "fmt" "html" "html/template" "net/http" "net/url" "strconv" - "strings" "code.gitea.io/gitea/models/auth" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/auth/httpauth" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -108,9 +106,8 @@ func InfoOAuth(ctx *context.Context) { var accessTokenScope auth.AccessTokenScope if auHead := ctx.Req.Header.Get("Authorization"); auHead != "" { - auths := strings.Fields(auHead) - if len(auths) == 2 && (auths[0] == "token" || strings.ToLower(auths[0]) == "bearer") { - accessTokenScope, _ = auth_service.GetOAuthAccessTokenScopeAndUserID(ctx, auths[1]) + if parsed, ok := httpauth.ParseAuthorizationHeader(auHead); ok && parsed.BearerToken != nil { + accessTokenScope, _ = auth_service.GetOAuthAccessTokenScopeAndUserID(ctx, parsed.BearerToken.Token) } } @@ -127,18 +124,12 @@ func InfoOAuth(ctx *context.Context) { ctx.JSON(http.StatusOK, response) } -func parseBasicAuth(ctx *context.Context) (username, password string, err error) { - authHeader := ctx.Req.Header.Get("Authorization") - if authType, authData, ok := strings.Cut(authHeader, " "); ok && strings.EqualFold(authType, "Basic") { - return base.BasicAuthDecode(authData) - } - return "", "", errors.New("invalid basic authentication") -} - // IntrospectOAuth introspects an oauth token func IntrospectOAuth(ctx *context.Context) { clientIDValid := false - if clientID, clientSecret, err := parseBasicAuth(ctx); err == nil { + authHeader := ctx.Req.Header.Get("Authorization") + if parsed, ok := httpauth.ParseAuthorizationHeader(authHeader); ok && parsed.BasicAuth != nil { + clientID, clientSecret := parsed.BasicAuth.Username, parsed.BasicAuth.Password app, err := auth.GetOAuth2ApplicationByClientID(ctx, clientID) if err != nil && !auth.IsErrOauthClientIDInvalid(err) { // this is likely a database error; log it and respond without details @@ -465,16 +456,16 @@ func AccessTokenOAuth(ctx *context.Context) { form := *web.GetForm(ctx).(*forms.AccessTokenForm) // if there is no ClientID or ClientSecret in the request body, fill these fields by the Authorization header and ensure the provided field matches the Authorization header if form.ClientID == "" || form.ClientSecret == "" { - authHeader := ctx.Req.Header.Get("Authorization") - if authType, authData, ok := strings.Cut(authHeader, " "); ok && strings.EqualFold(authType, "Basic") { - clientID, clientSecret, err := base.BasicAuthDecode(authData) - if err != nil { + if authHeader := ctx.Req.Header.Get("Authorization"); authHeader != "" { + parsed, ok := httpauth.ParseAuthorizationHeader(authHeader) + if !ok || parsed.BasicAuth == nil { handleAccessTokenError(ctx, oauth2_provider.AccessTokenError{ ErrorCode: oauth2_provider.AccessTokenErrorCodeInvalidRequest, ErrorDescription: "cannot parse basic auth header", }) return } + clientID, clientSecret := parsed.BasicAuth.Username, parsed.BasicAuth.Password // validate that any fields present in the form match the Basic auth header if form.ClientID != "" && form.ClientID != clientID { handleAccessTokenError(ctx, oauth2_provider.AccessTokenError{ diff --git a/services/auth/basic.go b/services/auth/basic.go index a208590d7b..b2bd14ef5d 100644 --- a/services/auth/basic.go +++ b/services/auth/basic.go @@ -7,12 +7,11 @@ package auth import ( "errors" "net/http" - "strings" actions_model "code.gitea.io/gitea/models/actions" auth_model "code.gitea.io/gitea/models/auth" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/auth/httpauth" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" @@ -54,17 +53,15 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore return nil, nil } - baHead := req.Header.Get("Authorization") - if len(baHead) == 0 { + authHeader := req.Header.Get("Authorization") + if authHeader == "" { return nil, nil } - - auths := strings.SplitN(baHead, " ", 2) - if len(auths) != 2 || (strings.ToLower(auths[0]) != "basic") { + parsed, ok := httpauth.ParseAuthorizationHeader(authHeader) + if !ok || parsed.BasicAuth == nil { return nil, nil } - - uname, passwd, _ := base.BasicAuthDecode(auths[1]) + uname, passwd := parsed.BasicAuth.Username, parsed.BasicAuth.Password // Check if username or password is a token isUsernameToken := len(passwd) == 0 || passwd == "x-oauth-basic" diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index 66cc686809..7df6f4638e 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -13,6 +13,7 @@ import ( actions_model "code.gitea.io/gitea/models/actions" auth_model "code.gitea.io/gitea/models/auth" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/auth/httpauth" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" @@ -97,9 +98,9 @@ func parseToken(req *http.Request) (string, bool) { // check header token if auHead := req.Header.Get("Authorization"); auHead != "" { - auths := strings.Fields(auHead) - if len(auths) == 2 && (auths[0] == "token" || strings.ToLower(auths[0]) == "bearer") { - return auths[1], true + parsed, ok := httpauth.ParseAuthorizationHeader(auHead) + if ok && parsed.BearerToken != nil { + return parsed.BearerToken.Token, true } } return "", false diff --git a/services/lfs/server.go b/services/lfs/server.go index 15a51ad534..c44cc35e53 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -27,6 +27,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/auth/httpauth" "code.gitea.io/gitea/modules/json" lfs_module "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" @@ -594,19 +595,11 @@ func parseToken(ctx stdCtx.Context, authorization string, target *repo_model.Rep if authorization == "" { return nil, errors.New("no token") } - - parts := strings.SplitN(authorization, " ", 2) - if len(parts) != 2 { - return nil, errors.New("no token") + parsed, ok := httpauth.ParseAuthorizationHeader(authorization) + if !ok || parsed.BearerToken == nil { + return nil, errors.New("token not found") } - tokenSHA := parts[1] - switch strings.ToLower(parts[0]) { - case "bearer": - fallthrough - case "token": - return handleLFSToken(ctx, tokenSHA, target, mode) - } - return nil, errors.New("token not found") + return handleLFSToken(ctx, parsed.BearerToken.Token, target, mode) } func requireAuth(ctx *context.Context) { From 71e151cc2244c05c616a8923e0d6299c76c954b2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 4 Jul 2025 19:03:22 +0800 Subject: [PATCH 11/13] Refactor head navbar icons (#34922) Co-authored-by: silverwind Co-authored-by: Giteabot --- routers/common/pagetmpl.go | 28 +++++--- routers/web/web.go | 2 +- templates/base/head_navbar.tmpl | 53 +------------- templates/base/head_navbar_icons.tmpl | 25 +++++++ templates/swagger/ui.tmpl | 1 + .../user/notification/notification_div.tmpl | 2 +- tests/integration/integration_test.go | 3 + tests/integration/links_test.go | 69 +++++++++---------- web_src/css/modules/navbar.css | 13 ---- 9 files changed, 83 insertions(+), 113 deletions(-) create mode 100644 templates/base/head_navbar_icons.tmpl diff --git a/routers/common/pagetmpl.go b/routers/common/pagetmpl.go index 52c9fceba3..c48596d48b 100644 --- a/routers/common/pagetmpl.go +++ b/routers/common/pagetmpl.go @@ -6,6 +6,7 @@ package common import ( goctx "context" "errors" + "sync" activities_model "code.gitea.io/gitea/models/activities" "code.gitea.io/gitea/models/db" @@ -22,8 +23,7 @@ type StopwatchTmplInfo struct { Seconds int64 } -func getActiveStopwatch(goCtx goctx.Context) *StopwatchTmplInfo { - ctx := context.GetWebContext(goCtx) +func getActiveStopwatch(ctx *context.Context) *StopwatchTmplInfo { if ctx.Doer == nil { return nil } @@ -48,8 +48,7 @@ func getActiveStopwatch(goCtx goctx.Context) *StopwatchTmplInfo { } } -func notificationUnreadCount(goCtx goctx.Context) int64 { - ctx := context.GetWebContext(goCtx) +func notificationUnreadCount(ctx *context.Context) int64 { if ctx.Doer == nil { return 0 } @@ -66,10 +65,19 @@ func notificationUnreadCount(goCtx goctx.Context) int64 { return count } -func PageTmplFunctions(ctx *context.Context) { - if ctx.IsSigned { - // defer the function call to the last moment when the tmpl renders - ctx.Data["NotificationUnreadCount"] = notificationUnreadCount - ctx.Data["GetActiveStopwatch"] = getActiveStopwatch - } +type pageGlobalDataType struct { + IsSigned bool + IsSiteAdmin bool + + GetNotificationUnreadCount func() int64 + GetActiveStopwatch func() *StopwatchTmplInfo +} + +func PageGlobalData(ctx *context.Context) { + var data pageGlobalDataType + data.IsSigned = ctx.Doer != nil + data.IsSiteAdmin = ctx.Doer != nil && ctx.Doer.IsAdmin + data.GetNotificationUnreadCount = sync.OnceValue(func() int64 { return notificationUnreadCount(ctx) }) + data.GetActiveStopwatch = sync.OnceValue(func() *StopwatchTmplInfo { return getActiveStopwatch(ctx) }) + ctx.Data["PageGlobalData"] = data } diff --git a/routers/web/web.go b/routers/web/web.go index 1039f9e739..66c3a2da09 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -281,7 +281,7 @@ func Routes() *web.Router { } mid = append(mid, goGet) - mid = append(mid, common.PageTmplFunctions) + mid = append(mid, common.PageGlobalData) webRoutes := web.NewRouter() webRoutes.Use(mid...) diff --git a/templates/base/head_navbar.tmpl b/templates/base/head_navbar.tmpl index dacc518873..b721779c95 100644 --- a/templates/base/head_navbar.tmpl +++ b/templates/base/head_navbar.tmpl @@ -1,11 +1,3 @@ -{{$notificationUnreadCount := 0}} -{{if and .IsSigned .NotificationUnreadCount}} - {{$notificationUnreadCount = call .NotificationUnreadCount ctx}} -{{end}} -{{$activeStopwatch := NIL}} -{{if and .IsSigned EnableTimetracking .GetActiveStopwatch}} - {{$activeStopwatch = call .GetActiveStopwatch ctx}} -{{end}}
{{else if .IsSigned}} - {{if $activeStopwatch}} - -
- {{svg "octicon-stopwatch"}} - -
-
- {{end}} - - -
- {{svg "octicon-bell"}} - {{$notificationUnreadCount}} -
-
- + {{template "base/head_navbar_icons" dict "ItemExtraClass" "not-mobile" "PageGlobalData" .PageGlobalData}}