From 742e26f5a5dab8768558231cb88801911b9abaa7 Mon Sep 17 00:00:00 2001
From: zeripath <art27@cantab.net>
Date: Mon, 11 May 2020 00:14:49 +0100
Subject: [PATCH] Prevent 500 with badly formed task list (#11328)

Fix #11317

Signed-off-by: Andrew Thornton <art27@cantab.net>
---
 modules/markup/markdown/goldmark.go      | 38 ++++++++++++++----------
 modules/markup/markdown/markdown_test.go |  8 ++---
 modules/markup/sanitizer.go              |  2 +-
 web_src/less/_markdown.less              |  4 +--
 4 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/modules/markup/markdown/goldmark.go b/modules/markup/markdown/goldmark.go
index 6a40a8683..bf02a22d5 100644
--- a/modules/markup/markdown/goldmark.go
+++ b/modules/markup/markdown/goldmark.go
@@ -125,24 +125,30 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa
 			}
 			v.Destination = link
 		case *ast.List:
-			if v.HasChildren() && v.FirstChild().HasChildren() && v.FirstChild().FirstChild().HasChildren() {
-				if _, ok := v.FirstChild().FirstChild().FirstChild().(*east.TaskCheckBox); ok {
-					v.SetAttributeString("class", []byte("task-list"))
-					children := make([]ast.Node, 0, v.ChildCount())
-					child := v.FirstChild()
-					for child != nil {
-						children = append(children, child)
-						child = child.NextSibling()
+			if v.HasChildren() {
+				children := make([]ast.Node, 0, v.ChildCount())
+				child := v.FirstChild()
+				for child != nil {
+					children = append(children, child)
+					child = child.NextSibling()
+				}
+				v.RemoveChildren(v)
+
+				for _, child := range children {
+					listItem := child.(*ast.ListItem)
+					if !child.HasChildren() || !child.FirstChild().HasChildren() {
+						v.AppendChild(v, child)
+						continue
 					}
-					v.RemoveChildren(v)
-
-					for _, child := range children {
-						listItem := child.(*ast.ListItem)
-						newChild := NewTaskCheckBoxListItem(listItem)
-						taskCheckBox := child.FirstChild().FirstChild().(*east.TaskCheckBox)
-						newChild.IsChecked = taskCheckBox.IsChecked
-						v.AppendChild(v, newChild)
+					taskCheckBox, ok := child.FirstChild().FirstChild().(*east.TaskCheckBox)
+					if !ok {
+						v.AppendChild(v, child)
+						continue
 					}
+					newChild := NewTaskCheckBoxListItem(listItem)
+					newChild.IsChecked = taskCheckBox.IsChecked
+					newChild.SetAttributeString("class", []byte("task-list-item"))
+					v.AppendChild(v, newChild)
 				}
 			}
 		}
diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go
index b9946d7d2..7f27a43a7 100644
--- a/modules/markup/markdown/markdown_test.go
+++ b/modules/markup/markdown/markdown_test.go
@@ -141,10 +141,10 @@ func testAnswers(baseURLContent, baseURLImages string) []string {
 <h2 id="user-content-custom-id">More tests</h2>
 <p>(from <a href="https://www.markdownguide.org/extended-syntax/" rel="nofollow">https://www.markdownguide.org/extended-syntax/</a>)</p>
 <h3 id="user-content-checkboxes">Checkboxes</h3>
-<ul class="task-list">
-<li><span class="ui checkbox"><input type="checkbox" readonly="readonly"/><label>unchecked</label></span></li>
-<li><span class="ui checked checkbox"><input type="checkbox" checked="" readonly="readonly"/><label>checked</label></span></li>
-<li><span class="ui checkbox"><input type="checkbox" readonly="readonly"/><label>still unchecked</label></span></li>
+<ul>
+<li class="task-list-item"><span class="ui checkbox"><input type="checkbox" readonly="readonly"/><label>unchecked</label></span></li>
+<li class="task-list-item"><span class="ui checked checkbox"><input type="checkbox" checked="" readonly="readonly"/><label>checked</label></span></li>
+<li class="task-list-item"><span class="ui checkbox"><input type="checkbox" readonly="readonly"/><label>still unchecked</label></span></li>
 </ul>
 <h3 id="user-content-definition-list">Definition list</h3>
 <dl>
diff --git a/modules/markup/sanitizer.go b/modules/markup/sanitizer.go
index 39e4a93dd..1041d56a3 100644
--- a/modules/markup/sanitizer.go
+++ b/modules/markup/sanitizer.go
@@ -54,7 +54,7 @@ func ReplaceSanitizer() {
 	sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`ref-issue`)).OnElements("a")
 
 	// Allow classes for task lists
-	sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`task-list`)).OnElements("ul")
+	sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`task-list-item`)).OnElements("li")
 
 	// Allow icons
 	sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`^icon(\s+[\p{L}\p{N}_-]+)+$`)).OnElements("i")
diff --git a/web_src/less/_markdown.less b/web_src/less/_markdown.less
index da542432d..651edb572 100644
--- a/web_src/less/_markdown.less
+++ b/web_src/less/_markdown.less
@@ -192,9 +192,9 @@
         list-style-type: none;
     }
 
-    ul.task-list,
-    ol.task-list {
+    li.task-list-item {
         list-style-type: none;
+        margin-left: calc(-2em + 2px);
     }
 
     ul ul,