Skip to content

Commit 49fd4b4

Browse files
committed
Prevent double sanitize.
1 parent 4fa791c commit 49fd4b4

File tree

1 file changed

+47
-59
lines changed

1 file changed

+47
-59
lines changed

modules/markup/markdown/markdown.go

Lines changed: 47 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,8 @@ var urlPrefixKey = parser.NewContextKey()
3636
var isWikiKey = parser.NewContextKey()
3737
var renderMetasKey = parser.NewContextKey()
3838

39-
type closesWithError interface {
40-
io.WriteCloser
41-
CloseWithError(err error) error
42-
}
43-
4439
type limitWriter struct {
45-
w closesWithError
40+
w io.Writer
4641
sum int64
4742
limit int64
4843
}
@@ -56,24 +51,13 @@ func (l *limitWriter) Write(data []byte) (int, error) {
5651
if err != nil {
5752
return n, err
5853
}
59-
_ = l.w.Close()
6054
return n, fmt.Errorf("Rendered content too large - truncating render")
6155
}
6256
n, err := l.w.Write(data)
6357
l.sum += int64(n)
6458
return n, err
6559
}
6660

67-
// Close closes the writer
68-
func (l *limitWriter) Close() error {
69-
return l.w.Close()
70-
}
71-
72-
// CloseWithError closes the writer
73-
func (l *limitWriter) CloseWithError(err error) error {
74-
return l.w.CloseWithError(err)
75-
}
76-
7761
// newParserContext creates a parser.Context with the render context set
7862
func newParserContext(ctx *markup.RenderContext) parser.Context {
7963
pc := parser.NewContext(parser.WithIDs(newPrefixedIDs()))
@@ -159,49 +143,37 @@ func actualRender(ctx *markup.RenderContext, input io.Reader, output io.Writer)
159143

160144
})
161145

162-
rd, wr := io.Pipe()
163-
defer func() {
164-
_ = rd.Close()
165-
_ = wr.Close()
166-
}()
167-
168146
lw := &limitWriter{
169-
w: wr,
147+
w: output,
170148
limit: setting.UI.MaxDisplayFileSize * 3,
171149
}
172150

173-
// FIXME: should we include a timeout that closes the pipe to abort the renderer and sanitizer if it takes too long?
174-
go func() {
175-
defer func() {
176-
err := recover()
177-
if err == nil {
178-
return
179-
}
180-
181-
log.Warn("Unable to render markdown due to panic in goldmark: %v", err)
182-
if log.IsDebug() {
183-
log.Debug("Panic in markdown: %v\n%s", err, string(log.Stack(2)))
184-
}
185-
_ = lw.CloseWithError(fmt.Errorf("%v", err))
186-
}()
187-
188-
// FIXME: Don't read all to memory, but goldmark doesn't support
189-
pc := newParserContext(ctx)
190-
buf, err := ioutil.ReadAll(input)
191-
if err != nil {
192-
log.Error("Unable to ReadAll: %v", err)
151+
// FIXME: should we include a timeout to abort the renderer if it takes too long?
152+
defer func() {
153+
err := recover()
154+
if err == nil {
193155
return
194156
}
195-
if err := converter.Convert(giteautil.NormalizeEOL(buf), lw, parser.WithContext(pc)); err != nil {
196-
log.Error("Unable to render: %v", err)
197-
_ = lw.CloseWithError(err)
198-
return
157+
158+
log.Warn("Unable to render markdown due to panic in goldmark: %v", err)
159+
if log.IsDebug() {
160+
log.Debug("Panic in markdown: %v\n%s", err, string(log.Stack(2)))
199161
}
200-
_ = lw.Close()
201162
}()
202-
buf := markup.SanitizeReader(rd, "")
203-
_, err := io.Copy(output, buf)
204-
return err
163+
164+
// FIXME: Don't read all to memory, but goldmark doesn't support
165+
pc := newParserContext(ctx)
166+
buf, err := ioutil.ReadAll(input)
167+
if err != nil {
168+
log.Error("Unable to ReadAll: %v", err)
169+
return err
170+
}
171+
if err := converter.Convert(giteautil.NormalizeEOL(buf), lw, parser.WithContext(pc)); err != nil {
172+
log.Error("Unable to render: %v", err)
173+
return err
174+
}
175+
176+
return nil
205177
}
206178

207179
func render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
@@ -211,14 +183,13 @@ func render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error
211183
return
212184
}
213185

214-
log.Warn("Unable to render markdown due to panic in goldmark - will return sanitized raw bytes")
186+
log.Warn("Unable to render markdown due to panic in goldmark - will return raw bytes")
215187
if log.IsDebug() {
216188
log.Debug("Panic in markdown: %v\n%s", err, string(log.Stack(2)))
217189
}
218-
ret := markup.SanitizeReader(input, "")
219-
_, err = io.Copy(output, ret)
190+
_, err = io.Copy(output, input)
220191
if err != nil {
221-
log.Error("SanitizeReader failed: %v", err)
192+
log.Error("io.Copy failed: %v", err)
222193
}
223194
}()
224195
return actualRender(ctx, input, output)
@@ -261,8 +232,8 @@ func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Wri
261232

262233
// Render renders Markdown to HTML with all specific handling stuff.
263234
func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
264-
if ctx.Filename == "" {
265-
ctx.Filename = "a.md"
235+
if ctx.Type == "" {
236+
ctx.Type = MarkupName
266237
}
267238
return markup.Render(ctx, input, output)
268239
}
@@ -278,7 +249,24 @@ func RenderString(ctx *markup.RenderContext, content string) (string, error) {
278249

279250
// RenderRaw renders Markdown to HTML without handling special links.
280251
func RenderRaw(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
281-
return render(ctx, input, output)
252+
rd, wr := io.Pipe()
253+
defer func() {
254+
_ = rd.Close()
255+
_ = wr.Close()
256+
}()
257+
258+
go func() {
259+
if err := render(ctx, input, wr); err != nil {
260+
wr.CloseWithError(err)
261+
return
262+
}
263+
_ = wr.Close()
264+
}()
265+
266+
buf := markup.SanitizeReader(rd, "")
267+
_, err := io.Copy(output, buf)
268+
269+
return err
282270
}
283271

284272
// RenderRawString renders Markdown to HTML without handling special links and return string

0 commit comments

Comments
 (0)