Files
Alejandro García MontoroandGitHub 2e7d762a6c MM-69445: Markdown fixes (#37388)
* Cap markdown block nesting depth

blockQuoteStart and listStart recursed once per nested '>' or
list marker on a line, guarded only by an indentation check that
bounds CommonMark indentation, not nesting count. A single
crafted line (e.g. "- " repeated tens of thousands of times)
forced recursion and slice-copy work that grew super-linearly
with nesting depth, letting an authenticated user burn a
disproportionate amount of server CPU per request.

Thread a depth counter through blockStart, blockStartOrParagraph,
blockQuoteStart, and listStart, and stop opening new nested
blocks past maxNestingDepth. This bounds nesting whether it is
built by one line or accrued across many, and caps the recursion
and copy cost to a small constant regardless of input size.

Also apply the existing maxLen byte cap directly inside Parse(),
not just Inspect(), so any caller of Parse is protected
uniformly.

* Add tests for markdown block nesting depth cap

Cover the new maxNestingDepth guard: rejecting a well-formed
block quote/list item once the depth limit is reached, capping
recursion for a single deeply-nested line, and honoring depth
already accrued by open ancestor blocks. Also cover Parse()'s
byte-length cap and add benchmarks showing the nesting-depth
cases now scale roughly linearly with input size instead of
super-linearly.

* Reject oversized posts before markdown parsing

createPost, updatePost, and patchPost ran
PostWithProxyRemovedFromImageURLs, which parses the raw message,
before the message-length check inside Post.IsValid ever ran.
An oversized message paid the full parsing cost even though it
was always going to be rejected.

Add rejectOversizedMessage, mirroring the rune-count check in
Post.IsValid, and call it in all three handlers before any
markdown processing touches the message.

* Add tests for oversized post/patch rejection

Cover the rejectOversizedMessage check added for createPost,
updatePost, and patchPost: a message over MaxPostSize is
rejected with a 400 in all three handlers before it reaches
markdown processing.

* Sync markdown parse cap with real max post size

Parse and Inspect capped input at a hardcoded ~128KB, assuming a
64KB max post size. The real configured limit comes from the
posts.message column and can be raised past that default, so on
installations with an enlarged column, markdown parsing (mentions,
links, formatting) was silently skipped for legitimately-sized
posts.

Have the markdown package pull its cap from a function registered
via SetMaxPostSizeFunc, defaulting to the old conservative constant
only if nothing was ever registered. PlatformService.New registers
PlatformService.MaxPostSize itself once the store is ready, so the
cap always reflects the real limit regardless of which code path
parses markdown first, and the store layer stays unaware that
markdown parsing exists.

* Add tests for markdown max post size sync

Cover SetMaxPostSizeFunc raising the cap and being called on every
MaxLen invocation. Also verify, end to end against a real database,
that PlatformService.New wires markdown.MaxLen to the real
configured max post size.

* Fix tests

* Address review comments

- Simplify setting the maximum number of runes for a post
- Simplify comments
- Rename variables and functions to reflect whether they represent bytes
  or runes.
2026-07-31 17:05:13 +00:00

178 lines
4.3 KiB
Go

// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package markdown
import (
"fmt"
"strings"
"testing"
"github.com/stretchr/testify/assert"
)
func TestInspect(t *testing.T) {
t.Run("base", func(t *testing.T) {
markdown := `
[foo]: bar
- a
> [![]()]()
> [![foo]][foo]
- d
`
visited := []string{}
level := 0
Inspect(markdown, func(blockOrInline any) bool {
if blockOrInline == nil {
level--
} else {
visited = append(visited, strings.Repeat(" ", level*4)+strings.TrimPrefix(fmt.Sprintf("%T", blockOrInline), "*markdown."))
level++
}
return true
})
assert.Equal(t, []string{
"Document",
" Paragraph",
" List",
" ListItem",
" Paragraph",
" Text",
" BlockQuote",
" Paragraph",
" InlineLink",
" InlineImage",
" SoftLineBreak",
" ReferenceLink",
" ReferenceImage",
" Text",
" ListItem",
" Paragraph",
" Text",
}, visited)
})
t.Run("visit nodes when len is smaller than maxLen", func(t *testing.T) {
n := MaxLen() / 5
markdown := strings.Repeat(`![`, n) + strings.Repeat(`]()`, n)
visited := []string{}
level := 0
Inspect(markdown, func(blockOrInline any) bool {
if blockOrInline == nil {
level--
} else {
visited = append(visited, strings.Repeat(" ", level*4)+strings.TrimPrefix(fmt.Sprintf("%T", blockOrInline), "*markdown."))
level++
}
return true
})
assert.NotEmpty(t, visited)
})
t.Run("do not visit any nodes when len is greater than maxLen", func(t *testing.T) {
n := (MaxLen() / 5) + 1
markdown := strings.Repeat(`![`, n) + strings.Repeat(`]()`, n)
visited := []string{}
level := 0
Inspect(markdown, func(blockOrInline any) bool {
if blockOrInline == nil {
level--
} else {
visited = append(visited, strings.Repeat(" ", level*4)+strings.TrimPrefix(fmt.Sprintf("%T", blockOrInline), "*markdown."))
level++
}
return true
})
assert.Empty(t, visited)
})
t.Run("a deeply nested single-line block quote is bounded rather than fully descended", func(t *testing.T) {
n := 20000
markdown := strings.Repeat("> ", n) + "x"
blockQuoteCount := 0
Inspect(markdown, func(blockOrInline any) bool {
if _, ok := blockOrInline.(*BlockQuote); ok {
blockQuoteCount++
}
return true
})
assert.Equal(t, maxNestingDepth-1, blockQuoteCount)
})
t.Run("a deeply nested single-line list is bounded rather than fully descended", func(t *testing.T) {
n := 20000
markdown := strings.Repeat("- ", n) + "x"
listCount := 0
Inspect(markdown, func(blockOrInline any) bool {
if _, ok := blockOrInline.(*List); ok {
listCount++
}
return true
})
assert.Equal(t, maxNestingDepth-1, listCount)
})
}
var counterSink int
func BenchmarkInspect(b *testing.B) {
text := `Some standard piece of text.
Has a link [post](https://github.com) and also has a blockquote.
> This is a famous quote.
Some bold text **Text for markdown?** to go with it.
At the end, some more lines`
for b.Loop() {
Inspect(text, func(_ any) bool {
counterSink++
return true
})
}
}
// BenchmarkInspectNestedBlockQuote and BenchmarkInspectNestedList measure the cost of parsing a
// single line made up of repeated block-quote/list markers. These benchmarks should scale roughly
// linearly with input size (i.e. per-op time should stay flat as the marker count grows), since the
// number of nested blocks actually created is capped at maxNestingDepth regardless of input size.
func BenchmarkInspectNestedBlockQuote(b *testing.B) {
for _, n := range []int{1_000, 10_000, 60_000} {
markdown := strings.Repeat("> ", n) + "x"
b.Run(fmt.Sprintf("n=%d", n), func(b *testing.B) {
for b.Loop() {
Inspect(markdown, func(_ any) bool {
counterSink++
return true
})
}
})
}
}
func BenchmarkInspectNestedList(b *testing.B) {
for _, n := range []int{1_000, 10_000, 60_000} {
markdown := strings.Repeat("- ", n) + "x"
b.Run(fmt.Sprintf("n=%d", n), func(b *testing.B) {
for b.Loop() {
Inspect(markdown, func(_ any) bool {
counterSink++
return true
})
}
})
}
}