From a4fb3cea360b3ad39aa542459faa380da08c685e Mon Sep 17 00:00:00 2001 From: Johnathan Fercher Date: Mon, 11 May 2026 18:43:45 -0300 Subject: [PATCH 1/2] fix: clone custom font bytes per gofpdf provider Under concurrent generation mode each worker builds its own gofpdf provider, but they were sharing the same underlying byte slice from the configured custom fonts. gofpdf mutates this slice inside putfonts / GenerateCutFont (specifically generateChecksum), which races when two workers are finalising documents at the same time. Clone the font bytes when calling AddUTF8FontFromBytes so every provider owns an independent backing array, eliminating the write/write and write/read data races reported by the Go race detector. Closes #550. --- internal/providers/gofpdf/builder.go | 8 ++- .../providers/gofpdf/builder_internal_test.go | 51 ++++++++++++++++++ maroto_test.go | 54 +++++++++++++++++++ 3 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 internal/providers/gofpdf/builder_internal_test.go diff --git a/internal/providers/gofpdf/builder.go b/internal/providers/gofpdf/builder.go index 065a5d4a..ee8f7fde 100644 --- a/internal/providers/gofpdf/builder.go +++ b/internal/providers/gofpdf/builder.go @@ -1,6 +1,8 @@ package gofpdf import ( + "bytes" + "github.com/phpdave11/gofpdf" "github.com/johnfercher/maroto/v2/internal/cache" @@ -51,7 +53,11 @@ func (b *builder) Build(cfg *entity.Config, cache cache.Cache) *Dependencies { }) for _, font := range cfg.CustomFonts { - fpdf.AddUTF8FontFromBytes(font.GetFamily(), string(font.GetStyle()), font.GetBytes()) + // Clone the font bytes so each provider instance owns its own + // underlying buffer. gofpdf mutates the slice in putfonts / + // GenerateCutFont, which races when multiple providers share the + // same backing array in concurrent generation mode. See issue #550. + fpdf.AddUTF8FontFromBytes(font.GetFamily(), string(font.GetStyle()), bytes.Clone(font.GetBytes())) } if cfg.DisableAutoPageBreak { diff --git a/internal/providers/gofpdf/builder_internal_test.go b/internal/providers/gofpdf/builder_internal_test.go new file mode 100644 index 00000000..a4a36abc --- /dev/null +++ b/internal/providers/gofpdf/builder_internal_test.go @@ -0,0 +1,51 @@ +// Regression test for issue #550: custom font bytes passed to gofpdf +// must be cloned so each provider instance owns its own backing array. +// gofpdf mutates the slice during PDF finalization (putfonts / +// GenerateCutFont), which previously caused a data race when multiple +// providers (one per worker in concurrent mode) shared the same bytes. +// +// This guards against a regression by confirming that calling Build +// does not mutate the caller's font bytes and that building twice +// produces providers that do not share the same backing array. +func TestBuilder_Build_DoesNotShareCustomFontBytes(t *testing.T) { + t.Parallel() + // Arrange + sut := gofpdf.NewBuilder() + font := fixture.FontProp() + ttf, err := os.ReadFile(buildPath("docs/assets/fonts/arial-unicode-ms.ttf")) + require.NoError(t, err) + snapshot := append([]byte(nil), ttf...) + + cfg := &entity.Config{ + Dimensions: &entity.Dimensions{Width: 100, Height: 200}, + Margins: &entity.Margins{Left: 10, Top: 10, Right: 10, Bottom: 10}, + DefaultFont: &font, + CustomFonts: []entity.CustomFont{ + fixture.TestFont{ + Family: fontfamily.Arial, + Style: fontstyle.Normal, + Bytes: ttf, + }, + }, + } + + // Act — build twice, simulating two concurrent workers sharing the + // same CustomFonts entry. + dep1 := sut.Build(cfg, nil) + dep2 := sut.Build(cfg, nil) + + // Assert + assert.NotNil(t, dep1) + assert.NotNil(t, dep2) + // The caller's bytes must be untouched after Build. + assert.Equal(t, snapshot, ttf) +} + +func buildPath(file string) string { + dir, err := os.Getwd() + if err != nil { + return "" + } + dir = strings.ReplaceAll(dir, "internal/providers/gofpdf", "") + return path.Join(dir, file) +} diff --git a/maroto_test.go b/maroto_test.go index 259c8804..b533cf87 100644 --- a/maroto_test.go +++ b/maroto_test.go @@ -2,6 +2,8 @@ package maroto_test import ( "fmt" + "os" + "path" "runtime" "testing" "time" @@ -13,12 +15,16 @@ import ( "github.com/johnfercher/maroto/v2/pkg/components/page" "github.com/johnfercher/maroto/v2/pkg/components/row" "github.com/johnfercher/maroto/v2/pkg/config" + "github.com/johnfercher/maroto/v2/pkg/consts/fontstyle" "github.com/johnfercher/maroto/v2/pkg/core" + "github.com/johnfercher/maroto/v2/pkg/fontrepository" + "github.com/johnfercher/maroto/v2/pkg/props" "github.com/johnfercher/maroto/v2/pkg/test" "github.com/johnfercher/maroto/v2" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNew(t *testing.T) { @@ -442,6 +448,42 @@ func TestMaroto_Generate(t *testing.T) { // Assert test.New(t).Assert(sut.GetStructure()).Equals("maroto_page_number.json") }) + // Regression test for https://github.com/johnfercher/maroto/issues/550. + // Under concurrent mode each worker creates its own gofpdf provider, + // but gofpdf mutates the custom-font byte slice in putfonts / + // GenerateCutFont. If the same backing array is shared across + // providers, `go test -race` flags a data race. The builder clones + // the font bytes per provider to fix it; this test guards against + // regressions. + t.Run("when concurrent mode is active with a custom font, should not race", func(t *testing.T) { + // Arrange + ttf, err := os.ReadFile(buildPath("docs/assets/fonts/arial-unicode-ms.ttf")) + require.NoError(t, err) + + customFonts, err := fontrepository.New(). + AddUTF8FontFromBytes("custom", fontstyle.Normal, ttf). + AddUTF8FontFromBytes("custom", fontstyle.Bold, ttf). + Load() + require.NoError(t, err) + + cfg := config.NewBuilder(). + WithCustomFonts(customFonts). + WithDefaultFont(&props.Font{Family: "custom"}). + WithConcurrentMode(4). + Build() + + sut := maroto.New(cfg) + for i := 0; i < 120; i++ { + sut.AddRow(10, col.New(12)) + } + + // Act + doc, err := sut.Generate() + + // Assert + assert.NoError(t, err) + assert.NotNil(t, doc) + }) } func TestMaroto_FitlnCurrentPage(t *testing.T) { @@ -598,3 +640,15 @@ func TestMaroto_RegisterFooter(t *testing.T) { test.New(t).Assert(sut.GetStructure()).Equals("footer_auto_row.json") }) } + +// buildPath converts a path relative to the repo root into an absolute +// path, so tests can read fixture files (such as custom TTF fonts) +// regardless of where `go test` is invoked from. maroto_test.go lives at +// the repo root, so os.Getwd() already returns the root. +func buildPath(file string) string { + dir, err := os.Getwd() + if err != nil { + return "" + } + return path.Join(dir, file) +} From 47c9ae49036e244c6bbf7cd9ef339c25c25ef4fb Mon Sep 17 00:00:00 2001 From: Johnathan Fercher Date: Mon, 11 May 2026 18:54:26 -0300 Subject: [PATCH 2/2] test: fix broken regression test for issue #550 The previous commit introduced internal/providers/gofpdf/builder_internal_test.go without a package declaration or imports, which broke the gofpdf test build. Restore the file so it compiles as an external (gofpdf_test) package test and keeps guarding against the regression where Build would mutate the caller's custom-font byte slice. --- .../providers/gofpdf/builder_internal_test.go | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/internal/providers/gofpdf/builder_internal_test.go b/internal/providers/gofpdf/builder_internal_test.go index a4a36abc..1c7392da 100644 --- a/internal/providers/gofpdf/builder_internal_test.go +++ b/internal/providers/gofpdf/builder_internal_test.go @@ -1,3 +1,21 @@ +package gofpdf_test + +import ( + "os" + "path" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/johnfercher/maroto/v2/internal/fixture" + "github.com/johnfercher/maroto/v2/internal/providers/gofpdf" + "github.com/johnfercher/maroto/v2/pkg/consts/fontfamily" + "github.com/johnfercher/maroto/v2/pkg/consts/fontstyle" + "github.com/johnfercher/maroto/v2/pkg/core/entity" +) + // Regression test for issue #550: custom font bytes passed to gofpdf // must be cloned so each provider instance owns its own backing array. // gofpdf mutates the slice during PDF finalization (putfonts / @@ -5,8 +23,7 @@ // providers (one per worker in concurrent mode) shared the same bytes. // // This guards against a regression by confirming that calling Build -// does not mutate the caller's font bytes and that building twice -// produces providers that do not share the same backing array. +// does not mutate the caller's font bytes. func TestBuilder_Build_DoesNotShareCustomFontBytes(t *testing.T) { t.Parallel() // Arrange @@ -17,8 +34,8 @@ func TestBuilder_Build_DoesNotShareCustomFontBytes(t *testing.T) { snapshot := append([]byte(nil), ttf...) cfg := &entity.Config{ - Dimensions: &entity.Dimensions{Width: 100, Height: 200}, - Margins: &entity.Margins{Left: 10, Top: 10, Right: 10, Bottom: 10}, + Dimensions: &entity.Dimensions{Width: 100, Height: 200}, + Margins: &entity.Margins{Left: 10, Top: 10, Right: 10, Bottom: 10}, DefaultFont: &font, CustomFonts: []entity.CustomFont{ fixture.TestFont{