IntenseWebs/mattermost
3
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2026-08-08 20:18:09 -05:00
Code Issues Packages Projects Releases Wiki Activity

MM-69725: Propagate context through Extractor.Extract using new mattermost/pdf fork (#37579)

Browse Source 
* Propagate context through Extractor.Extract

Adds ctx context.Context to the Extractor interface and all
implementations so callers can cancel in-flight extractions.

pdfExtractor passes the context down to GetPlainText(ctx), which
checks ctx.Done() at each page boundary. When extractWithTimeout
fires (deadline exceeded or parent cancel), it cancels the derived
context via defer cancel(), and the PDF goroutine stops at the next
page rather than running to completion, eliminating the heap
growth that caused OOM on complex documents.

For non-context-aware extractors (docconv, plain, archive) the
context is accepted but not yet acted on; they continue to run
to completion on a detached goroutine after the timeout, as before.

The call site in channels/app/file.go passes rctx.Context() so
request cancellation (e.g. HTTP disconnect) also propagates.

* Use new mattermost/pdf fork

* Update notice reflecting the new fork

* Fail archiveExtractor.Extract early on ctx errors

* Bump mattermost/pdf to cd8a834041c4

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Alejandro García Montoro
2026-07-28 17:29:36 +02:00
committed by GitHub
co-authored by Mattermost Build
parent 6fefcc80a4
commit 9230eb6f6a
16 changed files with 196 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files
NOTICE.txt
+3 -3
View File
@@ -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
server/channels/app/file.go
+1
View File
@@ -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,
server/go.mod
+5 -8
View File
@@ -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
server/go.sum
+10 -10
View File
@@ -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=
server/platform/services/docextractor/archive.go
+5 -2
View File
@@ -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
server/platform/services/docextractor/archive_test.go
+5 -4
View File
@@ -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
})
}
server/platform/services/docextractor/combine.go
+9 -2
View File
@@ -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
}
server/platform/services/docextractor/docextractor.go
+35 -21
View File
@@ -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())
}
}
server/platform/services/docextractor/docextractor_test.go
+83 -5
View File
@@ -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)
server/platform/services/docextractor/documents.go
+2 -1
View File
@@ -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 = ""
server/platform/services/docextractor/interface.go
+2 -1
View File
@@ -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
}
server/platform/services/docextractor/mmpreview.go
+4 -3
View File
@@ -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) {
server/platform/services/docextractor/pdf.go
+4 -3
View File
@@ -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
}
server/platform/services/docextractor/pdf_test.go
+20 -7
View File
@@ -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)
})
server/platform/services/docextractor/plain.go
+2 -1
View File
@@ -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)
server/platform/services/docextractor/plain_test.go
+6 -5
View File
@@ -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)
}