tpl/tplimpl: Escape Markdown attributes in render hooks and shortcodes

jmooring · bep · commit 54398f8d572c · 2024-12-09T17:58:57.000+01:00
diff --git a/hugolib/content_render_hooks_test.go b/hugolib/content_render_hooks_test.go
@@ -90,7 +90,7 @@ baseURL="https://example.org"
   [markup.goldmark]
     [markup.goldmark.renderer]
       unsafe = true
-    
+
 `)
 
 	b.WithTemplates("index.html", `
@@ -223,16 +223,16 @@ iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAA
 iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==
 -- layouts/_default/single.html --
 {{ .Title }}|{{ .Content }}|$
-	
+
 `
 
 	t.Run("Default multilingual", func(t *testing.T) {
 		b := Test(t, files)
 
 		b.AssertFileContent("public/nn/p1/index.html",
-			"p1|<p><a href=\"/nn/p2/\">P2</a\n></p>", "<img alt=\"Pixel\" src=\"/nn/p1/pixel.nn.png\">")
+			"p1|<p><a href=\"/nn/p2/\">P2</a\n></p>", "<img src=\"/nn/p1/pixel.nn.png\" alt=\"Pixel\">")
 		b.AssertFileContent("public/en/p1/index.html",
-			"p1 en|<p><a href=\"/en/p2/\">P2</a\n></p>", "<img alt=\"Pixel\" src=\"/nn/p1/pixel.nn.png\">")
+			"p1 en|<p><a href=\"/en/p2/\">P2</a\n></p>", "<img src=\"/nn/p1/pixel.nn.png\" alt=\"Pixel\">")
 	})
 
 	t.Run("Disabled", func(t *testing.T) {
@@ -279,7 +279,7 @@ Image: ![alt-"<>&](/destination-"<> 'title-"<>&')
 			if enabled {
 				b.AssertFileContent("public/index.html",
 					"Link: <a href=\"/destination-%22%3C%3E\" title=\"title-&#34;&lt;&gt;&amp;\">text-&quot;&lt;&gt;&amp;</a>",
-					"img alt=\"alt-&quot;&lt;&gt;&amp;\" src=\"/destination-%22%3C%3E\" title=\"title-&#34;&lt;&gt;&amp;\">",
+					"img src=\"/destination-%22%3C%3E\" alt=\"alt-&quot;&lt;&gt;&amp;\" title=\"title-&#34;&lt;&gt;&amp;\">",
 					"&gt;&lt;script&gt;",
 				)
 			} else {
diff --git a/markup/goldmark/tables/tables_integration_test.go b/markup/goldmark/tables/tables_integration_test.go
@@ -89,6 +89,12 @@ title = true
 | Codecademy Hoodie |  False   | 42.99 |
 {.foo}
 
+## Table 2
+
+a|b
+---|---
+1|2
+{id="\"><script>alert()</script>"}
 
 -- layouts/_default/single.html --
 Summary: {{ .Summary }}
@@ -97,7 +103,8 @@ Content: {{ .Content }}
 `
 	b := hugolib.Test(t, files)
 
-	b.AssertFileContent("public/p1/index.html", "<table class=\"foo\">")
+	b.AssertFileContent("public/p1/index.html", `<table class="foo">`)
+	b.AssertFileContent("public/p1/index.html", `<table id="&#34;&gt;&lt;script&gt;alert()&lt;/script&gt;">`)
 }
 
 // Issue 12811.
@@ -166,14 +173,8 @@ title: "Home"
 | Codecademy Tee    |  False   | 19.99 |
 | Codecademy Hoodie |  False   | 42.99 |
 
-
-
-
-
 -- layouts/index.xml --
 Content: {{ .Content }}
