diff --git a/NOTICE.txt b/NOTICE.txt index 2263b006a4c..3f033ef65fd 100644 --- a/NOTICE.txt +++ b/NOTICE.txt @@ -7892,14 +7892,14 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI --- -## ledongthuc/pdf +## mattermost/pdf -This product contains 'ledongthuc/pdf' by Thuc Le. +This product contains 'mattermost/pdf' by Mattermost, a fork of 'ledongthuc/pdf' by Thuc Le. PDF reader * HOMEPAGE: - * https://github.com/ledongthuc/pdf + * https://github.com/mattermost/pdf * LICENSE: BSD 3-Clause "New" or "Revised" License diff --git a/server/channels/app/file.go b/server/channels/app/file.go index 2d9ab8ff94b..761b5d91543 100644 --- a/server/channels/app/file.go +++ b/server/channels/app/file.go @@ -1767,6 +1767,7 @@ func (a *App) ExtractContentFromFileInfo(rctx request.CTX, fileInfo *model.FileI // detached goroutine after Extract returns, so closing the file here would // race with that goroutine still reading it. text, err := docextractor.Extract(rctx.Logger(), fileInfo.Name, file, docextractor.ExtractSettings{ + Ctx: rctx.Context(), ArchiveRecursion: *a.Config().FileSettings.ArchiveRecursion, MaxFileSize: *a.Config().FileSettings.MaxFileSize, Timeout: time.Duration(*a.Config().FileSettings.ExtractContentTimeout) * time.Second, diff --git a/server/go.mod b/server/go.mod index a36ef1d4c85..6efa9947b1d 100644 --- a/server/go.mod +++ b/server/go.mod @@ -42,7 +42,6 @@ require ( github.com/jaytaylor/html2text v0.0.0-20260303211410-1a4bdc82ecec github.com/jmoiron/sqlx v1.4.0 github.com/klauspost/compress v1.18.6 - github.com/ledongthuc/pdf v0.0.0-20250511090121-5959a4027728 github.com/lib/pq v1.12.3 github.com/mattermost/go-i18n v1.11.1-0.20211013152124-5c415071e404 github.com/mattermost/gosaml2 v0.10.0 @@ -51,6 +50,7 @@ require ( github.com/mattermost/mattermost-plugin-ai v1.14.0 github.com/mattermost/mattermost/server/public v0.4.0 github.com/mattermost/morph v1.1.0 + github.com/mattermost/pdf v0.0.0-20260728101013-cd8a834041c4 github.com/mattermost/rsc v0.0.0-20160330161541-bbaefb05eaa0 github.com/mattermost/squirrel v0.5.0 github.com/mholt/archives v0.1.5 @@ -74,15 +74,15 @@ require ( github.com/tylerb/graceful v1.2.15 github.com/vmihailenco/msgpack/v5 v5.4.1 github.com/wiggin77/merror v1.0.5 - github.com/wneessen/go-mail v0.7.3 + github.com/wneessen/go-mail v0.8.1 github.com/xtgo/uuid v0.0.0-20140804021211-a0b114877d4c github.com/yuin/goldmark v1.8.2 - golang.org/x/crypto v0.53.0 + golang.org/x/crypto v0.54.0 golang.org/x/image v0.44.0 golang.org/x/net v0.56.0 golang.org/x/sync v0.22.0 - golang.org/x/sys v0.46.0 - golang.org/x/term v0.44.0 + golang.org/x/sys v0.47.0 + golang.org/x/term v0.45.0 golang.org/x/text v0.40.0 ) @@ -236,6 +236,3 @@ require ( // See MM-66167, MM-68222 for more details. replace github.com/vmihailenco/msgpack/v5 => github.com/mattermost/msgpack/v5 v5.0.0-20260408165622-cadfad56a815 - -// See MM-63434 for more details. -replace github.com/ledongthuc/pdf => github.com/jgheithcock/pdf v0.0.0-20260404175814-28cd6530c1fe diff --git a/server/go.sum b/server/go.sum index 9c3e4bb9887..a35ec33a6fc 100644 --- a/server/go.sum +++ b/server/go.sum @@ -324,8 +324,6 @@ github.com/jaytaylor/html2text v0.0.0-20180606194806-57d518f124b0/go.mod h1:CVKl github.com/jaytaylor/html2text v0.0.0-20260303211410-1a4bdc82ecec h1:DrV+GDNKHeHyfqEZaoxQoHlWcgTBiaJ8ZUyNyd5vvkY= github.com/jaytaylor/html2text v0.0.0-20260303211410-1a4bdc82ecec/go.mod h1:CVKlgaMiht+LXvHG173ujK6JUhZXKb2u/BQtjPDIvyk= github.com/jellevandenhooff/dkim v0.0.0-20150330215556-f50fe3d243e1/go.mod h1:E0B/fFc00Y+Rasa88328GlI/XbtyysCtTHZS8h7IrBU= -github.com/jgheithcock/pdf v0.0.0-20260404175814-28cd6530c1fe h1:9GAP+hdboArdSUwi82IXaNd+Qq8+cGFQh7xAcwZNN+s= -github.com/jgheithcock/pdf v0.0.0-20260404175814-28cd6530c1fe/go.mod h1:1fEHWurg7pvf5SG6XNE5Q8UZmOwex51Mkx3SLhrW5B4= github.com/jhump/protoreflect v1.17.0 h1:qOEr613fac2lOuTgWN4tPAtLL7fUSbuJL5X5XumQh94= github.com/jhump/protoreflect v1.17.0/go.mod h1:h9+vUUL38jiBzck8ck+6G/aeMX8Z4QUY/NiJPwPNi+8= github.com/jmoiron/sqlx v1.4.0 h1:1PLqN7S1UYp5t4SrVVnt4nUVNemrDAtxlulVe+Qgm3o= @@ -397,6 +395,8 @@ github.com/mattermost/morph v1.1.0 h1:Q9vrJbeM3s2jfweGheq12EFIzdNp9a/6IovcbvOQ6C github.com/mattermost/morph v1.1.0/go.mod h1:gD+EaqX2UMyyuzmF4PFh4r33XneQ8Nzi+0E8nXjMa3A= github.com/mattermost/msgpack/v5 v5.0.0-20260408165622-cadfad56a815 h1:uOi89NvrFmDngqMKjlLDxi+MNzJQLA3TqcU2p8czv34= github.com/mattermost/msgpack/v5 v5.0.0-20260408165622-cadfad56a815/go.mod h1:GaZTsDaehaPpQVyxrf5mtQlH+pc21PIudVV/E3rRQok= +github.com/mattermost/pdf v0.0.0-20260728101013-cd8a834041c4 h1:iFMZLV6k/gOXuQmETk+Ym7FGn5nrBAKlQ0kY/FgK+ro= +github.com/mattermost/pdf v0.0.0-20260728101013-cd8a834041c4/go.mod h1:pNks5J7leEpCnswIJaGfqiz/MQ0lExHgKl+A53aEXrg= github.com/mattermost/rsc v0.0.0-20160330161541-bbaefb05eaa0 h1:G9tL6JXRBMzjuD1kkBtcnd42kUiT6QDwxfFYu7adM6o= github.com/mattermost/rsc v0.0.0-20160330161541-bbaefb05eaa0/go.mod h1:nV5bfVpT//+B1RPD2JvRnxbkLmJEYXmRaaVl15fsXjs= github.com/mattermost/squirrel v0.5.0 h1:81QPS0aA+inQbpA7Pzmv6O9sWwB6VaBh/VYw3oJf8ZY= @@ -660,8 +660,8 @@ github.com/wiggin77/merror v1.0.5 h1:P+lzicsn4vPMycAf2mFf7Zk6G9eco5N+jB1qJ2XW3ME github.com/wiggin77/merror v1.0.5/go.mod h1:H2ETSu7/bPE0Ymf4bEwdUoo73OOEkdClnoRisfw0Nm0= github.com/wiggin77/srslog v1.0.1 h1:gA2XjSMy3DrRdX9UqLuDtuVAAshb8bE1NhX1YK0Qe+8= github.com/wiggin77/srslog v1.0.1/go.mod h1:fehkyYDq1QfuYn60TDPu9YdY2bB85VUW2mvN1WynEls= -github.com/wneessen/go-mail v0.7.3 h1:g3DravXC5SMlVdboFrQA8Jx95A8sOzoBeS5F+vzNRK0= -github.com/wneessen/go-mail v0.7.3/go.mod h1:QGhBX0yNbc1J+Mkjcu7z2rpj4B4l+BmDY8gYznPC9sk= +github.com/wneessen/go-mail v0.8.1 h1:tVcncj02/QySVFw3zr/kXOzZcuFQqBNT6K+Rbgm/pcM= +github.com/wneessen/go-mail v0.8.1/go.mod h1:dWZ61zadzCIyvB4y1/YzC5O7MrbbzBfPkARmbosdf8w= github.com/xtgo/uuid v0.0.0-20140804021211-a0b114877d4c h1:3lbZUMbMiGUW/LMkfsEABsc5zNT9+b1CvsJx47JzJ8g= github.com/xtgo/uuid v0.0.0-20140804021211-a0b114877d4c/go.mod h1:UrdRz5enIKZ63MEE3IF9l2/ebyx59GyGgPi+tICQdmM= github.com/xyproto/randomstring v1.0.5 h1:YtlWPoRdgMu3NZtP45drfy1GKoojuR7hmRcnhZqKjWU= @@ -711,8 +711,8 @@ golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8= golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk= -golang.org/x/crypto v0.53.0 h1:QZ4Muo8THX6CizN2vPPd5fBGHyogrdK9fG4wLPFUsto= -golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsio= +golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw= +golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20260508232706-74f9aab9d74a h1:+3jdDGGB8NGb1Zktc737jlt3/A5f6UlwSzmvqUuufxw= golang.org/x/exp v0.0.0-20260508232706-74f9aab9d74a/go.mod h1:d2fgXJLVs4dYDHUk5lwMIfzRzSrWCfGZb0ZqeLa/Vcw= @@ -814,8 +814,8 @@ golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw= -golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/telemetry v0.0.0-20240228155512-f48c80bd79b2/go.mod h1:TeRTkGYfJXctD9OcfyVLyj2J3IxLnKwHJR8f4D8a3YE= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= @@ -826,8 +826,8 @@ golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U= golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= golang.org/x/term v0.20.0/go.mod h1:8UkIAJTvZgivsXaD6/pH6U9ecQzZ45awqEOzuCvwpFY= golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM= -golang.org/x/term v0.44.0 h1:0rLvDRCtNj0gZkyIXhCyOb2OAzEhLVqc4B+hrsBhrmc= -golang.org/x/term v0.44.0/go.mod h1:7ze4MdzUzLXpSAoFP1H0bOI9aXDqveSvatT5vKcFh2Y= +golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= +golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= diff --git a/server/platform/services/docextractor/archive.go b/server/platform/services/docextractor/archive.go index 64309a2b3de..c66d3d65e28 100644 --- a/server/platform/services/docextractor/archive.go +++ b/server/platform/services/docextractor/archive.go @@ -6,6 +6,7 @@ package docextractor import ( "bytes" "context" + "errors" "fmt" "io" "io/fs" @@ -40,7 +41,7 @@ func getExtAlsoTarGz(name string) string { return filepath.Ext(name) } -func (ae *archiveExtractor) Extract(name string, r io.ReadSeeker, maxFileSize int64) (string, error) { +func (ae *archiveExtractor) Extract(ctx context.Context, name string, r io.ReadSeeker, maxFileSize int64) (string, error) { // MM-65701: Skip 7zip files due to OOM vulnerability in bodgit/sevenzip library match, _ := (archives.SevenZip{}).Match(context.Background(), name, r) _, _ = r.Seek(0, io.SeekStart) // Reset reader position after Match reads from stream @@ -102,9 +103,11 @@ func (ae *archiveExtractor) Extract(name string, r io.ReadSeeker, maxFileSize in return fmt.Errorf("error reading archive entry %s: %w", path, err) } - subtext, extractErr := ae.SubExtractor.Extract(filename, bytes.NewReader(data), maxFileSize) + subtext, extractErr := ae.SubExtractor.Extract(ctx, filename, bytes.NewReader(data), maxFileSize) if extractErr == nil { text.WriteString(subtext + " ") + } else if errors.Is(extractErr, context.Canceled) || errors.Is(extractErr, context.DeadlineExceeded) { + return fmt.Errorf("error extracting %q: %w", filename, extractErr) } } return nil diff --git a/server/platform/services/docextractor/archive_test.go b/server/platform/services/docextractor/archive_test.go index 729f873a47a..b1eecb36c09 100644 --- a/server/platform/services/docextractor/archive_test.go +++ b/server/platform/services/docextractor/archive_test.go @@ -5,6 +5,7 @@ package docextractor import ( "bytes" + "context" "testing" "github.com/stretchr/testify/assert" @@ -17,7 +18,7 @@ func TestArchiveExtractorSkips7zip(t *testing.T) { t.Run("7zip file with .7z extension returns empty string", func(t *testing.T) { // Valid 7zip header (minimal) sevenZipData := []byte{0x37, 0x7a, 0xbc, 0xaf, 0x27, 0x1c, 0x00, 0x00} - result, err := ae.Extract("test.7z", bytes.NewReader(sevenZipData), 0) + result, err := ae.Extract(context.Background(), "test.7z", bytes.NewReader(sevenZipData), 0) require.NoError(t, err) assert.Empty(t, result) }) @@ -25,7 +26,7 @@ func TestArchiveExtractorSkips7zip(t *testing.T) { t.Run("7zip content with wrong extension is still blocked", func(t *testing.T) { // 7zip content disguised with .zip extension - should still be blocked via stream detection sevenZipData := []byte{0x37, 0x7a, 0xbc, 0xaf, 0x27, 0x1c, 0x00, 0x00} - result, err := ae.Extract("malicious.zip", bytes.NewReader(sevenZipData), 0) + result, err := ae.Extract(context.Background(), "malicious.zip", bytes.NewReader(sevenZipData), 0) require.NoError(t, err) assert.Empty(t, result) }) @@ -34,7 +35,7 @@ func TestArchiveExtractorSkips7zip(t *testing.T) { junkPrefix := []byte{0x00, 0x00, 0x00, 0x00} sevenZipSig := []byte{0x37, 0x7a, 0xbc, 0xaf, 0x27, 0x1c, 0x00, 0x00} dataWithOffset := append(junkPrefix, sevenZipSig...) - result, err := ae.Extract("test.7z", bytes.NewReader(dataWithOffset), 0) + result, err := ae.Extract(context.Background(), "test.7z", bytes.NewReader(dataWithOffset), 0) require.NoError(t, err) assert.Empty(t, result) }) @@ -46,7 +47,7 @@ func TestArchiveExtractorSkips7zip(t *testing.T) { junkPrefix := []byte{0x00, 0x00, 0x00, 0x00} sevenZipSig := []byte{0x37, 0x7a, 0xbc, 0xaf, 0x27, 0x1c, 0x00, 0x00} dataWithOffset := append(junkPrefix, sevenZipSig...) - _, err := ae.Extract("malicious.zip", bytes.NewReader(dataWithOffset), 0) + _, err := ae.Extract(context.Background(), "malicious.zip", bytes.NewReader(dataWithOffset), 0) assert.Error(t, err) // fails to extract as any valid archive format }) } diff --git a/server/platform/services/docextractor/combine.go b/server/platform/services/docextractor/combine.go index 277fe5772fe..6b0708d1304 100644 --- a/server/platform/services/docextractor/combine.go +++ b/server/platform/services/docextractor/combine.go @@ -4,6 +4,8 @@ package docextractor import ( + "context" + "errors" "io" "github.com/mattermost/mattermost/server/public/shared/mlog" @@ -31,12 +33,17 @@ func (ce *combineExtractor) Match(filename string) bool { return false } -func (ce *combineExtractor) Extract(filename string, r io.ReadSeeker, maxFileSize int64) (string, error) { +func (ce *combineExtractor) Extract(ctx context.Context, filename string, r io.ReadSeeker, maxFileSize int64) (string, error) { for _, extractor := range ce.SubExtractors { if extractor.Match(filename) { r.Seek(0, io.SeekStart) - text, err := extractor.Extract(filename, r, maxFileSize) + text, err := extractor.Extract(ctx, filename, r, maxFileSize) if err != nil { + // Context errors must not be retried: the caller explicitly + // cancelled or timed out, so stop immediately. + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return "", err + } ce.logger.Warn("Unable to extract file content", mlog.String("file_name", filename), mlog.String("extractor", extractor.Name()), mlog.Err(err)) continue } diff --git a/server/platform/services/docextractor/docextractor.go b/server/platform/services/docextractor/docextractor.go index 4e123decb4e..bdb1fe996e2 100644 --- a/server/platform/services/docextractor/docextractor.go +++ b/server/platform/services/docextractor/docextractor.go @@ -4,6 +4,8 @@ package docextractor import ( + "context" + "errors" "fmt" "io" "time" @@ -13,17 +15,21 @@ import ( // ExtractSettings defines the features enabled/disable during the document text extraction. type ExtractSettings struct { + // Ctx is the parent context for the extraction. Cancellation propagates + // to context-aware extractors (e.g. the Go-native PDF extractor stops at + // the next page boundary). If nil, context.Background() is used. + Ctx context.Context ArchiveRecursion bool MaxFileSize int64 MMPreviewURL string MMPreviewSecret string - // Timeout bounds how long a caller waits for a single extraction. A value - // <= 0 disables it. NOTE: this bounds wall-clock wait time, not CPU work. - // The docconv converters are not context-aware, so on timeout the - // converter keeps running to completion on a detached goroutine. A global - // cap on concurrent extractions (including those detached goroutines) - // prevents sustained uploads from accumulating unbounded docconv work. - // The per-extraction input bound is MaxFileSize. + // Timeout bounds how long a caller waits for a single extraction. When + // set, a child context with this deadline is derived from Ctx and passed + // to the extractor. Context-aware extractors (pdfExtractor) honour it + // and stop at the next page boundary; others run to completion on a + // detached goroutine but no longer hold the caller's worker slot. A + // global cap on concurrent extractions (including those detached + // goroutines) prevents sustained uploads from accumulating unbounded work. Timeout time.Duration // ReaderCloser, when set, transfers ownership of closing the input reader // to this package. It is closed only after extraction has actually @@ -73,13 +79,13 @@ func ExtractWithExtraExtractors(logger mlog.LoggerIFace, filename string, r io.R return "", nil } -// extractWithTimeout runs the extraction and stops waiting for it once -// settings.Timeout elapses. Because the underlying docconv converters are not -// context-aware, the extraction runs on a detached goroutine: on timeout we -// stop waiting and return an error, releasing the caller (and its worker slot) -// even though the converter keeps running. A global concurrency cap applies to -// every in-flight extraction, including detached goroutines, so timed-out work -// cannot accumulate beyond the limit. +// extractWithTimeout runs the extraction under a context derived from +// settings.Ctx. When settings.Timeout > 0 a deadline is added; on expiry the +// caller is released and the extraction runs on a detached goroutine. +// Context-aware extractors (pdfExtractor) honour cancellation at page +// boundaries and stop promptly; others run to completion in the background. +// A global concurrency cap applies to every in-flight extraction, including +// detached goroutines, so timed-out work cannot accumulate beyond the limit. func extractWithTimeout(e Extractor, filename string, r io.ReadSeeker, settings ExtractSettings) (string, error) { if !tryAcquireExtractionSlot() { if settings.ReaderCloser != nil { @@ -88,14 +94,22 @@ func extractWithTimeout(e Extractor, filename string, r io.ReadSeeker, settings return "", fmt.Errorf("document text extraction capacity exhausted (%d concurrent extractions)", maxConcurrentExtractions) } + ctx := settings.Ctx + if ctx == nil { + ctx = context.Background() + } + if settings.Timeout <= 0 { defer releaseExtractionSlot() if settings.ReaderCloser != nil { defer settings.ReaderCloser.Close() } - return e.Extract(filename, r, settings.MaxFileSize) + return e.Extract(ctx, filename, r, settings.MaxFileSize) } + ctx, cancel := context.WithTimeout(ctx, settings.Timeout) + defer cancel() + type extractResult struct { text string err error @@ -119,17 +133,17 @@ func extractWithTimeout(e Extractor, filename string, r io.ReadSeeker, settings resultCh <- extractResult{err: fmt.Errorf("panic during document text extraction: %v", rec)} } }() - text, err := e.Extract(filename, r, settings.MaxFileSize) + text, err := e.Extract(ctx, filename, r, settings.MaxFileSize) resultCh <- extractResult{text: text, err: err} }() - timer := time.NewTimer(settings.Timeout) - defer timer.Stop() - select { case res := <-resultCh: return res.text, res.err - case <-timer.C: - return "", fmt.Errorf("document text extraction timed out after %s", settings.Timeout) + case <-ctx.Done(): + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + return "", fmt.Errorf("document text extraction timed out after %s", settings.Timeout) + } + return "", fmt.Errorf("document text extraction cancelled: %w", ctx.Err()) } } diff --git a/server/platform/services/docextractor/docextractor_test.go b/server/platform/services/docextractor/docextractor_test.go index 66556ba2b29..67809914bbd 100644 --- a/server/platform/services/docextractor/docextractor_test.go +++ b/server/platform/services/docextractor/docextractor_test.go @@ -5,6 +5,7 @@ package docextractor import ( "bytes" + "context" "errors" "io" "runtime" @@ -175,7 +176,7 @@ func (te *customTestPdfExtractor) Match(filename string) bool { return strings.HasSuffix(filename, ".pdf") } -func (te *customTestPdfExtractor) Extract(filename string, r io.ReadSeeker, _ int64) (string, error) { +func (te *customTestPdfExtractor) Extract(_ context.Context, filename string, r io.ReadSeeker, _ int64) (string, error) { return "this is a text generated content", nil } @@ -189,7 +190,7 @@ func (te *failingExtractor) Match(filename string) bool { return true } -func (te *failingExtractor) Extract(filename string, r io.ReadSeeker, _ int64) (string, error) { +func (te *failingExtractor) Extract(_ context.Context, filename string, r io.ReadSeeker, _ int64) (string, error) { return "", errors.New("this always fail") } @@ -214,6 +215,44 @@ func TestExtractWithExtraExtractors(t *testing.T) { assert.Contains(t, text, "document") assert.Contains(t, text, "contains") }) + + t.Run("cancelled context aborts extraction", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + text, err := ExtractWithExtraExtractors( + logger, + "file.txt", + bytes.NewReader([]byte("hello world")), + ExtractSettings{ + Ctx: ctx, + Timeout: time.Second, + }, + []Extractor{&slowExtractor{delay: 10 * time.Second}}) + require.Error(t, err) + require.Empty(t, text) + require.Contains(t, err.Error(), "cancelled") + }) + + // Without context propagation a cancelled context would reach documentExtractor + // (which uses docconv and ignores the context), extract successfully, and return + // no error, silently swallowing the cancellation. + t.Run("cancelled context propagates through combineExtractor", func(t *testing.T) { + data, err := testutils.ReadTestFile("sample-doc.pdf") + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + // Inject pdfExtractor first so it runs before the system documentExtractor. + text, err := ExtractWithExtraExtractors( + logger, + "sample-doc.pdf", + bytes.NewReader(data), + ExtractSettings{Ctx: ctx}, []Extractor{&pdfExtractor{}}) + require.ErrorIs(t, err, context.Canceled) + require.Empty(t, text) + }) } type slowExtractor struct { @@ -225,7 +264,7 @@ func (se *slowExtractor) Name() string { return "slowExtractor" } func (se *slowExtractor) Match(filename string) bool { return true } -func (se *slowExtractor) Extract(filename string, r io.ReadSeeker, _ int64) (string, error) { +func (se *slowExtractor) Extract(_ context.Context, filename string, r io.ReadSeeker, _ int64) (string, error) { defer func() { if se.done != nil { close(se.done) @@ -283,7 +322,7 @@ func (pe *panickingExtractor) Name() string { return "panickingExtractor" } func (pe *panickingExtractor) Match(filename string) bool { return true } -func (pe *panickingExtractor) Extract(filename string, r io.ReadSeeker, _ int64) (string, error) { +func (pe *panickingExtractor) Extract(_ context.Context, filename string, r io.ReadSeeker, _ int64) (string, error) { panic("boom") } @@ -308,7 +347,7 @@ func (be *blockingExtractor) Name() string { return "blockingExtractor" } func (be *blockingExtractor) Match(filename string) bool { return true } -func (be *blockingExtractor) Extract(filename string, r io.ReadSeeker, _ int64) (string, error) { +func (be *blockingExtractor) Extract(_ context.Context, filename string, r io.ReadSeeker, _ int64) (string, error) { close(be.started) <-be.release if be.done != nil { @@ -352,6 +391,45 @@ func TestExtractReaderCloserOwnership(t *testing.T) { }) } +func TestExtractWithTimeout(t *testing.T) { + content, err := testutils.ReadTestFile("sample-doc.pdf") + require.NoError(t, err) + + t.Run("no-timeout path propagates cancelled context", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + text, err := extractWithTimeout( + &pdfExtractor{}, + "sample-doc.pdf", + bytes.NewReader(content), + ExtractSettings{Ctx: ctx}) + require.ErrorIs(t, err, context.Canceled) + require.Empty(t, text) + }) + + // context.WithTimeout is derived from the (already cancelled) ExtractSettings.Ctx, + // so ctx.Done() fires immediately and the select returns a cancellation error + // before extraction can complete. + t.Run("goroutine/select path returns cancellation error", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + text, err := extractWithTimeout(&pdfExtractor{}, + "sample-doc.pdf", + bytes.NewReader(content), + ExtractSettings{ + Ctx: ctx, + Timeout: time.Second, + }) + require.Error(t, err) + require.Empty(t, text) + // Error comes from either the select's ctx.Done() branch ("cancelled") or + // directly from pdfExtractor propagating the cancelled context ("canceled"). + require.Contains(t, strings.ToLower(err.Error()), "cancel") + }) +} + func TestDocumentMaxFileSize(t *testing.T) { logger := mlog.CreateConsoleTestLogger(t) diff --git a/server/platform/services/docextractor/documents.go b/server/platform/services/docextractor/documents.go index 7d2ad90fa26..bee71b6913d 100644 --- a/server/platform/services/docextractor/documents.go +++ b/server/platform/services/docextractor/documents.go @@ -4,6 +4,7 @@ package docextractor import ( + "context" "errors" "io" "path" @@ -38,7 +39,7 @@ func (de *documentExtractor) Match(filename string) bool { return ok } -func (de *documentExtractor) Extract(filename string, r io.ReadSeeker, maxFileSize int64) (out string, outErr error) { +func (de *documentExtractor) Extract(_ context.Context, filename string, r io.ReadSeeker, maxFileSize int64) (out string, outErr error) { defer func() { if r := recover(); r != nil { out = "" diff --git a/server/platform/services/docextractor/interface.go b/server/platform/services/docextractor/interface.go index 9bf55039177..d948f7d5cf2 100644 --- a/server/platform/services/docextractor/interface.go +++ b/server/platform/services/docextractor/interface.go @@ -4,12 +4,13 @@ package docextractor import ( + "context" "io" ) // Extractors define the interface needed to extract file content type Extractor interface { Match(filename string) bool - Extract(filename string, file io.ReadSeeker, maxFileSize int64) (string, error) + Extract(ctx context.Context, filename string, file io.ReadSeeker, maxFileSize int64) (string, error) Name() string } diff --git a/server/platform/services/docextractor/mmpreview.go b/server/platform/services/docextractor/mmpreview.go index 400cc495ff3..62f03b6e93e 100644 --- a/server/platform/services/docextractor/mmpreview.go +++ b/server/platform/services/docextractor/mmpreview.go @@ -9,6 +9,7 @@ package docextractor import ( "bytes" + "context" "io" "mime/multipart" "net/http" @@ -45,12 +46,12 @@ func (mpe *mmPreviewExtractor) Match(filename string) bool { return mmpreviewSupportedExtensions[extension] } -func (mpe *mmPreviewExtractor) Extract(filename string, file io.ReadSeeker, maxFileSize int64) (string, error) { +func (mpe *mmPreviewExtractor) Extract(ctx context.Context, filename string, file io.ReadSeeker, maxFileSize int64) (string, error) { b, w, err := createMultipartFormData("file", filename, file) if err != nil { return "", errors.Wrap(err, "Unable to generate file preview using mmpreview.") } - req, err := http.NewRequest("POST", mpe.url+"/toPDF", &b) + req, err := http.NewRequestWithContext(ctx, "POST", mpe.url+"/toPDF", &b) if err != nil { return "", errors.Wrap(err, "Unable to generate file preview using mmpreview.") } @@ -70,7 +71,7 @@ func (mpe *mmPreviewExtractor) Extract(filename string, file io.ReadSeeker, maxF if err != nil { return "", errors.Wrap(err, "unable to read the response from mmpreview") } - return mpe.pdfExtractor.Extract(filename, bytes.NewReader(data), maxFileSize) + return mpe.pdfExtractor.Extract(ctx, filename, bytes.NewReader(data), maxFileSize) } func createMultipartFormData(fieldName, fileName string, fileData io.ReadSeeker) (bytes.Buffer, *multipart.Writer, error) { diff --git a/server/platform/services/docextractor/pdf.go b/server/platform/services/docextractor/pdf.go index 67deb34bc9f..56167d939f7 100644 --- a/server/platform/services/docextractor/pdf.go +++ b/server/platform/services/docextractor/pdf.go @@ -5,6 +5,7 @@ package docextractor import ( "bytes" + "context" "errors" "fmt" "io" @@ -12,7 +13,7 @@ import ( "path" "strings" - "github.com/ledongthuc/pdf" + "github.com/mattermost/pdf" "github.com/mattermost/mattermost/server/v8/channels/utils" ) @@ -31,7 +32,7 @@ func (pe *pdfExtractor) Match(filename string) bool { return supportedExtensions[extension] } -func (pe *pdfExtractor) Extract(filename string, r io.ReadSeeker, maxFileSize int64) (out string, outErr error) { +func (pe *pdfExtractor) Extract(ctx context.Context, filename string, r io.ReadSeeker, maxFileSize int64) (out string, outErr error) { defer func() { if r := recover(); r != nil { out = "" @@ -62,7 +63,7 @@ func (pe *pdfExtractor) Extract(filename string, r io.ReadSeeker, maxFileSize in } var buf bytes.Buffer - b, err := reader.GetPlainText() + b, err := reader.GetPlainText(ctx) if err != nil { return "", err } diff --git a/server/platform/services/docextractor/pdf_test.go b/server/platform/services/docextractor/pdf_test.go index 5f5ca7e8c73..1b91912087d 100644 --- a/server/platform/services/docextractor/pdf_test.go +++ b/server/platform/services/docextractor/pdf_test.go @@ -5,6 +5,7 @@ package docextractor import ( "bytes" + "context" "testing" "github.com/stretchr/testify/require" @@ -14,7 +15,7 @@ import ( func TestPdfEmptyFile(t *testing.T) { extractor := pdfExtractor{} - _, err := extractor.Extract("test.pdf", bytes.NewReader([]byte{}), 0) + _, err := extractor.Extract(context.Background(), "test.pdf", bytes.NewReader([]byte{}), 0) require.Error(t, err) } @@ -23,7 +24,7 @@ func TestPdfFile(t *testing.T) { contentText := "\nThis is a simple document that contains some text." content, err := testutils.ReadTestFile("sample-doc.pdf") require.NoError(t, err) - extractedText, err := extractor.Extract("sample-doc.pdf", bytes.NewReader(content), 0) + extractedText, err := extractor.Extract(context.Background(), "sample-doc.pdf", bytes.NewReader(content), 0) require.NoError(t, err) require.Equal(t, contentText, extractedText) } @@ -38,7 +39,7 @@ func TestPdfDeeplyNestedObjects(t *testing.T) { buf.WriteString("startxref\n0\n%%EOF\n") extractor := pdfExtractor{} - text, err := extractor.Extract("excessive-nests.pdf", bytes.NewReader(buf.Bytes()), 0) + text, err := extractor.Extract(context.Background(), "excessive-nests.pdf", bytes.NewReader(buf.Bytes()), 0) require.Error(t, err) require.Empty(t, text) } @@ -47,10 +48,22 @@ func TestWrongPdfFile(t *testing.T) { extractor := pdfExtractor{} content, err := testutils.ReadTestFile("sample-doc.docx") require.NoError(t, err) - _, err = extractor.Extract("sample-doc.pdf", bytes.NewReader(content), 0) + _, err = extractor.Extract(context.Background(), "sample-doc.pdf", bytes.NewReader(content), 0) require.Error(t, err) } +func TestPdfCancelledContext(t *testing.T) { + extractor := pdfExtractor{} + content, err := testutils.ReadTestFile("sample-doc.pdf") + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err = extractor.Extract(ctx, "sample-doc.pdf", bytes.NewReader(content), 0) + require.ErrorIs(t, err, context.Canceled) +} + func TestPdfMaxFileSize(t *testing.T) { extractor := pdfExtractor{} content, err := testutils.ReadTestFile("sample-doc.pdf") @@ -58,13 +71,13 @@ func TestPdfMaxFileSize(t *testing.T) { require.Greater(t, len(content), 16, "fixture must be larger than the tight limit under test") t.Run("a zero limit means unlimited and extracts the content", func(t *testing.T) { - text, err := extractor.Extract("sample-doc.pdf", bytes.NewReader(content), 0) + text, err := extractor.Extract(context.Background(), "sample-doc.pdf", bytes.NewReader(content), 0) require.NoError(t, err) require.Contains(t, text, "simple") }) t.Run("a generous limit extracts the content", func(t *testing.T) { - text, err := extractor.Extract("sample-doc.pdf", bytes.NewReader(content), 10*1024*1024) + text, err := extractor.Extract(context.Background(), "sample-doc.pdf", bytes.NewReader(content), 10*1024*1024) require.NoError(t, err) require.Contains(t, text, "simple") }) @@ -72,7 +85,7 @@ func TestPdfMaxFileSize(t *testing.T) { t.Run("a tight limit prevents extraction", func(t *testing.T) { // The reader errors once it reads past the limit, so io.Copy to the // temp file fails and no text is extracted. - text, err := extractor.Extract("sample-doc.pdf", bytes.NewReader(content), 16) + text, err := extractor.Extract(context.Background(), "sample-doc.pdf", bytes.NewReader(content), 16) require.Error(t, err) require.Empty(t, text) }) diff --git a/server/platform/services/docextractor/plain.go b/server/platform/services/docextractor/plain.go index b6fff6bd69d..8489ce97d7e 100644 --- a/server/platform/services/docextractor/plain.go +++ b/server/platform/services/docextractor/plain.go @@ -4,6 +4,7 @@ package docextractor import ( + "context" "io" "unicode" "unicode/utf8" @@ -19,7 +20,7 @@ func (pe *plainExtractor) Match(filename string) bool { return true } -func (pe *plainExtractor) Extract(filename string, r io.ReadSeeker, _ int64) (string, error) { +func (pe *plainExtractor) Extract(_ context.Context, filename string, r io.ReadSeeker, _ int64) (string, error) { // This detects any visible character plus any whitespace validRanges := append(unicode.GraphicRanges, unicode.White_Space) diff --git a/server/platform/services/docextractor/plain_test.go b/server/platform/services/docextractor/plain_test.go index 8e1865e525d..1b3094a6359 100644 --- a/server/platform/services/docextractor/plain_test.go +++ b/server/platform/services/docextractor/plain_test.go @@ -5,6 +5,7 @@ package docextractor import ( "bytes" + "context" "strings" "testing" @@ -13,7 +14,7 @@ import ( func TestPlainEmptyFile(t *testing.T) { extractor := plainExtractor{} - extractedText, err := extractor.Extract("test.txt", bytes.NewReader([]byte{}), 0) + extractedText, err := extractor.Extract(context.Background(), "test.txt", bytes.NewReader([]byte{}), 0) require.NoError(t, err) require.Equal(t, "", extractedText) } @@ -21,7 +22,7 @@ func TestPlainEmptyFile(t *testing.T) { func TestPlainTextSmallFile(t *testing.T) { extractor := plainExtractor{} content := strings.Repeat("test \n", 5) - extractedText, err := extractor.Extract("test.txt", bytes.NewReader([]byte(content)), 0) + extractedText, err := extractor.Extract(context.Background(), "test.txt", bytes.NewReader([]byte(content)), 0) require.NoError(t, err) require.Equal(t, content, extractedText) } @@ -29,7 +30,7 @@ func TestPlainTextSmallFile(t *testing.T) { func TestPlainBigFile(t *testing.T) { extractor := plainExtractor{} content := strings.Repeat("test \n", 1000) - extractedText, err := extractor.Extract("test.txt", bytes.NewReader([]byte(content)), 0) + extractedText, err := extractor.Extract(context.Background(), "test.txt", bytes.NewReader([]byte(content)), 0) require.NoError(t, err) require.Equal(t, content, extractedText) } @@ -38,7 +39,7 @@ func TestSmallBinaryFile(t *testing.T) { extractor := plainExtractor{} notUTF8Char := byte(0x7) content := bytes.Repeat([]byte{notUTF8Char}, 1000) - extractedText, err := extractor.Extract("test.bin", bytes.NewReader(content), 0) + extractedText, err := extractor.Extract(context.Background(), "test.bin", bytes.NewReader(content), 0) require.NoError(t, err) require.Equal(t, "", extractedText) } @@ -47,7 +48,7 @@ func TestBigBinaryFile(t *testing.T) { extractor := plainExtractor{} notUTF8Char := byte(0x7) content := bytes.Repeat([]byte{notUTF8Char}, 10000) - extractedText, err := extractor.Extract("test.bin", bytes.NewReader(content), 0) + extractedText, err := extractor.Extract(context.Background(), "test.bin", bytes.NewReader(content), 0) require.NoError(t, err) require.Equal(t, "", extractedText) }