From 8164ef9762b41eee52917147408a118926f285cd Mon Sep 17 00:00:00 2001
From: Gergely Nagy <forgejo@gergo.csillger.hu>
Date: Wed, 1 May 2024 12:36:09 +0200
Subject: [PATCH] markup: Allow cross references to contain URL query
 parameters too

Adjust the `anyHashPattern` to match URL query parameters too, and
adjust `fullHashPatternProcessor` accordingly.

Includes a test case, and an update to an existing one to account for
the new capture group.

Fixes #3548.

Signed-off-by: Gergely Nagy <forgejo@gergo.csillger.hu>
---
 modules/markup/html.go               |  8 ++++----
 modules/markup/html_internal_test.go | 11 +++++++++++
 modules/markup/html_test.go          |  6 ++++++
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/modules/markup/html.go b/modules/markup/html.go
index 56f63b8a76..f73221a37f 100644
--- a/modules/markup/html.go
+++ b/modules/markup/html.go
@@ -54,7 +54,7 @@ var (
 	shortLinkPattern = regexp.MustCompile(`\[\[(.*?)\]\](\w*)`)
 
 	// anySHA1Pattern splits url containing SHA into parts
-	anyHashPattern = regexp.MustCompile(`https?://(?:\S+/){4,5}([0-9a-f]{40,64})(/[-+~_%.a-zA-Z0-9/]+)?(#[-+~_%.a-zA-Z0-9]+)?`)
+	anyHashPattern = regexp.MustCompile(`https?://(?:\S+/){4,5}([0-9a-f]{40,64})(/[-+~_%.a-zA-Z0-9/]+)?(\?[-+~_%\.a-zA-Z0-9=&]+)?(#[-+~_%.a-zA-Z0-9]+)?`)
 
 	// comparePattern matches "http://domain/org/repo/compare/COMMIT1...COMMIT2#hash"
 	comparePattern = regexp.MustCompile(`https?://(?:\S+/){4,5}([0-9a-f]{7,64})(\.\.\.?)([0-9a-f]{7,64})?(#[-+~_%.a-zA-Z0-9]+)?`)
@@ -969,10 +969,10 @@ func fullHashPatternProcessor(ctx *RenderContext, node *html.Node) {
 			subpath = node.Data[m[4]:m[5]]
 		}
 
-		// 4th capture group matches a optional url hash
+		// 5th capture group matches a optional url hash
 		hash := ""
-		if m[7] > 0 {
-			hash = node.Data[m[6]:m[7]][1:]
+		if m[9] > 0 {
+			hash = node.Data[m[8]:m[9]][1:]
 		}
 
 		start := m[0]
diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go
index e313be7040..917f280c73 100644
--- a/modules/markup/html_internal_test.go
+++ b/modules/markup/html_internal_test.go
@@ -403,28 +403,39 @@ func TestRegExp_anySHA1Pattern(t *testing.T) {
 		"https://github.com/jquery/jquery/blob/a644101ed04d0beacea864ce805e0c4f86ba1cd1/test/unit/event.js#L2703": {
 			"a644101ed04d0beacea864ce805e0c4f86ba1cd1",
 			"/test/unit/event.js",
+			"",
 			"#L2703",
 		},
 		"https://github.com/jquery/jquery/blob/a644101ed04d0beacea864ce805e0c4f86ba1cd1/test/unit/event.js": {
 			"a644101ed04d0beacea864ce805e0c4f86ba1cd1",
 			"/test/unit/event.js",
 			"",
+			"",
 		},
 		"https://github.com/jquery/jquery/commit/0705be475092aede1eddae01319ec931fb9c65fc": {
 			"0705be475092aede1eddae01319ec931fb9c65fc",
 			"",
 			"",
+			"",
 		},
 		"https://github.com/jquery/jquery/tree/0705be475092aede1eddae01319ec931fb9c65fc/src": {
 			"0705be475092aede1eddae01319ec931fb9c65fc",
 			"/src",
 			"",
+			"",
 		},
 		"https://try.gogs.io/gogs/gogs/commit/d8a994ef243349f321568f9e36d5c3f444b99cae#diff-2": {
 			"d8a994ef243349f321568f9e36d5c3f444b99cae",
 			"",
+			"",
 			"#diff-2",
 		},
+		"https://codeberg.org/forgejo/forgejo/src/commit/949ab9a5c4cac742f84ae5a9fa186f8d6eb2cdc0/RELEASE-NOTES.md?display=source&w=1#L7-L9": {
+			"949ab9a5c4cac742f84ae5a9fa186f8d6eb2cdc0",
+			"/RELEASE-NOTES.md",
+			"?display=source&w=1",
+			"#L7-L9",
+		},
 	}
 
 	for k, v := range testCases {
diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go
index a1a99c1a7f..cfd1a66a18 100644
--- a/modules/markup/html_test.go
+++ b/modules/markup/html_test.go
@@ -127,6 +127,12 @@ func TestRender_CrossReferences(t *testing.T) {
 	test(
 		util.URLJoin(markup.TestAppURL, "gogitea", "some-repo-name", "issues", "12345"),
 		`<p><a href="`+util.URLJoin(markup.TestAppURL, "gogitea", "some-repo-name", "issues", "12345")+`" class="ref-issue" rel="nofollow">gogitea/some-repo-name#12345</a></p>`)
+
+	sha := "65f1bf27bc3bf70f64657658635e66094edbcb4d"
+	urlWithQuery := util.URLJoin(markup.TestAppURL, "forgejo", "some-repo-name", "commit", sha, "README.md") + "?display=source#L1-L5"
+	test(
+		urlWithQuery,
+		`<p><a href="`+urlWithQuery+`" rel="nofollow"><code>`+sha[:10]+`/README.md (L1-L5)</code></a></p>`)
 }
 
 func TestMisc_IsSameDomain(t *testing.T) {