-
-
 `
 	b := hugolib.Test(t, files)
 
diff --git a/tpl/tplimpl/embedded/templates/_default/_markup/render-image.html b/tpl/tplimpl/embedded/templates/_default/_markup/render-image.html
@@ -1,7 +1,7 @@
 {{- $u := urls.Parse .Destination -}}
 {{- $src := $u.String -}}
 {{- if not $u.IsAbs -}}
-  {{- $path := strings.TrimPrefix "./" $u.Path }}
+  {{- $path := strings.TrimPrefix "./" $u.Path -}}
   {{- with or (.PageInner.Resources.Get $path) (resources.Get $path) -}}
     {{- $src = .RelPermalink -}}
     {{- with $u.RawQuery -}}
@@ -12,11 +12,12 @@
     {{- end -}}
   {{- end -}}
 {{- end -}}
-{{- $attributes := merge .Attributes (dict "alt" .Text "src" $src "title" (.Title | transform.HTMLEscape)) -}}
-<img
-  {{- range $k, $v := $attributes -}}
+<img src="{{ $src }}" alt="{{ .Text }}"
+  {{- with .Title }} title="{{ . }}" {{- end -}}
+  {{- range $k, $v := .Attributes -}}
     {{- if $v -}}
-      {{- printf " %s=%q" $k $v | safeHTMLAttr -}}
+      {{- printf " %s=%q" $k ($v | transform.HTMLEscape) | safeHTMLAttr -}}
     {{- end -}}
-  {{- end -}}>
+  {{- end -}}
+>
 {{- /**/ -}}
diff --git a/tpl/tplimpl/embedded/templates/_default/_markup/render-link.html b/tpl/tplimpl/embedded/templates/_default/_markup/render-link.html
@@ -1,9 +1,9 @@
 {{- $u := urls.Parse .Destination -}}
 {{- $href := $u.String -}}
-{{- if strings.HasPrefix $u.String "#" }}
-  {{- $href = printf "%s#%s" .PageInner.RelPermalink $u.Fragment }}
-{{- else if not $u.IsAbs -}}
-  {{- $path := strings.TrimPrefix "./" $u.Path }}
+{{- if strings.HasPrefix $u.String "#" -}}
+  {{- $href = printf "%s#%s" .PageInner.RelPermalink $u.Fragment -}}
+{{- else if and $href (not $u.IsAbs) -}}
+  {{- $path := strings.TrimPrefix "./" $u.Path -}}
   {{- with or
     ($.PageInner.GetPage $path)
     ($.PageInner.Resources.Get $path)
@@ -18,12 +18,5 @@
     {{- end -}}
   {{- end -}}
 {{- end -}}
-{{- $attributes := dict "href" $href "title" (.Title | transform.HTMLEscape) -}}
-<a
-  {{- range $k, $v := $attributes -}}
-    {{- if $v -}}
-      {{- printf " %s=%q" $k $v | safeHTMLAttr -}}
-    {{- end -}}
-  {{- end -}}
-  >{{ .Text }}</a>
+<a href="{{ $href }}" {{- with .Title }} title="{{ . }}" {{- end }}>{{ .Text }}</a>
 {{- /**/ -}}
diff --git a/tpl/tplimpl/embedded/templates/_default/_markup/render-table.html b/tpl/tplimpl/embedded/templates/_default/_markup/render-table.html
@@ -1,7 +1,7 @@
 <table
   {{- range $k, $v := .Attributes }}
     {{- if $v }}
-      {{- printf " %s=%q" $k $v | safeHTMLAttr }}
+      {{- printf " %s=%q" $k ($v | transform.HTMLEscape) | safeHTMLAttr }}
     {{- end }}
   {{- end }}>
   <thead>
diff --git a/tpl/tplimpl/embedded/templates/shortcodes/youtube.html b/tpl/tplimpl/embedded/templates/shortcodes/youtube.html
@@ -26,7 +26,7 @@
 {{- if not $pc.Disable }}
   {{- with $id := or (.Get "id") (.Get 0) }}
 
-    {{/* Set defaults. */}}
+    {{- /* Set defaults. */}}
     {{- $allowFullScreen := "allowfullscreen" }}
     {{- $autoplay := 0 }}
     {{- $class := "" }}
@@ -70,23 +70,8 @@
     {{- $start := or ($.Get "start") $start }}
     {{- $title := or ($.Get "title") $title }}
 
-    {{- /* Determine host. */}}
-    {{- $host := cond $pc.PrivacyEnhanced "www.youtube-nocookie.com" "www.youtube.com" }}
-
-    {{- /* Set styles. */}}
-    {{- $divStyle := "position: relative; padding-bottom: 56.25%; height: 0; overflow: hidden;" }}
-    {{- $iframeStyle := "position: absolute; top: 0; left: 0; width: 100%; height: 100%; border:0;" }}
-    {{- if $class }}
-      {{- $iframeStyle = "" }}
-    {{- end }}
-
-    {{- /* Set class or style of wrapping div element. */}}
-    {{- $divClassOrStyle := printf "style=%q" $divStyle }}
-    {{- with $class }}
-      {{- $divClassOrStyle = printf "class=%q" $class }}
-    {{- end }}
-
     {{- /* Define src attribute. */}}
+    {{- $host := cond $pc.PrivacyEnhanced "www.youtube-nocookie.com" "www.youtube.com" }}
     {{- $src := printf "https://%s/embed/%s" $host $id }}
     {{- $params := dict
       "autoplay" $autoplay
@@ -108,25 +93,33 @@
       {{- $src = printf "%s?%s" $src . }}
     {{- end }}
 
+    {{- /* Set div attributes. */}}
+    {{- $divStyle := "position: relative; padding-bottom: 56.25%; height: 0; overflow: hidden;" }}
+    {{- if $class }}
+      {{- $divStyle = "" }}
+    {{- end }}
+
     {{- /* Set iframe attributes. */}}
-    {{- $iframeAttributes := dict
-      "allow" "accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share"
-      "allowfullscreen" $allowFullScreen
-      "loading" $loading
-      "referrerpolicy" "strict-origin-when-cross-origin"
-      "src" $src
-      "style" $iframeStyle
-      "title" $title
-    }}
+    {{- $iframeStyle := "position: absolute; top: 0; left: 0; width: 100%; height: 100%; border:0;" }}
+    {{- if $class }}
+      {{- $iframeStyle = "" }}
+    {{- end }}
+    {{- $allow := "accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share" }}
+    {{- $referrerpolicy := "strict-origin-when-cross-origin" }}
 
     {{- /* Render. */}}
-    <div {{ $divClassOrStyle | safeHTMLAttr }}>
+    <div
+      {{- with $class }} class="{{ . }}" {{- end }}
+      {{- with $divStyle }} style="{{ . | safeCSS }}" {{- end -}}
+    >
       <iframe
-        {{- range $k, $v := $iframeAttributes }}
-          {{- if $v }}
-            {{- printf " %s=%q" $k $v | safeHTMLAttr }}
-          {{- end }}
-        {{- end }}
+        {{- with $allow }} allow="{{ . }}" {{- end }}
+        {{- with $allowFullScreen }} allowfullscreen="{{ . }}" {{- end }}
+        {{- with $loading }} loading="{{ . }}" {{- end }}
+        {{- with $referrerpolicy }} referrerpolicy="{{ . }}" {{- end }}
+        {{- with $src }} src="{{ . }}" {{- end }}
+        {{- with $iframeStyle}} style="{{ . | safeCSS }}" {{- end }}
+        {{- with $title }} title="{{ . }}" {{- end -}}
       ></iframe>
     </div>
   {{- else }}
diff --git a/tpl/tplimpl/render_hook_integration_test.go b/tpl/tplimpl/render_hook_integration_test.go
@@ -91,6 +91,9 @@ title: s1/p3
 [430](p2/)
 [440](/s1/p2/)
 [450](../s1/p2/)
+
+// empty
+[]()
 `
 
 	b := hugolib.Test(t, files)
