perf(charset): skip plain ASCII text when escaping control characters (#39937)

Every line of file, blame, diff and code preview views goes through
`EscapeControlHTML`, which streamed it through a 32 KiB read buffer and
copied every tag and text part. Views render the `EscapeStatus` before
the content, so every caller holds the complete HTML and streaming can't
help. It is now escaped in one pass over the string and returned
unchanged when nothing needs escaping.

- Skip text of printable ASCII, tabs and newlines, such lines return
immediately
- Render markup and wiki pages into a buffer instead of a pipe and
goroutine, and remove `EscapeControlReader`
- Analyze long text parts in windows overlapping by the detection
context, which bounds memory without changing results
- Escape invalid bytes before a tag, they were moved into the tag part
unescaped and could hide a `<math>` tag

| | before | after |
|---|---|---|
| ASCII line | 5.1 µs, 59 allocs | 0.08 µs, 1 alloc |
| line with one `é` | 3.9 µs, 30 allocs | 0.29 µs, 3 allocs |
| 150 KB document | 2.5 ms, 5.6 MB | 0.27 ms, 0.6 KB |

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: silverwind <me@silverwind.io>

modules/charset/ambiguous_gen_test.go

@@ -29,6 +29,9 @@ func TestAmbiguousCharacters(t *testing.T) {
 			found := i < len(ambiguous.Confusable) && ambiguous.Confusable[i] == confusable
 			assert.True(t, found, "%c is not in %d", confusable, i)
 		}
+		for r := rune(0x20); r < 0x7f; r++ {
+			assert.False(t, unicode.Is(ambiguous.RangeTable, r), "%c must not be ambiguous, isStringAsciiSafe skips it", r)
+		}
 	}
 
 	var confusableTo rune

modules/charset/charset.go

@@ -22,15 +22,14 @@ import (
 	"golang.org/x/text/transform"
 )
 
