Fix various bugs (#36446)

* Fix #36409
* Fix #36322
* Fix #30101
* Fix #36317

---------

Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>

modules/markup/html.go

@@ -62,7 +62,7 @@ var globalVars = sync.OnceValue(func() *globalVarsType {
 	v.shortLinkPattern = regexp.MustCompile(`\[\[(.*?)\]\](\w*)`)
 
 	// anyHashPattern splits url containing SHA into parts
-	v.anyHashPattern = regexp.MustCompile(`https?://(?:\S+/){4,5}([0-9a-f]{40,64})(/[-+~%./\w]+)?(\?[-+~%.\w&=]+)?(#[-+~%.\w]+)?`)
+	v.anyHashPattern = regexp.MustCompile(`https?://(?:\S+/){4,5}([0-9a-f]{40,64})((\.\w+)*)(/[-+~%./\w]+)?(\?[-+~%.\w&=]+)?(#[-+~%.\w]+)?`)
 
 	// comparePattern matches "http://domain/org/repo/compare/COMMIT1...COMMIT2#hash"
 	v.comparePattern = regexp.MustCompile(`https?://(?:\S+/){4,5}([0-9a-f]{7,64})(\.\.\.?)([0-9a-f]{7,64})?(#[-+~_%.a-zA-Z0-9]+)?`)

modules/markup/html_commit.go

@@ -16,12 +16,14 @@ import (
 )
 
 type anyHashPatternResult struct {
-	PosStart  int
-	PosEnd    int
-	FullURL   string
-	CommitID  string
-	SubPath   string
-	QueryHash string
+	PosStart    int
+	PosEnd      int
+	FullURL     string
+	CommitID    string
+	CommitExt   string
+	SubPath     string
+	QueryParams string
+	QueryHash   string
 }
 
 func createCodeLink(href, content, class string) *html.Node {
@@ -56,7 +58,11 @@ func anyHashPatternExtract(s string) (ret anyHashPatternResult, ok bool) {
 		return ret, false
 	}
 
-	ret.PosStart, ret.PosEnd = m[0], m[1]
+	pos := 0
+
+	ret.PosStart, ret.PosEnd = m[pos], m[pos+1]
+	pos += 2
+
 	ret.FullURL = s[ret.PosStart:ret.PosEnd]
 	if strings.HasSuffix(ret.FullURL, ".") {
 		// if url ends in '.', it's very likely that it is not part of the actual url but used to finish a sentence.
@@ -67,14 +73,24 @@ func anyHashPatternExtract(s string) (ret anyHashPatternResult, ok bool) {
 		}
 	}
 
-	ret.CommitID = s[m[2]:m[3]]
-	if m[5] > 0 {
-		ret.SubPath = s[m[4]:m[5]]
-	}
+	ret.CommitID = s[m[pos]:m[pos+1]]
+	pos += 2
 
-	lastStart, lastEnd := m[len(m)-2], m[len(m)-1]
-	if lastEnd > 0 {
-		ret.QueryHash = s[lastStart:lastEnd][1:]
+	ret.CommitExt = s[m[pos]:m[pos+1]]
+	pos += 4
+
+	if m[pos] > 0 {
+		ret.SubPath = s[m[pos]:m[pos+1]]
+	}
+	pos += 2
+
+	if m[pos] > 0 {
+		ret.QueryParams = s[m[pos]:m[pos+1]]
+	}
+	pos += 2
+
+	if m[pos] > 0 {
+		ret.QueryHash = s[m[pos]:m[pos+1]][1:]
 	}
 	return ret, true
 }
@@ -96,6 +112,9 @@ func fullHashPatternProcessor(ctx *RenderContext, node *html.Node) {
 			continue
 		}
 		text := base.ShortSha(ret.CommitID)
+		if ret.CommitExt != "" {
+			text += ret.CommitExt
+		}
 		if ret.SubPath != "" {
 			text += ret.SubPath
 		}

modules/markup/html_test.go

@@ -102,6 +102,16 @@ func TestRender_CrossReferences(t *testing.T) {
 	test(
 		inputURL,
 		`<p><a href="`+inputURL+`" rel="nofollow"><code>0123456789/foo.txt (L2-L3)</code></a></p>`)
+
+	inputURL = "https://example.com/repo/owner/archive/0123456789012345678901234567890123456789.tar.gz"
+	test(
+		inputURL,
+		`<p><a href="`+inputURL+`" rel="nofollow"><code>0123456789.tar.gz</code></a></p>`)
+
+	inputURL = "https://example.com/owner/repo/commit/0123456789012345678901234567890123456789.patch?key=val"
+	test(
+		inputURL,
+		`<p><a href="`+inputURL+`" rel="nofollow"><code>0123456789.patch</code></a></p>`)
 }
 
 func TestRender_links(t *testing.T) {

modules/templates/helper_test.go

@@ -168,3 +168,10 @@ func TestQueryBuild(t *testing.T) {
 		assert.Equal(t, "&a=b&c=d&e=f", string(QueryBuild("&a=b&c=d&e=f", "k", "")))
 	})
 }
+
+func TestQueryEscape(t *testing.T) {
+	// this test is a reference for "urlQueryEscape" in JS
+	in := "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~" // all non-letter & non-number chars
+	expected := "%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F%3A%3B%3C%3D%3E%3F%40%5B%5C%5D%5E_%60%7B%7C%7D~"
+	assert.Equal(t, expected, string(queryEscape(in)))
+}

options/locale/locale_en-US.json

@@ -1490,6 +1490,7 @@
   "repo.issues.filter_sort.feweststars": "Fewest stars",
   "repo.issues.filter_sort.mostforks": "Most forks",
   "repo.issues.filter_sort.fewestforks": "Fewest forks",
+  "repo.issues.quick_goto": "Go to issue",
   "repo.issues.action_open": "Open",
   "repo.issues.action_close": "Close",
   "repo.issues.action_label": "Label",

templates/repo/issue/search.tmpl

@@ -12,7 +12,7 @@
 		{{end}}
 		{{template "shared/search/input" dict "Value" .Keyword}}
 		{{if .PageIsIssueList}}
-			<button id="issue-list-quick-goto" class="ui small icon button tw-hidden" data-tooltip-content="{{ctx.Locale.Tr "explore.go_to"}}" data-repo-link="{{.RepoLink}}">{{svg "octicon-hash"}}</button>
+			<button id="issue-list-quick-goto" type="button" class="ui small icon button tw-hidden tw-mr-[-1px]" data-repo-link="{{.RepoLink}}">{{svg "octicon-hash" 12}} {{ctx.Locale.Tr "repo.issues.quick_goto"}}</button>
 		{{end}}
 		{{template "shared/search/button"}}
 	</div>

web_src/css/base.css

@@ -103,13 +103,13 @@ samp,
   font-size: 0.95em; /* compensate for monospace fonts being usually slightly larger */
 }
 
-/* there are many <code> blocks in non-markup(.markup code) / non-code-diff(code.code-inner) containers (for example: translation strings, etc),
-so we need to make <code> have default global styles, ".markup code" has its own styles and doesn't conflict, but `.code-inner` is special.
-TODO: in the future, we should use `div` instead of `code` for `.code-inner` because it is a container for highlighted code line */
-code:not(.code-inner) {
+/* there are many <code> blocks in non-markup(.markup code) / non-code-diff(code.code-inner) containers, for example: translation strings, etc,
+so we need to make <code> have default global styles, ".markup code" has its own styles and these styles sometimes conflict.
+TODO: in the future, we should use `div` instead of `code` for `.code-inner` because it is a container for highlighted code line, then drop this ":not" patch */
+code:where(:not(.code-inner)) {
   padding: 1px 4px;
   border-radius: var(--border-radius);
-  background-color: var(--color-label-bg);
+  background-color: var(--color-markup-code-inline);
 }
 
 b,

web_src/js/features/admin/common.ts

@@ -2,6 +2,7 @@ import {checkAppUrl} from '../common-page.ts';
 import {hideElem, queryElems, showElem, toggleElem} from '../../utils/dom.ts';
 import {POST} from '../../modules/fetch.ts';
 import {fomanticQuery} from '../../modules/fomantic/base.ts';
+import {urlQueryEscape} from '../../utils.ts';
 
 const {appSubUrl} = window.config;
 
@@ -230,7 +231,7 @@ function initAdminAuthentication() {
   const elAuthName = document.querySelector<HTMLInputElement>('#auth_name')!;
   const onAuthNameChange = function () {
     // appSubUrl is either empty or is a path that starts with `/` and doesn't have a trailing slash.
-    document.querySelector('#oauth2-callback-url')!.textContent = `${window.location.origin}${appSubUrl}/user/oauth2/${encodeURIComponent(elAuthName.value)}/callback`;
+    document.querySelector('#oauth2-callback-url')!.textContent = `${window.location.origin}${appSubUrl}/user/oauth2/${urlQueryEscape(elAuthName.value)}/callback`;
   };
   elAuthName.addEventListener('input', onAuthNameChange);
   onAuthNameChange();

web_src/js/features/common-issue-list.test.ts

@@ -5,7 +5,7 @@ test('parseIssueListQuickGotoLink', () => {
   expect(parseIssueListQuickGotoLink('/link', 'abc')).toEqual('');
   expect(parseIssueListQuickGotoLink('/link', '123')).toEqual('/link/issues/123');
   expect(parseIssueListQuickGotoLink('/link', '#123')).toEqual('/link/issues/123');
-  expect(parseIssueListQuickGotoLink('/link', 'owner/repo#123')).toEqual('');
+  expect(parseIssueListQuickGotoLink('/link', 'owner/repo#123')).toEqual('/owner/repo/issues/123');
 
   expect(parseIssueListQuickGotoLink('', '')).toEqual('');
   expect(parseIssueListQuickGotoLink('', 'abc')).toEqual('');

web_src/js/features/common-issue-list.ts

@@ -1,4 +1,4 @@
-import {isElemVisible, onInputDebounce, submitEventSubmitter, toggleElem} from '../utils/dom.ts';
+import {onInputDebounce, toggleElem} from '../utils/dom.ts';
 import {GET} from '../modules/fetch.ts';
 
 const {appSubUrl} = window.config;
@@ -17,37 +17,25 @@ export function parseIssueListQuickGotoLink(repoLink: string, searchText: string
     } else if (reIssueSharpIndex.test(searchText)) {
       targetUrl = `${repoLink}/issues/${searchText.substring(1)}`;
     }
-  } else {
-    // try to parse it for a global search (eg: "owner/repo#123")
-    const [_, owner, repo, index] = reIssueOwnerRepoIndex.exec(searchText) || [];
-    if (owner) {
-      targetUrl = `${appSubUrl}/${owner}/${repo}/issues/${index}`;
-    }
+  }
+  // try to parse it for a global search (eg: "owner/repo#123")
+  const [_, owner, repo, index] = reIssueOwnerRepoIndex.exec(searchText) || [];
+  if (owner) {
+    targetUrl = `${appSubUrl}/${owner}/${repo}/issues/${index}`;
   }
   return targetUrl;
 }
 
 export function initCommonIssueListQuickGoto() {
-  const goto = document.querySelector<HTMLElement>('#issue-list-quick-goto');
-  if (!goto) return;
+  const elGotoButton = document.querySelector<HTMLElement>('#issue-list-quick-goto');
+  if (!elGotoButton) return;
 
-  const form = goto.closest('form')!;
+  const form = elGotoButton.closest('form')!;
   const input = form.querySelector<HTMLInputElement>('input[name=q]')!;
-  const repoLink = goto.getAttribute('data-repo-link')!;
+  const repoLink = elGotoButton.getAttribute('data-repo-link') || '';
 
-  form.addEventListener('submit', (e) => {
-    // if there is no goto button, or the form is submitted by non-quick-goto elements, submit the form directly
-    let doQuickGoto = isElemVisible(goto);
-    const submitter = submitEventSubmitter(e);
-    if (submitter !== form && submitter !== input && submitter !== goto) doQuickGoto = false;
-    if (!doQuickGoto) return;
-
-    // if there is a goto button, use its link
-    e.preventDefault();
-    const link = goto.getAttribute('data-issue-goto-link');
-    if (link) {
-      window.location.href = link;
-    }
+  elGotoButton.addEventListener('click', () => {
+    window.location.href = elGotoButton.getAttribute('data-issue-goto-link')!;
   });
 
   const onInput = async () => {
@@ -61,8 +49,8 @@ export function initCommonIssueListQuickGoto() {
     // if the input value has changed, then ignore the result
     if (input.value !== searchText) return;
 
-    toggleElem(goto, Boolean(targetUrl));
-    goto.setAttribute('data-issue-goto-link', targetUrl);
+    toggleElem(elGotoButton, Boolean(targetUrl));
+    elGotoButton.setAttribute('data-issue-goto-link', targetUrl);
   };
 
   input.addEventListener('input', onInputDebounce(onInput));

web_src/js/utils.test.ts

@@ -2,6 +2,7 @@ import {
   dirname, basename, extname, isObject, stripTags, parseIssueHref,
   parseUrl, translateMonth, translateDay, blobToDataURI,
   toAbsoluteUrl, encodeURLEncodedBase64, decodeURLEncodedBase64, isImageFile, isVideoFile, parseRepoOwnerPathInfo,
+  urlQueryEscape,
 } from './utils.ts';
 
 test('dirname', () => {
@@ -33,6 +34,12 @@ test('stripTags', () => {
   expect(stripTags('<a>test</a>')).toEqual('test');
 });
 
+test('urlQueryEscape', () => {
+  const input = "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~";
+  const expected = '%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F%3A%3B%3C%3D%3E%3F%40%5B%5C%5D%5E_%60%7B%7C%7D~';
+  expect(urlQueryEscape(input)).toEqual(expected);
+});
+
 test('parseIssueHref', () => {
   expect(parseIssueHref('/owner/repo/issues/1')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'});
   expect(parseIssueHref('/owner/repo/pulls/1?query')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'pulls', indexString: '1'});

web_src/js/utils.ts

@@ -43,6 +43,15 @@ export function stripTags(text: string): string {
   return text;
 }
 
+export function urlQueryEscape(s: string) {
+  // See "TestQueryEscape" in backend
+  // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent#encoding_for_rfc3986
+  return encodeURIComponent(s).replace(
+    /[!'()*]/g,
+    (c) => `%${c.charCodeAt(0).toString(16).toUpperCase()}`,
+  );
+}
+
 export function parseIssueHref(href: string): IssuePathInfo {
   // FIXME: it should use pathname and trim the appSubUrl ahead
   const path = (href || '').replace(/[#?].*$/, '');