From 164e35ead3c1b9b82d4a23644f6fe96652a747eb Mon Sep 17 00:00:00 2001
From: zeripath <art27@cantab.net>
Date: Sun, 14 Mar 2021 16:36:51 +0000
Subject: [PATCH] Make sure sibling images get a link too (#14979)

* Make sure sibling images get a link too

Due a problem with the ast.Walker in the our transformer in goldmark
an image with a sibling image will not be transformed to gain a parent
link. This PR fixes this.

Fix #12925

Signed-off-by: Andrew Thornton <art27@cantab.net>
---
 modules/markup/markdown/goldmark.go      | 33 +++++++++++++++++++++++-
 modules/markup/markdown/markdown_test.go | 11 ++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/modules/markup/markdown/goldmark.go b/modules/markup/markdown/goldmark.go
index f9fd6eb976..148067f1b0 100644
--- a/modules/markup/markdown/goldmark.go
+++ b/modules/markup/markdown/goldmark.go
@@ -10,6 +10,7 @@ import (
 	"regexp"
 	"strings"
 
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/markup"
 	"code.gitea.io/gitea/modules/markup/common"
 	"code.gitea.io/gitea/modules/setting"
@@ -101,11 +102,41 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa
 			parent := n.Parent()
 			// Create a link around image only if parent is not already a link
 			if _, ok := parent.(*ast.Link); !ok && parent != nil {
+				next := n.NextSibling()
+
+				// Create a link wrapper
 				wrap := ast.NewLink()
 				wrap.Destination = link
 				wrap.Title = v.Title
+
+				// Duplicate the current image node
+				image := ast.NewImage(ast.NewLink())
+				image.Destination = link
+				image.Title = v.Title
+				for _, attr := range v.Attributes() {
+					image.SetAttribute(attr.Name, attr.Value)
+				}
+				for child := v.FirstChild(); child != nil; {
+					next := child.NextSibling()
+					image.AppendChild(image, child)
+					child = next
+				}
+
+				// Append our duplicate image to the wrapper link
+				wrap.AppendChild(wrap, image)
+
+				// Wire in the next sibling
+				wrap.SetNextSibling(next)
+
+				// Replace the current node with the wrapper link
 				parent.ReplaceChild(parent, n, wrap)
-				wrap.AppendChild(wrap, n)
+
+				// But most importantly ensure the next sibling is still on the old image too
+				v.SetNextSibling(next)
+
+			} else {
+				log.Debug("ast.Image: %s has parent: %v", link, parent)
+
 			}
 		case *ast.Link:
 			// Links need their href to munged to be a real value
diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go
index 89005fc25d..3196beea1f 100644
--- a/modules/markup/markdown/markdown_test.go
+++ b/modules/markup/markdown/markdown_test.go
@@ -308,3 +308,14 @@ func TestRender_RenderParagraphs(t *testing.T) {
 	test(t, "A\n\nB\nC\n", 2)
 	test(t, "A\n\n\nB\nC\n", 2)
 }
+
+func TestRenderSiblingImages_Issue12925(t *testing.T) {
+	testcase := `![image1](/image1)
+![image2](/image2)
+`
+	expected := `<p><a href="/image1" rel="nofollow"><img src="/image1" alt="image1"></a><br>
+<a href="/image2" rel="nofollow"><img src="/image2" alt="image2"></a></p>
+`
+	res := string(RenderRaw([]byte(testcase), "", false))
+	assert.Equal(t, expected, res)
+}