refactor: render highlight language (#38793)

Avoid CSS injection

More details are in the comment of CodeBlockAttributes

docs/guidelines-backend.md

@@ -90,7 +90,7 @@ To make template code maintainable:
 
 - Go code should take over complex logic and prepare template data as much as possible, templates only render the data.
 - Prefer struct types provided by Go code instead of map types for template data.
-- Avoid using single world names for non-local variables.
+- Avoid using single word names for non-local variables.
 - Avoid passing `"root" $` or `"." .` to sub-templates, instead pass the specific data needed by the sub-template.
 - Use explicit variable names instead of `.` to access data: ``{{range $item := $.TargetItems}}{{ $item.Name }}{{end}}``
 - Use Go code to implement render helpers if the render logic is too complex.

models/renderhelper/repo_file_test.go

@@ -113,7 +113,7 @@ int a = 1;
 `)
 		assert.NoError(t, err)
 		assert.Equal(t, `<div>
-<pre><code class="chroma language-c"><span class="kt">int</span> <span class="n">a</span> <span class="o">=</span> <span class="mi">1</span><span class="p">;</span></code></pre>
+<pre class="code-block"><code class="chroma language-c"><span class="kt">int</span> <span class="n">a</span> <span class="o">=</span> <span class="mi">1</span><span class="p">;</span></code></pre>
 </div>
 `, rendered)
 	})

modules/highlight/highlight.go

@@ -8,8 +8,10 @@ import (
 	"bytes"
 	gohtml "html"
 	"html/template"
+	"strings"
 	"sync"
 
+	"gitea.dev/modules/htmlutil"
 	"gitea.dev/modules/log"
 	"gitea.dev/modules/setting"
 	"gitea.dev/modules/util"
@@ -161,3 +163,51 @@ func formatLexerName(name string) string {
 	}
 	return util.ToTitleCaseNoLower(name)
 }
+
+func languageForCssAttrName(lang string) (forCSS, forAttr string) {
+	s := strings.ToLower(lang)
+	if s == "" || s == LanguagePlaintext || s == chromaLexerFallback {
+		return "text", "text"
+	}
+	isValid := func(c byte) bool {
+		// although "-" is valid in CSS name, it is used as a field separator, so we don't want to keep it in the name
+		return 'a' <= c && c <= 'z' || '0' <= c && c <= '9' || c == '_'
+	}
+	idx := 0
+	for ; idx < len(s); idx++ {
+		if !isValid(s[idx]) {
+			break
+		}
+	}
+	if idx == len(s) {
+		return s, lang
+	}
+	out := []byte(s)
+	for i := idx; i < len(s); i++ {
+		if !isValid(out[i]) {
+			out[i] = '_'
+		}
+	}
+	return string(out), lang
+}
+
+func CodeBlockAttributes(lang string) (preAttrs, codeAttrs template.HTML) {
+	// Code block's "chroma" class is used to highlight the code.
+	// "language-{LanguageName}" class is used as part of commonmark spec.
+	// It's unclear about how to handle special chars for a language name like "Visual Basic.NET" or "C++" or "F#".
+	// The commonmark spec seems wrong: https://spec.commonmark.org/0.31.2/#info-string, it just outputs invalid CSS class names.
+
+	cssName, attrLang := languageForCssAttrName(lang)
+	renderByFrontend := lang == "mermaid" || lang == "math"
+	preExtraClasses := ""
+	if renderByFrontend {
+		preExtraClasses = " is-loading"
+	}
+
+	// The "math.ts" strictly depends on the structure: <pre class="code-block"><code class="language-math">...</code></pre>
+	// * If "pre" exists, it is rendered as "block", otherwise, it is rendered as "inline"
+	// The "mermaid.ts" also strictly depends on the structure: "pre" must exist because it is always rendered as "block".
+	//
+	// Hint: "data-code-language" is not exposed in some cases due to the Markup sanitizer, the rules can be refactored in the future if the attribute is useful.
+	return htmlutil.HTMLFormat(`class="code-block%s"`, preExtraClasses), htmlutil.HTMLFormat(`class="chroma language-%s" data-code-language="%s"`, cssName, attrLang)
+}

modules/highlight/highlight_test.go

@@ -216,3 +216,19 @@ func TestUnsafeSplitHighlightedLines(t *testing.T) {
 	assert.Equal(t, "<span>a</span>\n", string(ret[0]))
 	assert.Equal(t, "<span>b\n</span>", string(ret[1]))
 }
+
+func TestCodeBlockAttributes(t *testing.T) {
+	test := func(t *testing.T, lang string, css, attr template.HTML) {
+		t.Helper()
+		cssActual, attrActual := CodeBlockAttributes(lang)
+		assert.Equal(t, css, cssActual)
+		assert.Equal(t, attr, attrActual)
+	}
+	for _, s := range []string{"", "FALLback", "plainTEXT"} {
+		test(t, s, `class="code-block"`, `class="chroma language-text" data-code-language="text"`)
+	}
+	test(t, "math", `class="code-block is-loading"`, `class="chroma language-math" data-code-language="math"`)
+	test(t, "mermaid", `class="code-block is-loading"`, `class="chroma language-mermaid" data-code-language="mermaid"`)
+	test(t, "Visual Basic.NET", `class="code-block"`, `class="chroma language-visual_basic_net" data-code-language="Visual Basic.NET"`)
+	test(t, "c++-x", `class="code-block"`, `class="chroma language-c___x" data-code-language="c++-x"`)
+}

modules/markup/internal/renderinternal.go

@@ -4,8 +4,6 @@
 package internal
 
 import (
-	"crypto/rand"
-	"encoding/base64"
 	"html/template"
 	"io"
 	"regexp"
@@ -13,6 +11,7 @@ import (
 	"sync"
 
 	"gitea.dev/modules/htmlutil"
+	"gitea.dev/modules/util"
 
 	"golang.org/x/net/html"
 )
@@ -30,12 +29,7 @@ type RenderInternal struct {
 }
 
 func (r *RenderInternal) Init(output io.Writer, extraHeadHTML template.HTML) io.WriteCloser {
-	buf := make([]byte, 12)
+	return r.init(util.FastCryptoRandomHex(16), output, extraHeadHTML)
-	_, err := rand.Read(buf)
-	if err != nil {
-		panic("unable to generate secure id")
-	}
-	return r.init(base64.URLEncoding.EncodeToString(buf), output, extraHeadHTML)
 }
 
 func (r *RenderInternal) init(secID string, output io.Writer, extraHeadHTML template.HTML) io.WriteCloser {

modules/markup/jupyter/jupyter.go

@@ -214,8 +214,9 @@ func renderCellCode(output htmlutil.HTMLWriter, cell Cell, language string) erro
 		}
 
 		// Highlight code
+		preAttrs, codeAttrs := highlight.CodeBlockAttributes(language)
 		lexer := highlight.DetectChromaLexerByFileName("", language)
-		output.WriteFormat(`<div class="cell-right cell-input"><pre><code class="chroma language-%s">`, strings.ToLower(language))
+		output.WriteFormat(`<div class="cell-right cell-input"><pre %s><code %s>`, preAttrs, codeAttrs)
 		output.WriteHTML(highlight.RenderCodeByLexer(lexer, source))
 		output.WriteHTML("</code></pre></div>")
 	}

modules/markup/jupyter/jupyter_test.go

@@ -261,7 +261,7 @@ func TestIntegrationAndSanitization(t *testing.T) {
 	maliciousNotebook := `{
 		"nbformat": 4,
 		"nbformat_minor": 2,
-		"metadata": {},
+		"metadata": {"language_info":{"name":"any lang"}},
 		"cells": [
 			{
 				"cell_type": "code",
@@ -295,8 +295,8 @@ func TestIntegrationAndSanitization(t *testing.T) {
 		<div class="cell-line">
 			<div class="cell-left cell-prompt">In [1]:</div>
 			<div class="cell-right cell-input">
-				<pre><code class="chroma language-python">
+				<pre class="code-block"><code class="chroma language-any_lang" data-code-language="any lang">
-					<span class="n">a</span><span class="o">=</span><span class="mi">1</span>
+					a=1
 				</code></pre>
 			</div>
 		</div>

modules/markup/markdown/goldmark.go

@@ -7,6 +7,8 @@ import (
 	"fmt"
 
 	"gitea.dev/modules/container"
+	"gitea.dev/modules/highlight"
+	"gitea.dev/modules/htmlutil"
 	"gitea.dev/modules/markup"
 	"gitea.dev/modules/markup/internal"
 
@@ -129,7 +131,8 @@ func (r *HTMLRenderer) RegisterFuncs(reg renderer.NodeRendererFuncRegisterer) {
 // renderCodeBlock wraps indented code blocks like the fenced renderer
 func (r *HTMLRenderer) renderCodeBlock(w util.BufWriter, source []byte, n ast.Node, entering bool) (ast.WalkStatus, error) {
 	if entering {
-		opening := r.renderInternal.ProtectSafeAttrs(`<div class="code-block-container code-overflow-scroll"><pre class="code-block"><code>`)
+		preAttrs, codeAttrs := highlight.CodeBlockAttributes("") // no language
+		opening := r.renderInternal.ProtectSafeAttrs(htmlutil.HTMLFormat(`<div class="code-block-container code-overflow-scroll"><pre %s><code %s>`, preAttrs, codeAttrs))
 		if _, err := w.WriteString(string(opening)); err != nil {
 			return ast.WalkStop, err
 		}

modules/markup/markdown/markdown.go

@@ -11,6 +11,7 @@ import (
 	"io"
 	"strings"
 
+	"gitea.dev/modules/highlight"
 	"gitea.dev/modules/htmlutil"
 	"gitea.dev/modules/log"
 	"gitea.dev/modules/markup"
@@ -78,17 +79,8 @@ func (r *GoldmarkRender) Convert(source []byte, writer io.Writer, opts ...parser
 func (r *GoldmarkRender) highlightingRenderer(w util.BufWriter, c highlighting.CodeBlockContext, entering bool) {
 	if entering {
 		languageBytes, _ := c.Language()
-		languageStr := giteautil.IfZero(string(languageBytes), "text")
+		preAttrs, codeAttrs := highlight.CodeBlockAttributes(string(languageBytes))
-
+		err := r.ctx.RenderInternal.FormatWithSafeAttrs(w, `<div class="code-block-container code-overflow-scroll"><pre %s><code %s>`, preAttrs, codeAttrs)
-		preClasses := "code-block"
-		if languageStr == "mermaid" || languageStr == "math" {
-			preClasses += " is-loading"
-		}
-
-		// include language-x class as part of commonmark spec, "chroma" class is used to highlight the code
-		// the "display" class is used by "js/markup/math.ts" to render the code element as a block
-		// the "math.ts" strictly depends on the structure: <pre class="code-block is-loading"><code class="language-math display">...</code></pre>
-		err := r.ctx.RenderInternal.FormatWithSafeAttrs(w, `<div class="code-block-container code-overflow-scroll"><pre class="%s"><code class="chroma language-%s display">`, preClasses, languageStr)
 		if err != nil {
 			return
 		}

modules/markup/markdown/markdown_math_test.go

@@ -135,7 +135,7 @@ func TestMathRenderBlockIndent(t *testing.T) {
 \alpha
 \]
 `,
-			`<pre class="code-block is-loading"><code class="language-math display">
+			`<pre class="code-block is-loading"><code class="language-math">
 \alpha
 </code></pre>
 `,
@@ -147,7 +147,7 @@ func TestMathRenderBlockIndent(t *testing.T) {
  \alpha
  \]
 `,
-			`<pre class="code-block is-loading"><code class="language-math display">
+			`<pre class="code-block is-loading"><code class="language-math">
 \alpha
 </code></pre>
 `,
@@ -162,7 +162,7 @@ a
    d
   \]
 `,
-			`<pre class="code-block is-loading"><code class="language-math display">
+			`<pre class="code-block is-loading"><code class="language-math">
 a
 b
 c
@@ -179,7 +179,7 @@ c
   c
   \]
 `,
-			`<pre class="code-block is-loading"><code class="language-math display">
+			`<pre class="code-block is-loading"><code class="language-math">
 a
  b
 c
@@ -190,7 +190,7 @@ c
 			"indent-0-oneline",
 			`$$ x $$
 foo`,
-			`<code class="language-math display"> x </code>
+			`<code class="language-math"> x </code>
 <p>foo</p>
 `,
 		},
@@ -198,7 +198,7 @@ foo`,
 			"indent-3-oneline",
 			`   $$ x $$<SPACE>
 foo`,
-			`<code class="language-math display"> x </code>
+			`<code class="language-math"> x </code>
 <p>foo</p>
 `,
 		},
@@ -213,10 +213,10 @@ foo`,
 > \]
 `,
 			`<blockquote>
-<pre class="code-block is-loading"><code class="language-math display">
+<pre class="code-block is-loading"><code class="language-math">
 a
 </code></pre>
-<pre class="code-block is-loading"><code class="language-math display">
+<pre class="code-block is-loading"><code class="language-math">
 b
 </code></pre>
 </blockquote>
@@ -232,7 +232,7 @@ b
 2. b`,
 			`<ol>
 <li>a
-<pre class="code-block is-loading"><code class="language-math display">
+<pre class="code-block is-loading"><code class="language-math">
 x
 </code></pre>
 </li>
@@ -288,7 +288,7 @@ a
 $$
 `)
 	setting.Markdown.MathCodeBlockOptions.ParseBlockDollar = true
-	test(t, `<pre class="code-block is-loading"><code class="language-math display">
+	test(t, `<pre class="code-block is-loading"><code class="language-math">
 a
 </code></pre>
 `, `
@@ -307,7 +307,7 @@ a
 \]
 `)
 	setting.Markdown.MathCodeBlockOptions.ParseBlockSquareBrackets = true
-	test(t, `<pre class="code-block is-loading"><code class="language-math display">
+	test(t, `<pre class="code-block is-loading"><code class="language-math">
 a
 </code></pre>
 `, `

modules/markup/markdown/markdown_test.go

@@ -611,13 +611,12 @@ func TestMarkdownCodeBlock(t *testing.T) {
 	const prefix = `<div class="code-block-container code-overflow-scroll"><pre class="code-block">`
 	const suffix = `</pre></div>`
 
-	testRender("```\ncode\n```", prefix+`<code class="chroma language-text display">code`+nl+`</code>`+suffix)
+	testRender("```\ncode\n```", prefix+`<code class="chroma language-text">code`+nl+`</code>`+suffix)
 
-	const jsCommon = prefix + `<code class="chroma language-js display"><span class="nx">code</span>` + nl + `</code>` + suffix
+	const jsCommon = prefix + `<code class="chroma language-js"><span class="nx">code</span>` + nl + `</code>` + suffix
 	testRender("```js\ncode\n```", jsCommon)
 	testRender("```js:app.ts\ncode\n```", jsCommon)
 	testRender("```js,ignore\ncode\n```", jsCommon)
 	testRender("```js ignore\ncode\n```", jsCommon)
-	testRender("    code\n", prefix+`<code>code`+nl+`</code>`+suffix)
+	testRender("    <any&content>\n", prefix+`<code class="chroma language-text">&lt;any&amp;content&gt;`+nl+`</code>`+suffix)
-	testRender("    <script>alert(1)</script>\n", prefix+`<code>&lt;script&gt;alert(1)&lt;/script&gt;`+nl+`</code>`+suffix)
 }

modules/markup/markdown/math/block_renderer.go

@@ -15,11 +15,11 @@ import (
 )
 
 // Block render output:
-// 	<pre class="code-block is-loading"><code class="language-math display">...</code></pre>
+// 	<pre class="code-block is-loading"><code class="language-math">...</code></pre>
 //
-// Keep in mind that there is another "code block" render in "func (r *GlodmarkRender) highlightingRenderer"
+// Keep in mind that there is another "code block" render in "func (r *GoldmarkRender) highlightingRenderer"
 // "highlightingRenderer" outputs the math block with extra "chroma" class:
-// 	<pre class="code-block is-loading"><code class="chroma language-math display">...</code></pre>
+// 	<pre class="code-block is-loading"><code class="chroma language-math">...</code></pre>
 //
 // Special classes:
 // * "is-loading": show a loading indicator
@@ -51,7 +51,7 @@ func (r *BlockRenderer) writeLines(w util.BufWriter, source []byte, n gast.Node)
 func (r *BlockRenderer) renderBlock(w util.BufWriter, source []byte, node gast.Node, entering bool) (gast.WalkStatus, error) {
 	n := node.(*Block)
 	if entering {
-		codeHTML := giteaUtil.Iif[template.HTML](n.Inline, "", `<pre class="code-block is-loading">`) + `<code class="language-math display">`
+		codeHTML := giteaUtil.Iif[template.HTML](n.Inline, "", `<pre class="code-block is-loading">`) + `<code class="language-math">`
 		_, _ = w.WriteString(string(r.renderInternal.ProtectSafeAttrs(codeHTML)))
 		r.writeLines(w, source, n)
 	} else {

modules/markup/orgmode/orgmode.go

@@ -56,12 +56,12 @@ func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error
 			}
 		}()
 
+		preAttrs, codeAttrs := highlight.CodeBlockAttributes(lang)
 		lexer := highlight.DetectChromaLexerByFileName("", lang) // don't use content to detect, it is too slow
 		lexer = chroma.Coalesce(lexer)
 
 		sb := &strings.Builder{}
-		// include language-x class as part of commonmark spec
+		_ = ctx.RenderInternal.FormatWithSafeAttrs(sb, `<pre %s><code %s>`, preAttrs, codeAttrs)
-		_ = ctx.RenderInternal.FormatWithSafeAttrs(sb, `<pre><code class="chroma language-%s">`, strings.ToLower(lexer.Config().Name))
 		_, _ = sb.WriteString(string(highlight.RenderCodeByLexer(lexer, source)))
 		_, _ = sb.WriteString("</code></pre>")
 		return sb.String()

modules/markup/orgmode/orgmode_test.go

@@ -83,12 +83,12 @@ func TestRender_Source(t *testing.T) {
 int a;
 #+end_src
 `, `<div class="src src-c">
-<pre><code class="chroma language-c"><span class="kt">int</span> <span class="n">a</span><span class="p">;</span></code></pre>
+<pre class="code-block"><code class="chroma language-c" data-code-language="c"><span class="kt">int</span> <span class="n">a</span><span class="p">;</span></code></pre>
 </div>`)
 }
 
 func TestRender_IncludeLink(t *testing.T) {
 	testRender(t, `#+INCLUDE: "./other.org" src text`, `<div class="src src-text">
-<pre><code class="chroma language-plaintext">#+INCLUDE: [[other.org]]</code></pre>
+<pre class="code-block"><code class="chroma language-text" data-code-language="text">#+INCLUDE: [[other.org]]</code></pre>
 </div>`)
 }

web_src/js/markup/math.ts

@@ -1,14 +1,15 @@
 import {displayError} from './common.ts';
 import {queryElems} from '../utils/dom.ts';
 
-function targetElement(el: Element): {target: Element, displayAsBlock: boolean} {
+function targetElement(elCode: Element): {target: Element, displayAsBlock: boolean} {
   // The target element is either the parent "code block with loading indicator", or itself
   // It is designed to work for 2 cases (guaranteed by backend code):
-  // * <pre class="code-block is-loading"><code class="language-math display">...</code></pre>
+  // * <pre class="code-block"><code class="language-math">...</code></pre>
   // * <code class="language-math">...</code>
+  const elPre = elCode.parentElement?.matches('pre.code-block') ? elCode.parentElement : null;
   return {
-    target: el.closest('.code-block.is-loading') ?? el,
+    target: elPre ?? elCode,
-    displayAsBlock: el.classList.contains('display'),
+    displayAsBlock: elPre !== null,
   };
 }
 

web_src/js/markup/mermaid.ts

@@ -156,11 +156,11 @@ let elkLayoutsRegistered = false;
 
 export async function initMarkupCodeMermaid(elMarkup: HTMLElement): Promise<void> {
   // .markup code.language-mermaid
-  const mermaidBlocks: Array<{source: string, parentContainer: HTMLElement}> = [];
+  const mermaidBlocks: Array<{source: string, parentContainer: Element}> = [];
   const attrMermaidRendered = 'data-markup-mermaid-rendered';
   let needElkRender = false;
   for (const elCodeBlock of queryElems(elMarkup, 'code.language-mermaid')) {
-    const parentContainer = elCodeBlock.closest('pre')!; // it must exist, if no, there must be a bug
+    const parentContainer = elCodeBlock.closest('pre.code-block')!; // it must exist, if no, there must be a bug
     if (parentContainer.hasAttribute(attrMermaidRendered)) continue;
     parentContainer.setAttribute(attrMermaidRendered, 'true');
 

web_src/js/modules/observer.ts

@@ -34,7 +34,7 @@ export function registerGlobalSelectorFunc<T extends Element>(selector: string,
   }
 }
 
-// It handles the global init functions for all `<div data-global-int="initSomeElem"></div>` elements.
+// It handles the global init functions for all `<div data-global-init="initSomeElem"></div>` elements.
 export function registerGlobalInitFunc<T extends HTMLElement>(name: string, handler: GlobalInitFunc<T>) {
   globalInitFuncs[name] = handler as GlobalInitFunc<Element>;
   // The "global init" functions are managed internally and called by callGlobalInitFunc