From 6ff3a65708dd6a6472f7b164bb31a842a4c4d672 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 6 Aug 2026 18:07:36 +0800 Subject: [PATCH] refactor: render highlight language (#38793) Avoid CSS injection More details are in the comment of CodeBlockAttributes --- docs/guidelines-backend.md | 2 +- models/renderhelper/repo_file_test.go | 2 +- modules/highlight/highlight.go | 50 +++++++++++++++++++ modules/highlight/highlight_test.go | 16 ++++++ modules/markup/internal/renderinternal.go | 10 +--- modules/markup/jupyter/jupyter.go | 3 +- modules/markup/jupyter/jupyter_test.go | 6 +-- modules/markup/markdown/goldmark.go | 5 +- modules/markup/markdown/markdown.go | 14 ++---- modules/markup/markdown/markdown_math_test.go | 22 ++++---- modules/markup/markdown/markdown_test.go | 7 ++- .../markup/markdown/math/block_renderer.go | 8 +-- modules/markup/orgmode/orgmode.go | 4 +- modules/markup/orgmode/orgmode_test.go | 4 +- web_src/js/markup/math.ts | 9 ++-- web_src/js/markup/mermaid.ts | 4 +- web_src/js/modules/observer.ts | 2 +- 17 files changed, 112 insertions(+), 56 deletions(-) diff --git a/docs/guidelines-backend.md b/docs/guidelines-backend.md index 8fca9cb1a03..87c67b2771e 100644 --- a/docs/guidelines-backend.md +++ b/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. diff --git a/models/renderhelper/repo_file_test.go b/models/renderhelper/repo_file_test.go index 45b7006a43e..2f28d2c9065 100644 --- a/models/renderhelper/repo_file_test.go +++ b/models/renderhelper/repo_file_test.go @@ -113,7 +113,7 @@ int a = 1; `) assert.NoError(t, err) assert.Equal(t, `
-
int a = 1;
+
int a = 1;
`, rendered) }) diff --git a/modules/highlight/highlight.go b/modules/highlight/highlight.go index 1ca97c4968e..a93fe1859d4 100644 --- a/modules/highlight/highlight.go +++ b/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:
...
+ // * 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) +} diff --git a/modules/highlight/highlight_test.go b/modules/highlight/highlight_test.go index a31f8752f15..5c50844e3b9 100644 --- a/modules/highlight/highlight_test.go +++ b/modules/highlight/highlight_test.go @@ -216,3 +216,19 @@ func TestUnsafeSplitHighlightedLines(t *testing.T) { assert.Equal(t, "a\n", string(ret[0])) assert.Equal(t, "b\n", 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"`) +} diff --git a/modules/markup/internal/renderinternal.go b/modules/markup/internal/renderinternal.go index 3b5886a19ff..bf33b01f3db 100644 --- a/modules/markup/internal/renderinternal.go +++ b/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) - _, err := rand.Read(buf) - if err != nil { - panic("unable to generate secure id") - } - return r.init(base64.URLEncoding.EncodeToString(buf), output, extraHeadHTML) + return r.init(util.FastCryptoRandomHex(16), output, extraHeadHTML) } func (r *RenderInternal) init(secID string, output io.Writer, extraHeadHTML template.HTML) io.WriteCloser { diff --git a/modules/markup/jupyter/jupyter.go b/modules/markup/jupyter/jupyter.go index a02eb4a0287..40109f73586 100644 --- a/modules/markup/jupyter/jupyter.go +++ b/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(`
`, strings.ToLower(language))
+		output.WriteFormat(`
`, preAttrs, codeAttrs)
 		output.WriteHTML(highlight.RenderCodeByLexer(lexer, source))
 		output.WriteHTML("
") } diff --git a/modules/markup/jupyter/jupyter_test.go b/modules/markup/jupyter/jupyter_test.go index 61d362da987..673cc4309f6 100644 --- a/modules/markup/jupyter/jupyter_test.go +++ b/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) {
In [1]:
-

-					a=1
+				

+					a=1
 				
diff --git a/modules/markup/markdown/goldmark.go b/modules/markup/markdown/goldmark.go index 0fdd4cc24ff..88c86dd7b4a 100644 --- a/modules/markup/markdown/goldmark.go +++ b/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(`
`)
+		preAttrs, codeAttrs := highlight.CodeBlockAttributes("") // no language
+		opening := r.renderInternal.ProtectSafeAttrs(htmlutil.HTMLFormat(`
`, preAttrs, codeAttrs))
 		if _, err := w.WriteString(string(opening)); err != nil {
 			return ast.WalkStop, err
 		}
diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go
index 0467ed9ba0c..b906c1c0ac7 100644
--- a/modules/markup/markdown/markdown.go
+++ b/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")
-
-		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: 
...
- err := r.ctx.RenderInternal.FormatWithSafeAttrs(w, `
`, preClasses, languageStr)
+		preAttrs, codeAttrs := highlight.CodeBlockAttributes(string(languageBytes))
+		err := r.ctx.RenderInternal.FormatWithSafeAttrs(w, `
`, preAttrs, codeAttrs)
 		if err != nil {
 			return
 		}
diff --git a/modules/markup/markdown/markdown_math_test.go b/modules/markup/markdown/markdown_math_test.go
index fc3ca2cf470..08100742476 100644
--- a/modules/markup/markdown/markdown_math_test.go
+++ b/modules/markup/markdown/markdown_math_test.go
@@ -135,7 +135,7 @@ func TestMathRenderBlockIndent(t *testing.T) {
 \alpha
 \]
 `,
-			`

+			`

 \alpha
 
`, @@ -147,7 +147,7 @@ func TestMathRenderBlockIndent(t *testing.T) { \alpha \] `, - `

+			`

 \alpha
 
`, @@ -162,7 +162,7 @@ a d \] `, - `

+			`

 a
 b
 c
@@ -179,7 +179,7 @@ c
   c
   \]
 `,
-			`

+			`

 a
  b
 c
@@ -190,7 +190,7 @@ c
 			"indent-0-oneline",
 			`$$ x $$
 foo`,
-			` x 
+			` x 
 

foo

`, }, @@ -198,7 +198,7 @@ foo`, "indent-3-oneline", ` $$ x $$ foo`, - ` x + ` x

foo

`, }, @@ -213,10 +213,10 @@ foo`, > \] `, `
-