@@ -122,6 +125,8 @@ title: s1/p3
 		`<a href="/s1/p2/">430</a>`,
 		`<a href="/s1/p2/">440</a>`,
 		`<a href="/s1/p2/">450</a>`,
+
+		`<a href=""></a>`,
 	)
 
 	b.AssertFileContent("public/s1/p2/index.html",
@@ -148,10 +153,17 @@ block = false
 [markup.goldmark.renderHooks.image]
 enableDefault = true
 -- content/p1/index.md --
+![]()
+
 ![alt1](./pixel.png)
 
-![alt2](pixel.png?a=b&c=d#fragment)
+![alt2-&<>'](pixel.png "&<>'")
+
+![alt3](pixel.png?a=b&c=d#fragment)
 {.foo #bar}
+
+![alt4](pixel.png)
+{id="\"><script>alert()</script>"}
 -- content/p1/pixel.png --
 iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==
 -- layouts/_default/single.html --
@@ -160,15 +172,21 @@ iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAA
 
 	b := hugolib.Test(t, files)
 	b.AssertFileContent("public/p1/index.html",
-		`<img alt="alt1" src="/dir/p1/pixel.png">`,
-		`<img alt="alt2" src="/dir/p1/pixel.png?a=b&c=d#fragment">`,
+		`<img src="" alt="">`,
+		`<img src="/dir/p1/pixel.png" alt="alt1">`,
+		`<img src="/dir/p1/pixel.png" alt="alt2-&amp;&lt;&gt;&rsquo;" title="&amp;&lt;&gt;&#39;">`,
+		`<img src="/dir/p1/pixel.png?a=b&amp;c=d#fragment" alt="alt3">`,
+		`<img src="/dir/p1/pixel.png" alt="alt4">`,
 	)
 
 	files = strings.Replace(files, "block = false", "block = true", -1)
 
 	b = hugolib.Test(t, files)
 	b.AssertFileContent("public/p1/index.html",
-		`<img alt="alt1" src="/dir/p1/pixel.png">`,
-		`<img alt="alt2" class="foo" id="bar" src="/dir/p1/pixel.png?a=b&c=d#fragment">`,
+		`<img src="" alt="">`,
+		`<img src="/dir/p1/pixel.png" alt="alt1">`,
+		`<img src="/dir/p1/pixel.png" alt="alt2-&amp;&lt;&gt;&rsquo;" title="&amp;&lt;&gt;&#39;">`,
+		`<img src="/dir/p1/pixel.png?a=b&amp;c=d#fragment" alt="alt3" class="foo" id="bar">`,
+		`<img src="/dir/p1/pixel.png" alt="alt4" id="&#34;&gt;&lt;script&gt;alert()&lt;/script&gt;">`,
 	)
 }
