fix: reporter.SetOutputs must ignore values that overflow the size (#662)

Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/662
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>

internal/pkg/report/reporter.go

@@ -5,6 +5,7 @@ package report
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"regexp"
 	"strings"
@@ -21,6 +22,11 @@ import (
 	"runner.forgejo.org/internal/pkg/client"
 )
 
+var (
+	outputKeyMaxLength   = 255
+	outputValueMaxLength = 1024 * 1024
+)
+
 type Reporter struct {
 	ctx    context.Context
 	cancel context.CancelFunc
@@ -199,24 +205,31 @@ func (r *Reporter) logf(format string, a ...interface{}) {
 	}
 }
 
-func (r *Reporter) SetOutputs(outputs map[string]string) {
+func (r *Reporter) SetOutputs(outputs map[string]string) error {
 	r.stateMu.Lock()
 	defer r.stateMu.Unlock()
 
+	var errs []error
+	recordError := func(format string, a ...interface{}) {
+		r.logf(format, a...)
+		errs = append(errs, fmt.Errorf(format, a...))
+	}
 	for k, v := range outputs {
-		if len(k) > 255 {
+		if len(k) > outputKeyMaxLength {
-			r.logf("ignore output because the key is too long: %q", k)
+			recordError("ignore output because the key is longer than %d: %q", outputKeyMaxLength, k)
 			continue
 		}
-		if l := len(v); l > 1024*1024 {
+		if l := len(v); l > outputValueMaxLength {
-			log.Println("ignore output because the value is too long:", k, l)
+			recordError("ignore output because the length of the value for %q is %d (the maximum is %d)", k, l, outputValueMaxLength)
-			r.logf("ignore output because the value %q is too long: %d", k, l)
+			continue
 		}
 		if _, ok := r.outputs.Load(k); ok {
+			recordError("ignore output because a value already exists for the key %q", k)
 			continue
 		}
 		r.outputs.Store(k, v)
 	}
+	return errors.Join(errs...)
 }
 
 func (r *Reporter) Close(lastWords string) error {

internal/pkg/report/reporter_test.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -20,6 +21,7 @@ import (
 	"google.golang.org/protobuf/types/known/structpb"
 
 	"runner.forgejo.org/internal/pkg/client/mocks"
+	"runner.forgejo.org/internal/pkg/testutils"
 )
 
 func rowsToString(rows []*runnerv1.LogRow) string {
@@ -58,6 +60,60 @@ func mockReporter(t *testing.T) (*Reporter, *mocks.Client, func()) {
 	return reporter, client, close
 }
 
+func TestReporterSetOutputs(t *testing.T) {
+	assertEqual := func(t *testing.T, expected map[string]string, actual *sync.Map) {
+		t.Helper()
+		actualMap := map[string]string{}
+		actual.Range(func(k, v interface{}) bool {
+			val, ok := v.(string)
+			require.True(t, ok)
+			actualMap[k.(string)] = val
+			return true
+		})
+		assert.Equal(t, expected, actualMap)
+	}
+
+	t.Run("All", func(t *testing.T) {
+		reporter, _, _ := mockReporter(t)
+
+		expected := map[string]string{"a": "b", "c": "d"}
+		assert.NoError(t, reporter.SetOutputs(expected))
+		assertEqual(t, expected, &reporter.outputs)
+	})
+
+	t.Run("IgnoreTooBig", func(t *testing.T) {
+		reporter, _, _ := mockReporter(t)
+
+		testutils.MockVariable(&outputKeyMaxLength, 5)
+		testutils.MockVariable(&outputValueMaxLength, 5)
+
+		in := map[string]string{
+			"0123456": "b",       // key too big
+			"c":       "ABCDEFG", // value too big
+			"d":       "e",
+		}
+		err := reporter.SetOutputs(in)
+		assert.ErrorContains(t, err, "ignore output because the length of the value for \"c\" is 7 (the maximum is 5)")
+		assert.ErrorContains(t, err, "ignore output because the key is longer than 5: \"0123456\"")
+		expected := map[string]string{"d": "e"}
+		assertEqual(t, expected, &reporter.outputs)
+	})
+
+	t.Run("IgnoreDuplicates", func(t *testing.T) {
+		reporter, _, _ := mockReporter(t)
+
+		first := map[string]string{"a": "b", "c": "d"}
+		assert.NoError(t, reporter.SetOutputs(first))
+		assertEqual(t, first, &reporter.outputs)
+
+		second := map[string]string{"c": "d", "e": "f"}
+		assert.ErrorContains(t, reporter.SetOutputs(second), "ignore output because a value already exists for the key \"c\"")
+
+		expected := map[string]string{"a": "b", "c": "d", "e": "f"}
+		assertEqual(t, expected, &reporter.outputs)
+	})
+}
+
 func TestReporter_parseLogRow(t *testing.T) {
 	tests := []struct {
 		name               string