+

 a
 
-

+

 b
 
@@ -232,7 +232,7 @@ b 2. b`, `
  1. a -
    
    +
    
     x
     
  2. @@ -288,7 +288,7 @@ a $$ `) setting.Markdown.MathCodeBlockOptions.ParseBlockDollar = true - test(t, `
    
    +	test(t, `
    
     a
     
    `, ` @@ -307,7 +307,7 @@ a \] `) setting.Markdown.MathCodeBlockOptions.ParseBlockSquareBrackets = true - test(t, `
    
    +	test(t, `
    
     a
     
    `, ` diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index 11ff7b09c5c..c0cb740ae9a 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -611,13 +611,12 @@ func TestMarkdownCodeBlock(t *testing.T) { const prefix = `
    `
     	const suffix = `
    ` - testRender("```\ncode\n```", prefix+`code`+nl+``+suffix) + testRender("```\ncode\n```", prefix+`code`+nl+``+suffix) - const jsCommon = prefix + `code` + nl + `` + suffix + const jsCommon = prefix + `code` + nl + `` + 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`+nl+``+suffix) - testRender(" \n", prefix+`<script>alert(1)</script>`+nl+``+suffix) + testRender(" \n", prefix+`<any&content>`+nl+``+suffix) } diff --git a/modules/markup/markdown/math/block_renderer.go b/modules/markup/markdown/math/block_renderer.go index 2d589cf7d77..b2a5662bc9d 100644 --- a/modules/markup/markdown/math/block_renderer.go +++ b/modules/markup/markdown/math/block_renderer.go @@ -15,11 +15,11 @@ import ( ) // Block render output: -//
    ...
    +//
    ...
    // -// 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: -//
    ...
    +//
    ...
    // // 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, "", `
    `) + ``
    +		codeHTML := giteaUtil.Iif[template.HTML](n.Inline, "", `
    `) + ``
     		_, _ = w.WriteString(string(r.renderInternal.ProtectSafeAttrs(codeHTML)))
     		r.writeLines(w, source, n)
     	} else {
    diff --git a/modules/markup/orgmode/orgmode.go b/modules/markup/orgmode/orgmode.go
    index bcb0df8ffd2..4685028c015 100644
    --- a/modules/markup/orgmode/orgmode.go
    +++ b/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, `
    `, strings.ToLower(lexer.Config().Name))
    +		_ = ctx.RenderInternal.FormatWithSafeAttrs(sb, `
    `, preAttrs, codeAttrs)
     		_, _ = sb.WriteString(string(highlight.RenderCodeByLexer(lexer, source)))
     		_, _ = sb.WriteString("
    ") return sb.String() diff --git a/modules/markup/orgmode/orgmode_test.go b/modules/markup/orgmode/orgmode_test.go index 28607a15311..d1dea919fe0 100644 --- a/modules/markup/orgmode/orgmode_test.go +++ b/modules/markup/orgmode/orgmode_test.go @@ -83,12 +83,12 @@ func TestRender_Source(t *testing.T) { int a; #+end_src `, `
    -
    int a;
    +
    int a;
    `) } func TestRender_IncludeLink(t *testing.T) { testRender(t, `#+INCLUDE: "./other.org" src text`, `
    -
    #+INCLUDE: [[other.org]]
    +
    #+INCLUDE: [[other.org]]
    `) } diff --git a/web_src/js/markup/math.ts b/web_src/js/markup/math.ts index a3ee102ccde..69cd164f8d2 100644 --- a/web_src/js/markup/math.ts +++ b/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): - // *
    ...
    + // *
    ...
    // * ... + const elPre = elCode.parentElement?.matches('pre.code-block') ? elCode.parentElement : null; return { - target: el.closest('.code-block.is-loading') ?? el, - displayAsBlock: el.classList.contains('display'), + target: elPre ?? elCode, + displayAsBlock: elPre !== null, }; } diff --git a/web_src/js/markup/mermaid.ts b/web_src/js/markup/mermaid.ts index 59a10a1b9ec..10016c4471b 100644 --- a/web_src/js/markup/mermaid.ts +++ b/web_src/js/markup/mermaid.ts @@ -156,11 +156,11 @@ let elkLayoutsRegistered = false; export async function initMarkupCodeMermaid(elMarkup: HTMLElement): Promise { // .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'); diff --git a/web_src/js/modules/observer.ts b/web_src/js/modules/observer.ts index c82d9ce7e0a..5abe3033bea 100644 --- a/web_src/js/modules/observer.ts +++ b/web_src/js/modules/observer.ts @@ -34,7 +34,7 @@ export function registerGlobalSelectorFunc(selector: string, } } -// It handles the global init functions for all `
    ` elements. +// It handles the global init functions for all `
    ` elements. export function registerGlobalInitFunc(name: string, handler: GlobalInitFunc) { globalInitFuncs[name] = handler as GlobalInitFunc; // The "global init" functions are managed internally and called by callGlobalInitFunc