diff --git a/modules/charset/ambiguous_gen_test.go b/modules/charset/ambiguous_gen_test.go index 7b5ebc2c294..c7ed2061f82 100644 --- a/modules/charset/ambiguous_gen_test.go +++ b/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 diff --git a/modules/charset/charset.go b/modules/charset/charset.go index 8b902f3c341..dab04685694 100644 --- a/modules/charset/charset.go +++ b/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 diff --git a/modules/charset/escape.go b/modules/charset/escape.go index bdb4b5f309d..356b7e8be05 100644 --- a/modules/charset/escape.go +++ b/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))) } diff --git a/modules/charset/escape_stream.go b/modules/charset/escape_stream.go index df31c3b16cf..34dab69c928 100644 --- a/modules/charset/escape_stream.go +++ b/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("= 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, `�`) - return err + e.out.WriteString(`�`) } -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( ``, @@ -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( ``, @@ -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( ``, @@ -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 } diff --git a/modules/charset/escape_test.go b/modules/charset/escape_test.go index 9961d55bc65..e7e53773129 100644 --- a/modules/charset/escape_test.go +++ b/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\xef\xbb\xbftest\xef\xbb\xbf", + result: "\xef\xbb\xbftest\xef\xbb\xbftest" + `` + "\xef\xbb\xbf" + ``, + status: EscapeStatus{Escaped: true, HasInvisible: true}, }, { name: "ambiguous", @@ -141,45 +140,35 @@ then resh (ר), and finally heh (ה) (which should appear leftmost).`, result: `O𝐾`, 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) + `` + "\u202e" + ``, + status: EscapeStatus{Escaped: true, HasInvisible: true}, + }, + { + name: "broken byte before tag", + text: "00\xa9", + result: `00�`, + }, { name: "ambiguous in math", - text: "−b −", - result: `−b −`, + text: `−−`, + result: `−−`, 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{"`}, true}, - {[]string{""}, true}, - {[]string{""}, false}, - {[]string{""}, 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, "abcghi", []textPart{ - {text: "abc", isTag: false}, - {text: "", isTag: true}, - {text: "gh", isTag: false}, - // -- chunk - {text: "i", isTag: false}, - }) - - testReadChunks(t, 10, "ghi", []textPart{ - {text: "", isTag: true}, - {text: "", 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{{"", true}, {"Aé", false}, {"啊", false}, {"🌞", false}}) - testReadChunks(t, 5, input, []textPart{{"", true}, {"Aé", false}, {"啊", false}, {"🌞", false}}) - testReadChunks(t, 6, input, []textPart{{"", true}, {"A", false}, {"é啊", false}, {"🌞", false}}) - testReadChunks(t, 7, input, []textPart{{"", true}, {"A", false}, {"é啊", false}, {"🌞", false}}) -} diff --git a/modules/htmlutil/html.go b/modules/htmlutil/html.go index 8d7c1350b9c..66980e58b7b 100644 --- a/modules/htmlutil/html.go +++ b/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 diff --git a/modules/markup/html.go b/modules/markup/html.go index 41a8590a5d9..be3f1e64f74 100644 --- a/modules/markup/html.go +++ 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 } diff --git a/modules/markup/internal/finalprocessor.go b/modules/markup/internal/finalprocessor.go index 4442afa0c9e..24c055d64cd 100644 --- a/modules/markup/internal/finalprocessor.go +++ b/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) diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go index 3fc3893d5bd..726d1ec0dd4 100644 --- a/modules/markup/markdown/markdown.go +++ b/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 diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 2dd11ab7ee8..bd2e867746c 100644 --- a/routers/web/repo/view.go +++ b/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) { diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go index d67e7cfb5a7..dd6c514fff3 100644 --- a/routers/web/repo/wiki.go +++ b/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) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index a75d7a9a273..a7b15c1d85d 100644 --- a/services/gitdiff/gitdiff.go +++ b/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(`%s`, locale.Tr("repo.diff.line_truncated")) + content += htmlutil.HTMLFormat(`%s`, 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 {