-var globalVars = sync.OnceValue(func() (ret struct {
-	utf8Bom []byte
+const utf8Bom = "\xef\xbb\xbf"
 
+var globalVars = sync.OnceValue(func() (ret struct {
 	defaultWordRegexp   *regexp.Regexp
 	ambiguousTableMap   map[string]*AmbiguousTable
 	invisibleRangeTable *unicode.RangeTable
 },
 ) {
-	ret.utf8Bom = []byte("\xef\xbb\xbf")
 	ret.ambiguousTableMap = newAmbiguousTableMap()
 	ret.invisibleRangeTable = newInvisibleRangeTable()
 	return ret
@@ -139,7 +138,7 @@ func maybeRemoveBOM(content []byte, opts ConvertOpts) []byte {
 	if opts.KeepBOM {
 		return content
 	}
-	return bytes.TrimPrefix(content, globalVars().utf8Bom)
+	return bytes.TrimPrefix(content, []byte(utf8Bom))
 }
 
 // DetectEncoding detect the encoding of content

modules/charset/escape.go

@@ -5,10 +5,8 @@ package charset
 
 import (
 	"html/template"
-	"io"
-	"strings"
+	"sync"
 
-	"gitea.dev/modules/htmlutil"
 	"gitea.dev/modules/setting"
 	"gitea.dev/modules/translation"
 )
@@ -28,21 +26,31 @@ func EscapeOptionsForView() EscapeOptions {
 	}
 }
 
-// EscapeControlHTML escapes the Unicode control sequences in a provided html document
-func EscapeControlHTML(html template.HTML, locale translation.Locale, opts ...EscapeOptions) (escaped *EscapeStatus, output template.HTML) {
-	if !setting.UI.AmbiguousUnicodeDetection {
-		return &EscapeStatus{}, html
+var asciiSafeBytes = sync.OnceValue(func() (ret [256]bool) {
+	for c := range ret {
+		ret[c] = (c >= 0x20 && c < 0x7f) || c == '\t' || c == '\n'
 	}
-	w := &htmlutil.HTMLBuilder{}
-	escaped, _ = EscapeControlReader(strings.NewReader(string(html)), w, locale, opts...)
-	return escaped, w.HTMLString()
+	return ret
+})
+
+func isStringAsciiSafe(str string) bool {
+	safeBytes := asciiSafeBytes()
+	for i := range len(str) {
+		if !safeBytes[str[i]] {
+			return false
+		}
+	}
+	return true
 }
 
-// EscapeControlReader escapes the Unicode control sequences in a provided reader of HTML content and writer in a locale and returns the findings as an EscapeStatus
-func EscapeControlReader(reader io.Reader, writer htmlutil.HTMLWriter, locale translation.Locale, opts ...EscapeOptions) (*EscapeStatus, error) {
-	if !setting.UI.AmbiguousUnicodeDetection {
-		_, err := io.Copy(writer.OriginWriter(), reader)
-		return &EscapeStatus{}, err
+// EscapeControlHTML escapes the Unicode control sequences in a provided html document, in memory as views render the EscapeStatus first
+func EscapeControlHTML(html template.HTML, locale translation.Locale, opts ...EscapeOptions) (escaped *EscapeStatus, output template.HTML) {
+	if !setting.UI.AmbiguousUnicodeDetection || isStringAsciiSafe(string(html)) {
+		return &EscapeStatus{}, html
 	}
-	return escapeStream(locale, reader, writer.OriginWriter(), opts...)
+	escaper := &htmlEscaper{escaped: &EscapeStatus{}, locale: locale, ambiguousTables: AmbiguousTablesForLocale(locale)}
+	if len(opts) > 0 {
+		escaper.allowed = opts[0].Allowed
+	}
+	return escaper.escaped, template.HTML(escaper.escape(string(html)))
 }

modules/charset/escape_stream.go

@@ -4,116 +4,96 @@
 package charset
 
 import (
-	"bytes"
 	"fmt"
 	"html"
-	"io"
+	"slices"
 	"strings"
 	"unicode"
 	"unicode/utf8"
 
-	"gitea.dev/modules/setting"
 	"gitea.dev/modules/translation"
 	"gitea.dev/modules/util"
 )
 
-type htmlChunkReader struct {
-	in       io.Reader
-	readErr  error
-	readBuf  []byte
-	curInTag bool
-}
-
-type escapeStreamer struct {
-	htmlChunkReader
-
+type htmlEscaper struct {
 	escaped         *EscapeStatus
 	locale          translation.Locale
 	ambiguousTables []*AmbiguousTable
 	allowed         map[rune]bool
 
-	tagPartial []byte // partial tag content, used to detect if we are in some tags
-
 	inTagMath bool // MathML operators like U+2212 are intended and wrapping them breaks the math layout
 
-	out io.Writer
+	detectResults []detectResult
+	written       int // content before this offset has been written to out
+	out           strings.Builder
 }
 
-func escapeStream(locale translation.Locale, in io.Reader, out io.Writer, opts ...EscapeOptions) (*EscapeStatus, error) {
-	es := &escapeStreamer{
-		escaped:         &EscapeStatus{},
-		locale:          locale,
-		ambiguousTables: AmbiguousTablesForLocale(locale),
-		htmlChunkReader: htmlChunkReader{
-			in:      in,
-			readBuf: make([]byte, 0, 32*1024),
-		},
-		out: out,
-	}
+const detectWindowRunes = 4096 // bounds the detection results of long text parts
 
-	if len(opts) > 0 {
-		es.allowed = opts[0].Allowed
-	}
-
-	readCount := 0
-	lastIsTag := false
-	for {
-		parts, partInTag, err := es.readRunes()
-		readCount++
-		if err == io.EOF {
-			return es.escaped, nil
-		} else if err != nil {
-			return nil, err
+func (e *htmlEscaper) escape(content string) string {
+	for pos := 0; pos < len(content); {
+		if content[pos] == '<' {
+			tagEnd := len(content)
+			if idx := strings.IndexByte(content[pos:], '>'); idx != -1 {
+				tagEnd = pos + idx + 1
+			}
+			e.trackHtmlTag(content[pos:tagEnd])
+			pos = tagEnd
+			continue
 		}
-		for i, part := range parts {
-			if partInTag[i] {
-				lastIsTag = true
-				es.trackHtmlTag(part)
-				if _, err := out.Write(part); err != nil {
-					return nil, err
-				}
-			} else {
-				// if last part is tag, then this part is content begin
-				// if the content is the first part of the first read, then it's also content begin
-				isContentBegin := lastIsTag || (readCount == 1 && i == 0)
-				lastIsTag = false
-				if isContentBegin {
-					if part, err = es.trimAndWriteBom(part); err != nil {
-						return nil, err
-					}
-				}
-				if es.inTagMath {
-					if _, err := out.Write(part); err != nil {
-						return nil, err
-					}
-				} else if err = es.detectAndWriteRunes(part); err != nil {
-					return nil, err
-				}
+		textEnd := len(content)
+		if idx := strings.IndexByte(content[pos:], '<'); idx != -1 {
+			textEnd = pos + idx
+		}
+		if strings.HasPrefix(content[pos:textEnd], utf8Bom) {
+			pos += len(utf8Bom) // a BOM at the start of a text part is kept as is
+		}
+		if !e.inTagMath && !isStringAsciiSafe(content[pos:textEnd]) {
+			e.escapeText(content, pos, textEnd)
+		}
+		pos = textEnd
+	}
+	if e.written == 0 {
+		return content
+	}
+	e.out.WriteString(content[e.written:])
+	return e.out.String()
+}
+
+// escapeText analyzes content[start:end] in windows overlapping by the context analyzeDetectResults looks at
+func (e *htmlEscaper) escapeText(content string, start, end int) {
+	for skip := 0; ; skip = longSentenceDetectionLimit {
+		results := e.detectRunes(content[start:end])
+		e.analyzeDetectResults(results)
+		lastResult := results[len(results)-1]
+		isLastWindow := start+lastResult.position+lastResult.runeSize == end
+		emitEnd := len(results)
+		if !isLastWindow {
+			emitEnd -= longSentenceDetectionLimit
+		}
+		for _, res := range results[skip:emitEnd] {
+			if res.needEscape {
+				e.out.WriteString(content[e.written : start+res.position])
+				e.writeEscapedRune(res)
+				e.written = start + res.position + res.runeSize
 			}
 		}
+		if isLastWindow {
+			return
+		}
+		start += results[emitEnd-longSentenceDetectionLimit].position
 	}
 }
 
-// trackHtmlTag receives tag parts, a tag might be split into multiple parts
-func (e *escapeStreamer) trackHtmlTag(part []byte) {
-	const maxHeadLen = 100 // only read the first N bytes of the tag for detection purpose
-	if part[0] == '<' {
-		// start a new tag
-		e.tagPartial = e.tagPartial[:0]
-	}
-	if len(e.tagPartial) >= maxHeadLen {
-		return
-	}
-	e.tagPartial = append(e.tagPartial, part[:min(len(part), maxHeadLen-len(e.tagPartial))]...)
-
+func (e *htmlEscaper) trackHtmlTag(tag string) {
 	isTag := func(prefix string) bool {
-		if len(e.tagPartial) < len(prefix)+1 {
+		if len(tag) < len(prefix)+1 {
 			return false
 		}
-		if !util.AsciiEqualFold(e.tagPartial[:len(prefix)], []byte(prefix)) {
+		if !util.AsciiEqualFold(tag[:len(prefix)], prefix) {
 			return false
 		}
-		return strings.IndexByte(" \t\n\r\f>", e.tagPartial[len(prefix)]) != -1
+		return strings.IndexByte(" \t\n\r\f>", tag[len(prefix)]) != -1
 	}
 	if isTag("<math") {
 		e.inTagMath = true
@@ -122,20 +102,9 @@ func (e *escapeStreamer) trackHtmlTag(part []byte) {
 	}
 }
 
-func (e *escapeStreamer) trimAndWriteBom(part []byte) ([]byte, error) {
-	remaining, ok := bytes.CutPrefix(part, globalVars().utf8Bom)
-	if ok {
-		part = remaining
-		if _, err := e.out.Write(globalVars().utf8Bom); err != nil {
-			return part, err
-		}
-	}
-	return part, nil
-}
-
 const longSentenceDetectionLimit = 20
 
-func (e *escapeStreamer) possibleLongSentence(results []detectResult, pos int) bool {
+func (e *htmlEscaper) possibleLongSentence(results []detectResult, pos int) bool {
 	countBasic := 0
 	countNonASCII := 0
 	for i := max(pos-longSentenceDetectionLimit, 0); i < min(pos+longSentenceDetectionLimit, len(results)); i++ {
@@ -152,214 +121,56 @@ func (e *escapeStreamer) possibleLongSentence(results []detectResult, pos int) b
 	return countChar != 0 && countNonASCII*100/countChar >= 50
 }
 
-func (e *escapeStreamer) analyzeDetectResults(results []detectResult) {
+func (e *htmlEscaper) analyzeDetectResults(results []detectResult) {
 	for i := range results {
 		res := &results[i]
 		if res.runeType == runeTypeInvisible || res.runeType == runeTypeAmbiguous {
 			leftIsNonASCII := i > 0 && (results[i-1].runeType == runeTypeNonASCII || results[i-1].runeType == runeTypeAmbiguous)
 			rightIsNonASCII := i < len(results)-1 && (results[i+1].runeType == runeTypeNonASCII || results[i+1].runeType == runeTypeAmbiguous)
 			surroundingNonASCII := leftIsNonASCII || rightIsNonASCII
-			if !surroundingNonASCII {
-				if len(results) < longSentenceDetectionLimit {
-					res.needEscape = setting.UI.AmbiguousUnicodeDetection
-				} else if !e.possibleLongSentence(results, i) {
-					res.needEscape = setting.UI.AmbiguousUnicodeDetection
-				}
-			}
+			res.needEscape = !surroundingNonASCII && (len(results) < longSentenceDetectionLimit || !e.possibleLongSentence(results, i))
 		}
 	}
 }
 
-func (e *escapeStreamer) detectAndWriteRunes(part []byte) error {
-	results := e.detectRunes(part)
-	e.analyzeDetectResults(results)
-	return e.writeDetectResults(part, results)
+func (e *htmlEscaper) writeEscapedRune(res detectResult) {
+	switch res.runeType {
+	case runeTypeBroken:
+		e.writeBrokenRune()
+	case runeTypeAmbiguous:
+		e.writeAmbiguousRune(res.runeChar, res.confusable)
+	case runeTypeInvisible:
+		e.writeInvisibleRune(res.runeChar)
+	case runeTypeControlChar:
+		e.writeControlRune(res.runeChar)
+	default:
+		panic("unreachable")
+	}
 }
 
-func (e *htmlChunkReader) readRunes() (parts [][]byte, partInTag []bool, _ error) {
-	// we have read everything, eof
-	if e.readErr != nil && len(e.readBuf) == 0 {
-		return nil, nil, e.readErr
-	}
-
-	// not eof, and the there is space in the buffer, try to read more data
-	if e.readErr == nil && len(e.readBuf) <= cap(e.readBuf)*3/4 {
-		n, err := e.in.Read(e.readBuf[len(e.readBuf):cap(e.readBuf)])
-		e.readErr = err
-		e.readBuf = e.readBuf[:len(e.readBuf)+n]
-	}
-	if len(e.readBuf) == 0 {
-		return nil, nil, e.readErr
-	}
-
-	// try to exact tag parts and content parts
-	pos := 0
-	for pos < len(e.readBuf) {
-		var curPartEnd int
-		nextInTag := e.curInTag
-		if e.curInTag {
-			// if cur part is in tag, try to find the tag close char '>'
-			idx := bytes.IndexByte(e.readBuf[pos:], '>')
-			if idx == -1 {
-				// if no tag close char, then the whole buffer is in tag
-				curPartEnd = len(e.readBuf)
-			} else {
-				// tag part ends, switch to content part
-				curPartEnd = pos + idx + 1
-				nextInTag = !nextInTag
-			}
-		} else {
-			// if cur part is in content, try to find the tag open char '<'
-			idx := bytes.IndexByte(e.readBuf[pos:], '<')
-			if idx == -1 {
-				// if no tag open char, then the whole buffer is in content
-				curPartEnd = len(e.readBuf)
-			} else {
-				// content part ends, switch to tag part
-				curPartEnd = pos + idx
-				nextInTag = !nextInTag
-			}
-		}
-
-		curPartLen := curPartEnd - pos
-		if curPartLen == 0 {
-			// if cur part is empty, only need to switch the part type
-			if e.curInTag == nextInTag {
-				panic("impossible, curPartLen is 0 but the part in tag status is not switched")
-			}
-			e.curInTag = nextInTag
-			continue
-		}
-
-		// now, curPartLen can't be 0
-		curPart := make([]byte, curPartLen)
-		copy(curPart, e.readBuf[pos:curPartEnd])
-		// now we get the curPart bytes, but we can't directly use it, the last rune in it might have been cut
-		// try to decode the last rune, if it's invalid, then we cut the last byte and try again until we get a valid rune or no byte left
-		for i := curPartLen - 1; i >= 0; i-- {
-			last, lastSize := utf8.DecodeRune(curPart[i:])
-			if last == utf8.RuneError && lastSize == 1 {
-				curPartLen--
-			} else {
-				curPartLen += lastSize - 1
-				break
-			}
-		}
-		if curPartLen == 0 {
-			// actually it's impossible that the part doesn't contain any valid rune,
-			// the only case is that the cap(readBuf) is too small, or the origin contain indeed doesn't contain any valid rune
-			// * try to leave the last 4 bytes (possible longest utf-8 encoding) to next round
-			// * at least consume 1 byte to avoid infinite loop
-			curPartLen = max(len(curPart)-utf8.UTFMax, 1)
-		}
-
-		// if curPartLen is not the same as curPart, it means we have cut some bytes,
-		// need to wait for more data if not eof
-		trailingCorrupted := curPartLen != len(curPart)
-
-		// finally, we get the real part we need
-		curPart = curPart[:curPartLen]
-		parts = append(parts, curPart)
-		partInTag = append(partInTag, e.curInTag)
-
-		pos += curPartLen
-		e.curInTag = nextInTag
-
-		if trailingCorrupted && e.readErr == nil {
-			// if the last part is corrupted, and we haven't reach eof, then we need to wait for more data to get the complete part
-			break
-		}
-	}
-
-	copy(e.readBuf, e.readBuf[pos:])
-	e.readBuf = e.readBuf[:len(e.readBuf)-pos]
-	return parts, partInTag, nil
-}
-
-func (e *escapeStreamer) writeDetectResults(data []byte, results []detectResult) error {
-	lastWriteRawIdx := -1
-	for idx := range results {
-		res := &results[idx]
-		if !res.needEscape {
-			if lastWriteRawIdx == -1 {
-				lastWriteRawIdx = idx
-			}
-			continue
-		}
-
-		if lastWriteRawIdx != -1 {
-			if _, err := e.out.Write(data[results[lastWriteRawIdx].position:res.position]); err != nil {
-				return err
-			}
-			lastWriteRawIdx = -1
-		}
-		switch res.runeType {
-		case runeTypeBroken:
-			if err := e.writeBrokenRune(data[res.position : res.position+res.runeSize]); err != nil {
-				return err
-			}
-		case runeTypeAmbiguous:
-			if err := e.writeAmbiguousRune(res.runeChar, res.confusable); err != nil {
-				return err
-			}
-		case runeTypeInvisible:
-			if err := e.writeInvisibleRune(res.runeChar); err != nil {
-				return err
-			}
-		case runeTypeControlChar:
-			if err := e.writeControlRune(res.runeChar); err != nil {
-				return err
-			}
-		default:
-			panic("unreachable")
-		}
-	}
-	if lastWriteRawIdx != -1 {
-		lastResult := results[len(results)-1]
-		if _, err := e.out.Write(data[results[lastWriteRawIdx].position : lastResult.position+lastResult.runeSize]); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-func (e *escapeStreamer) writeBrokenRune(_ []byte) (err error) {
+func (e *htmlEscaper) writeBrokenRune() {
 	// Although we'd like to use the original bytes to display (show the real broken content to users),
-	// however, when this "escape stream" module is applied to the content, the content has already been processed by other modules.
+	// however, when this escaper is applied to the content, the content has already been processed by other modules.
 	// So the invalid bytes just can't be kept till this step, in most (all) cases, the only thing we see here is utf8.RuneError
-	_, err = io.WriteString(e.out, `<span class="broken-code-point"><3E></span>`)
-	return err
+	e.out.WriteString(`<span class="broken-code-point"><3E></span>`)
 }
 
-func (e *escapeStreamer) writeEscapedCharHTML(tag1, attr, tag2, content, tag3 string) (err error) {
-	_, err = io.WriteString(e.out, tag1)
-	if err != nil {
-		return err
-	}
-	_, err = io.WriteString(e.out, html.EscapeString(attr))
-	if err != nil {
-		return err
-	}
-	_, err = io.WriteString(e.out, tag2)
-	if err != nil {
-		return err
-	}
-	_, err = io.WriteString(e.out, html.EscapeString(content))
-	if err != nil {
-		return err
-	}
-	_, err = io.WriteString(e.out, tag3)
-	return err
+func (e *htmlEscaper) writeEscapedCharHTML(tag1, attr, tag2, content, tag3 string) {
+	e.out.WriteString(tag1)
+	e.out.WriteString(html.EscapeString(attr))
+	e.out.WriteString(tag2)
+	e.out.WriteString(html.EscapeString(content))
+	e.out.WriteString(tag3)
 }
 
 func runeToHex(r rune) string {
 	return fmt.Sprintf("[U+%04X]", r)
 }
 
-func (e *escapeStreamer) writeAmbiguousRune(r, c rune) (err error) {
+func (e *htmlEscaper) writeAmbiguousRune(r, c rune) {
 	e.escaped.Escaped = true
 	e.escaped.HasAmbiguous = true
-	return e.writeEscapedCharHTML(
+	e.writeEscapedCharHTML(
 		`<span class="ambiguous-code-point" data-tooltip-content="`,
 		e.locale.TrString("repo.ambiguous_character", string(r)+" "+runeToHex(r), string(c)+" "+runeToHex(c)),
 		`"><span class="char">`,
@@ -368,10 +179,10 @@ func (e *escapeStreamer) writeAmbiguousRune(r, c rune) (err error) {
 	)
 }
 
-func (e *escapeStreamer) writeInvisibleRune(r rune) error {
+func (e *htmlEscaper) writeInvisibleRune(r rune) {
 	e.escaped.Escaped = true
 	e.escaped.HasInvisible = true
-	return e.writeEscapedCharHTML(
+	e.writeEscapedCharHTML(
 		`<span class="escaped-code-point" data-escaped="`,
 		runeToHex(r),
 		`"><span class="char">`,
@@ -380,7 +191,7 @@ func (e *escapeStreamer) writeInvisibleRune(r rune) error {
 	)
 }
 
-func (e *escapeStreamer) writeControlRune(r rune) error {
+func (e *htmlEscaper) writeControlRune(r rune) {
 	var display string
 	if r >= 0 && r <= 0x1f {
 		display = string(0x2400 + r)
@@ -389,7 +200,7 @@ func (e *escapeStreamer) writeControlRune(r rune) error {
 	} else {
 		display = runeToHex(r)
 	}
-	return e.writeEscapedCharHTML(
+	e.writeEscapedCharHTML(
 		`<span class="broken-code-point" data-escaped="`,
 		display,
 		`"><span class="char">`,
@@ -416,42 +227,39 @@ const (
 	runeTypeControlChar
 )
 
-func (e *escapeStreamer) detectRunes(data []byte) []detectResult {
-	runeCount := utf8.RuneCount(data)
-	results := make([]detectResult, runeCount)
+func (e *htmlEscaper) detectRunes(data string) []detectResult {
+	results := slices.Grow(e.detectResults[:0], min(len(data), detectWindowRunes))
 	invisibleRangeTable := globalVars().invisibleRangeTable
-	var i int
-	var confusable rune
-	for pos := 0; pos < len(data); i++ {
-		r, runeSize := utf8.DecodeRune(data[pos:])
-		results[i].runeChar = r
-		results[i].runeSize = runeSize
-		results[i].position = pos
+	safeBytes := asciiSafeBytes()
+	for pos := 0; pos < len(data) && len(results) < detectWindowRunes; {
+		r, runeSize := utf8.DecodeRuneInString(data[pos:])
+		res := detectResult{runeChar: r, runeSize: runeSize, position: pos}
 		pos += runeSize
 
 		switch {
+		case r < utf8.RuneSelf && safeBytes[r]: // basic rune, no table contains printable ASCII
 		case r == utf8.RuneError:
-			results[i].runeType = runeTypeBroken
-			results[i].needEscape = true
-		case r == ' ' || r == '\t' || r == '\n' || e.allowed[r]:
-			results[i].runeType = runeTypeBasic
+			res.runeType = runeTypeBroken
+			res.needEscape = true
+		case e.allowed[r]:
+			res.runeType = runeTypeBasic
 			if r >= 0x80 {
-				results[i].runeType = runeTypeNonASCII
+				res.runeType = runeTypeNonASCII
 			}
 		case r < 0x20 || r == 0x7f:
-			results[i].runeType = runeTypeControlChar
-			results[i].needEscape = true
+			res.runeType = runeTypeControlChar
+			res.needEscape = true
 		case unicode.Is(invisibleRangeTable, r):
-			results[i].runeType = runeTypeInvisible
-			// not sure about results[i].needEscape, will be detected separately
-		case isAmbiguous(r, &confusable, e.ambiguousTables...):
-			results[i].runeType = runeTypeAmbiguous
-			results[i].confusable = confusable
-			// not sure about results[i].needEscape, will be detected separately
-		case r >= 0x80:
-			results[i].runeType = runeTypeNonASCII
-		default: // details to basic runes
+			res.runeType = runeTypeInvisible
+			// not sure about res.needEscape, will be detected separately
+		case isAmbiguous(r, &res.confusable, e.ambiguousTables...):
+			res.runeType = runeTypeAmbiguous
+			// not sure about res.needEscape, will be detected separately
+		default:
+			res.runeType = runeTypeNonASCII
 		}
+		results = append(results, res)
 	}
+	e.detectResults = results
 	return results
 }

modules/charset/escape_test.go

@@ -4,16 +4,15 @@
 package charset
 
 import (
+	"html/template"
 	"strings"
 	"testing"
 
-	"gitea.dev/modules/htmlutil"
 	"gitea.dev/modules/setting"
 	"gitea.dev/modules/test"
 	"gitea.dev/modules/translation"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 type escapeControlTest struct {
@@ -131,9 +130,9 @@ then resh (ר), and finally heh (ה) (which should appear leftmost).`,
 		// UTF-8/16/32 all use the same codepoint for BOM
 		// Gitea could read UTF-16/32 content and convert into UTF-8 internally then render it, so we only process UTF-8 internally
 		name:   "UTF BOM",
-		text:   "\xef\xbb\xbftest",
-		result: "\xef\xbb\xbftest",
-		status: EscapeStatus{},
+		text:   "\xef\xbb\xbftest<b>\xef\xbb\xbftest\xef\xbb\xbf",
+		result: "\xef\xbb\xbftest<b>\xef\xbb\xbftest" + `<span class="escaped-code-point" data-escaped="[U+FEFF]"><span class="char">` + "\xef\xbb\xbf" + `</span></span>`,
+		status: EscapeStatus{Escaped: true, HasInvisible: true},
 	},
 	{
 		name:   "ambiguous",
@@ -141,45 +140,35 @@ then resh (ר), and finally heh (ה) (which should appear leftmost).`,
 		result: `O<span class="ambiguous-code-point" data-tooltip-content="repo.ambiguous_character:𝐾 [U+1D43E],K [U+004B]"><span class="char">𝐾</span></span>`,
 		status: EscapeStatus{Escaped: true, HasAmbiguous: true},
 	},
+	{
+		name:   "long text analyzed in overlapping windows",
+		text:   strings.Repeat("a", 4095) + "\u202e\u0436" + strings.Repeat("a", 5000) + "\u202e",
+		result: strings.Repeat("a", 4095) + "\u202e\u0436" + strings.Repeat("a", 5000) + `<span class="escaped-code-point" data-escaped="[U+202E]"><span class="char">` + "\u202e" + `</span></span>`,
+		status: EscapeStatus{Escaped: true, HasInvisible: true},
+	},
+	{
+		name:   "broken byte before tag",
+		text:   "00\xa9<b>",
+		result: `00<span class="broken-code-point"><3E></span><b>`,
+	},
 	{
 		name:   "ambiguous in math",
-		text:   "<math><mo>−</mo><mi>b</mi></math> −",
-		result: `<math><mo>−</mo><mi>b</mi></math> <span class="ambiguous-code-point" data-tooltip-content="repo.ambiguous_character:− [U+2212],- [U+002D]"><span class="char">−</span></span>`,
+		text:   `<maTH display="block"><mo>−</mo></MAth><mathx>−`,
+		result: `<maTH display="block"><mo>−</mo></MAth><mathx><span class="ambiguous-code-point" data-tooltip-content="repo.ambiguous_character:− [U+2212],- [U+002D]"><span class="char">−</span></span>`,
 		status: EscapeStatus{Escaped: true, HasAmbiguous: true},
 	},
 }
 
-func TestEscapeControlReader(t *testing.T) {
+func TestEscapeControlHTML(t *testing.T) {
 	for _, tt := range escapeControlTests {
 		t.Run(tt.name, func(t *testing.T) {
-			output := &htmlutil.HTMLBuilder{}
-			status, err := EscapeControlReader(strings.NewReader(tt.text), output, &translation.MockLocale{})
-			assert.NoError(t, err)
+			status, output := EscapeControlHTML(template.HTML(tt.text), &translation.MockLocale{})
 			assert.Equal(t, tt.status, *status)
-			outStr := output.String()
-			assert.Equal(t, tt.result, outStr)
+			assert.EqualValues(t, tt.result, output)
 		})
 	}
 }
 
-func TestTrackHtmlTag(t *testing.T) {
-	e := &escapeStreamer{}
-	for _, tt := range []struct {
-		parts  []string
-		inMath bool
-	}{
-		{[]string{"<ma", `TH display="block">`}, true},
-		{[]string{"<mo>"}, true},
-		{[]string{"</MA", "th>"}, false},
-		{[]string{"<mathx>"}, false},
-	} {
-		for _, part := range tt.parts {
-			e.trackHtmlTag([]byte(part))
-		}
-		assert.Equal(t, tt.inMath, e.inTagMath, "%v", tt.parts)
-	}
-}
-
 func TestSettingAmbiguousUnicodeDetection(t *testing.T) {
 	defer test.MockVariableValue(&setting.UI.AmbiguousUnicodeDetection, true)()
 	_, out := EscapeControlHTML("a test", &translation.MockLocale{})
@@ -188,50 +177,3 @@ func TestSettingAmbiguousUnicodeDetection(t *testing.T) {
 	_, out = EscapeControlHTML("a test", &translation.MockLocale{})
 	assert.EqualValues(t, `a test`, out)
 }
-
-func TestHTMLChunkReader(t *testing.T) {
-	type textPart struct {
-		text  string
-		isTag bool
-	}
-	testReadChunks := func(t *testing.T, chunkSize int, input string, expected []textPart) {
-		r := &htmlChunkReader{in: strings.NewReader(input), readBuf: make([]byte, 0, chunkSize)}
-		var results []textPart
-		for {
-			parts, partIsTag, err := r.readRunes()
-			if err != nil {
-				break
-			}
-			for i, part := range parts {
-				results = append(results, textPart{string(part), partIsTag[i]})
-			}
-		}
-		assert.Equal(t, expected, results, "chunk size: %d, input: %s", chunkSize, input)
-	}
-
-	testReadChunks(t, 10, "abc<def>ghi", []textPart{
-		{text: "abc", isTag: false},
-		{text: "<def>", isTag: true},
-		{text: "gh", isTag: false},
-		// -- chunk
-		{text: "i", isTag: false},
-	})
-
-	testReadChunks(t, 10, "<abc><def>ghi", []textPart{
-		{text: "<abc>", isTag: true},
-		{text: "<def>", isTag: true},
-		// -- chunk
-		{text: "ghi", isTag: false},
-	})
-
-	rune1, rune2, rune3, rune4 := "A", "é", "啊", "🌞"
-	require.Len(t, rune1, 1)
-	require.Len(t, rune2, 2)
-	require.Len(t, rune3, 3)
-	require.Len(t, rune4, 4)
-	input := "<" + rune1 + rune2 + rune3 + rune4 + ">" + rune1 + rune2 + rune3 + rune4
-	testReadChunks(t, 4, input, []textPart{{"<Aé", true}, {"啊", true}, {"🌞", true}, {">", true}, {"Aé", false}, {"啊", false}, {"🌞", false}})
-	testReadChunks(t, 5, input, []textPart{{"<Aé", true}, {"啊", true}, {"🌞>", true}, {"Aé", false}, {"啊", false}, {"🌞", false}})
-	testReadChunks(t, 6, input, []textPart{{"<Aé", true}, {"啊", true}, {"🌞>", true}, {"A", false}, {"é啊", false}, {"🌞", false}})
-	testReadChunks(t, 7, input, []textPart{{"<Aé啊", true}, {"🌞>", true}, {"A", false}, {"é啊", false}, {"🌞", false}})
-}

modules/htmlutil/html.go

@@ -140,11 +140,6 @@ func NewHTMLWriter(w io.Writer) HTMLWriter {
 	return &htmlWriter{w: w}
 }
 
-func NewHTMLStringWriter() (*strings.Builder, HTMLWriter) {
-	sb := &strings.Builder{}
-	return sb, &htmlWriter{w: sb}
-}
-
 type HTMLBuilder struct {
 	sb strings.Builder
 }
@@ -157,10 +152,6 @@ func (b *HTMLBuilder) OriginWriter() io.Writer {
 	return &b.sb
 }
 
-func (b *HTMLBuilder) Reset() {
-	b.sb.Reset()
-}
-
 func (b *HTMLBuilder) WriteString(s string) HTMLWriter {
 	b.sb.WriteString(template.HTMLEscapeString(s))
 	return b

modules/markup/html.go

@@ -252,8 +252,7 @@ func postProcess(ctx *RenderContext, procs []processor, input io.Reader, output
 	if !ctx.usedByRender && ctx.RenderHelper != nil {
 		defer ctx.RenderHelper.CleanUp()
 	}
-	// FIXME: don't read all content to memory
-	rawHTML, err := io.ReadAll(input)
+	rawHTML, err := io.ReadAll(input) // no streaming by design, the TOC before the content needs all headings
 	if err != nil {
 		return err
 	}

modules/markup/internal/finalprocessor.go

@@ -23,9 +23,7 @@ func (p *finalProcessor) Write(data []byte) (int, error) {
 }
 
 func (p *finalProcessor) Close() error {
-	// TODO: reading the whole markdown isn't a problem at the moment,
-	// because "postProcess" already does so. In the future we could optimize the code to process data on the fly.
-	buf := p.buf.Bytes()
+	buf := p.buf.Bytes() // no streaming by design, detecting HTML needs the whole content
 	buf = bytes.ReplaceAll(buf, []byte(` data-attr-class="`+p.renderInternal.secureIDPrefix), []byte(` class="`))
 
 	tmp := bytes.TrimSpace(buf)

modules/markup/markdown/markdown.go

@@ -140,8 +140,7 @@ func render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error
 		limit: setting.UI.MaxDisplayFileSize * 3,
 	}
 
-	// FIXME: Don't read all to memory, but goldmark doesn't support
-	buf, err := io.ReadAll(input)
+	buf, err := io.ReadAll(input) // no streaming by design, link reference definitions may follow their use
 	if err != nil {
 		log.Error("Unable to ReadAll: %v", err)
 		return err

routers/web/repo/view.go

@@ -13,6 +13,7 @@ import (
 	"net/http"
 	"net/url"
 	"path"
+	"strings"
 	"time"
 
 	activities_model "gitea.dev/models/activities"
@@ -28,7 +29,6 @@ import (
 	"gitea.dev/modules/charset"
 	"gitea.dev/modules/fileicon"
 	"gitea.dev/modules/git"
-	"gitea.dev/modules/htmlutil"
 	"gitea.dev/modules/lfs"
 	"gitea.dev/modules/log"
 	"gitea.dev/modules/markup"
@@ -152,27 +152,16 @@ func loadLatestCommitData(ctx *context.Context, latestCommit *git.Commit) bool {
 	return true
 }
 
-func markupRenderToHTML(ctx *context.Context, renderCtx *markup.RenderContext, renderer markup.Renderer, input io.Reader) (escaped *charset.EscapeStatus, output template.HTML, err error) {
-	markupRd, markupWr := io.Pipe()
-	defer markupWr.Close()
-
-	done := make(chan struct{})
-	go func() {
-		sb := &htmlutil.HTMLBuilder{}
-		if markup.RendererNeedPostProcess(renderer) {
-			escaped, _ = charset.EscapeControlReader(markupRd, sb, ctx.Locale, charset.EscapeOptionsForView())
-		} else {
-			escaped = &charset.EscapeStatus{}
-			_, _ = io.Copy(sb.OriginWriter(), markupRd)
-		}
-		output = sb.HTMLString()
-		close(done)
-	}()
-
-	err = markup.RenderWithRenderer(renderCtx, renderer, input, markupWr)
-	_ = markupWr.CloseWithError(err)
-	<-done
-	return escaped, output, err
+func markupRenderToHTML(ctx *context.Context, renderCtx *markup.RenderContext, renderer markup.Renderer, input io.Reader) (*charset.EscapeStatus, template.HTML, error) {
+	sb := &strings.Builder{}
+	if err := markup.RenderWithRenderer(renderCtx, renderer, input, sb); err != nil {
+		return nil, "", err
+	}
+	if !markup.RendererNeedPostProcess(renderer) {
+		return &charset.EscapeStatus{}, template.HTML(sb.String()), nil
+	}
+	escaped, output := charset.EscapeControlHTML(template.HTML(sb.String()), ctx.Locale, charset.EscapeOptionsForView())
+	return escaped, output, nil
 }
 
 func checkHomeCodeViewable(ctx *context.Context) {

routers/web/repo/wiki.go

@@ -8,7 +8,6 @@ import (
 	"bytes"
 	gocontext "context"
 	"html/template"
-	"io"
 	"net/http"
 	"net/url"
 	"path"
@@ -20,7 +19,6 @@ import (
 	"gitea.dev/modules/base"
 	"gitea.dev/modules/charset"
 	"gitea.dev/modules/git"
-	"gitea.dev/modules/htmlutil"
 	"gitea.dev/modules/log"
 	"gitea.dev/modules/markup"
 	"gitea.dev/modules/markup/markdown"
@@ -252,22 +250,8 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) {
 
 	rctx := renderhelper.NewRenderContextRepoWiki(ctx, ctx.Repo.Repository)
 
-	renderFn := func(data []byte) (escaped *charset.EscapeStatus, output template.HTML, err error) {
-		buf := &htmlutil.HTMLBuilder{}
-		markupRd, markupWr := io.Pipe()
-		defer markupWr.Close()
-		done := make(chan struct{})
-		go func() {
-			escaped, _ = charset.EscapeControlReader(markupRd, buf, ctx.Locale, charset.EscapeOptionsForView())
-			output = buf.HTMLString()
-			buf.Reset()
-			close(done)
-		}()
-
-		err = markdown.Render(rctx, bytes.NewReader(data), markupWr)
-		_ = markupWr.CloseWithError(err)
-		<-done
-		return escaped, output, err
+	renderFn := func(data []byte) (*charset.EscapeStatus, template.HTML, error) {
+		return markupRenderToHTML(ctx, rctx, markdown.Renderer{}, bytes.NewReader(data))
 	}
 
 	ctx.Data["EscapeStatus"], ctx.Data["WikiContentHTML"], err = renderFn(data)

services/gitdiff/gitdiff.go

@@ -339,12 +339,11 @@ type DiffInlineComputed struct {
 
 // computeDiffInline makes a DiffInline with computed content, e.g.: Unicode escaping, truncation hint, etc
 func computeDiffInline(s template.HTML, isTruncated bool, locale translation.Locale) DiffInlineComputed {
-	sb, w := htmlutil.NewHTMLStringWriter()
-	status, _ := charset.EscapeControlReader(strings.NewReader(string(s)), w, locale)
+	status, content := charset.EscapeControlHTML(s, locale)
 	if isTruncated {
-		w.WriteFormatf(`<span class="ui label diff-line-truncated">%s</span>`, locale.Tr("repo.diff.line_truncated"))
+		content += htmlutil.HTMLFormat(`<span class="ui label diff-line-truncated">%s</span>`, locale.Tr("repo.diff.line_truncated"))
 	}
-	return DiffInlineComputed{EscapeStatus: status, IsTruncated: isTruncated, Content: template.HTML(sb.String())}
+	return DiffInlineComputed{EscapeStatus: status, IsTruncated: isTruncated, Content: content}
 }
 
 func (diffSection *DiffSection) getLineContentForRender(lineIdx int, diffLine *DiffLine, fileLanguage string, highlightLines map[int]template.HTML) template.HTML {