fix: markup rendering panic must not abort the process (#9478)

It must return on error instead, and log a stack trace for forensic analysis.

Refs https://codeberg.org/forgejo/forgejo/issues/9472

## Checklist

The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).

### Tests

- I added test coverage for Go changes...
  - [x] in their respective `*_test.go` for unit tests.
  - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
  - [ ] in `web_src/js/*.test.js` if it can be unit tested.
  - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).

### Documentation

- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.

### Release notes

- [x] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9478
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>

modules/markup/renderer.go

@@ -17,6 +17,7 @@ import (
 	"forgejo.org/modules/git"
 	"forgejo.org/modules/setting"
 	"forgejo.org/modules/util"
+	"forgejo.org/modules/util/donotpanic"
 
 	"github.com/yuin/goldmark/ast"
 )
@@ -267,6 +268,15 @@ sandbox="allow-scripts"
 	return err
 }
 
+func postProcessOrCopy(ctx *RenderContext, renderer Renderer, reader io.Reader, writer io.Writer) (err error) {
+	if r, ok := renderer.(PostProcessRenderer); ok && r.NeedPostProcess() {
+		err = PostProcess(ctx, reader, writer)
+	} else {
+		_, err = io.Copy(writer, reader)
+	}
+	return err
+}
+
 func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error {
 	var wg sync.WaitGroup
 	var err error
@@ -293,7 +303,7 @@ func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Wr
 
 		wg.Add(1)
 		go func() {
-			err = SanitizeReader(pr2, renderer.Name(), output)
+			err = donotpanic.SafeFuncWithError(func() error { return SanitizeReader(pr2, renderer.Name(), output) })
 			_ = pr2.Close()
 			wg.Done()
 		}()
@@ -303,11 +313,7 @@ func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Wr
 
 	wg.Add(1)
 	go func() {
-		if r, ok := renderer.(PostProcessRenderer); ok && r.NeedPostProcess() {
-			err = PostProcess(ctx, pr, pw2)
-		} else {
-			_, err = io.Copy(pw2, pr)
-		}
+		err = donotpanic.SafeFuncWithError(func() error { return postProcessOrCopy(ctx, renderer, pr, pw2) })
 		_ = pr.Close()
 		_ = pw2.Close()
 		wg.Done()

modules/markup/renderer_test.go

@@ -1,4 +1,49 @@
-// Copyright 2017 The Gitea Authors. All rights reserved.
-// SPDX-License-Identifier: MIT
+// Copyright 2025 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
 
-package markup_test
+package markup
+
+import (
+	"bytes"
+	"errors"
+	"strings"
+	"testing"
+
+	"forgejo.org/modules/test"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+type failReader struct{}
+
+func (*failReader) Read(p []byte) (n int, err error) {
+	return 0, errors.New("FAIL")
+}
+
+func TestRender_postProcessOrCopy(t *testing.T) {
+	renderContext := &RenderContext{Ctx: t.Context()}
+
+	t.Run("CopyOK", func(t *testing.T) {
+		input := "SOMETHING"
+		output := &bytes.Buffer{}
+		require.NoError(t, postProcessOrCopy(renderContext, nil, strings.NewReader(input), output))
+		assert.Equal(t, input, output.String())
+	})
+
+	renderer := GetRendererByType("markdown")
+
+	t.Run("PostProcessOK", func(t *testing.T) {
+		input := "SOMETHING"
+		output := &bytes.Buffer{}
+		defer test.MockVariableValue(&defaultProcessors, []processor{})()
+		require.NoError(t, postProcessOrCopy(renderContext, renderer, strings.NewReader(input), output))
+		assert.Equal(t, input, output.String())
+	})
+
+	t.Run("PostProcessError", func(t *testing.T) {
+		input := &failReader{}
+		defer test.MockVariableValue(&defaultProcessors, []processor{})()
+		assert.ErrorContains(t, postProcessOrCopy(renderContext, renderer, input, &bytes.Buffer{}), "FAIL")
+	})
+}

modules/util/donotpanic/donotpanic.go

@@ -0,0 +1,28 @@
+// Copyright 2025 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package donotpanic
+
+import (
+	"fmt"
+
+	"forgejo.org/modules/log"
+)
+
+type FuncWithError func() error
+
+func SafeFuncWithError(fun FuncWithError) (err error) {
+	defer func() {
+		if r := recover(); r != nil {
+			log.Error("PANIC recovered: %v\nStacktrace: %s", r, log.Stack(2))
+			rErr, ok := r.(error)
+			if ok {
+				err = fmt.Errorf("PANIC recover with error: %w", rErr)
+			} else {
+				err = fmt.Errorf("PANIC recover: %v", r)
+			}
+		}
+	}()
+
+	return fun()
+}

modules/util/donotpanic/donotpanic_test.go

@@ -0,0 +1,28 @@
+// Copyright 2025 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package donotpanic
+
+import (
+	"errors"
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDoNotPanic_SafeFuncWithError(t *testing.T) {
+	t.Run("OK", func(t *testing.T) {
+		assert.NoError(t, SafeFuncWithError(func() error { return nil }))
+	})
+
+	t.Run("PanickString", func(t *testing.T) {
+		errorMessage := "ERROR MESSAGE"
+		assert.ErrorContains(t, SafeFuncWithError(func() error { panic(errorMessage) }), fmt.Sprintf("recover: %s", errorMessage))
+	})
+
+	t.Run("PanickError", func(t *testing.T) {
+		errorMessage := "ERROR MESSAGE"
+		assert.ErrorContains(t, SafeFuncWithError(func() error { panic(errors.New(errorMessage)) }), fmt.Sprintf("recover with error: %s", errorMessage))
+	})
+}