From 130b94aa8cd5644d4cf6b94e147e7477779778f4 Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Tue, 5 Dec 2023 16:34:35 +0100 Subject: [PATCH] fix: special characters issues with filenames (#736) --- go.mod | 12 +-- go.sum | 24 +++--- pkg/modules/api/context.go | 16 +--- pkg/modules/libreoffice/api/libreoffice.go | 67 +++++++++++++++ .../libreoffice/api/libreoffice_test.go | 86 ++++++++++++++++++- 5 files changed, 174 insertions(+), 31 deletions(-) diff --git a/go.mod b/go.mod index 3f5b28db..39863123 100644 --- a/go.mod +++ b/go.mod @@ -5,13 +5,13 @@ go 1.21 require ( github.com/alexliesenfeld/health v0.8.0 github.com/andybalholm/brotli v1.0.6 // indirect - github.com/chromedp/cdproto v0.0.0-20231114014204-3e458d5176f9 + github.com/chromedp/cdproto v0.0.0-20231205062650-00455a960d61 github.com/chromedp/chromedp v0.9.3 github.com/golang/snappy v0.0.4 // indirect github.com/google/uuid v1.4.0 github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/hashicorp/go-retryablehttp v0.7.5 - github.com/klauspost/compress v1.17.3 // indirect + github.com/klauspost/compress v1.17.4 // indirect github.com/klauspost/pgzip v1.2.6 // indirect github.com/labstack/echo/v4 v4.11.3 github.com/labstack/gommon v0.4.1 @@ -27,12 +27,12 @@ require ( github.com/ulikunitz/xz v0.5.11 // indirect go.uber.org/multierr v1.11.0 go.uber.org/zap v1.26.0 - golang.org/x/crypto v0.15.0 // indirect + golang.org/x/crypto v0.16.0 // indirect golang.org/x/image v0.14.0 // indirect - golang.org/x/net v0.18.0 + golang.org/x/net v0.19.0 golang.org/x/sync v0.5.0 - golang.org/x/sys v0.14.0 // indirect - golang.org/x/term v0.14.0 + golang.org/x/sys v0.15.0 // indirect + golang.org/x/term v0.15.0 golang.org/x/text v0.14.0 ) diff --git a/go.sum b/go.sum index e2aeef11..2ca88a51 100644 --- a/go.sum +++ b/go.sum @@ -10,8 +10,8 @@ github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6r github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44= github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/chromedp/cdproto v0.0.0-20231011050154-1d073bb38998/go.mod h1:GKljq0VrfU4D5yc+2qA6OVr8pmO/MBbPEWqWQ/oqGEs= -github.com/chromedp/cdproto v0.0.0-20231114014204-3e458d5176f9 h1:e3tMnG8i9SfKOilykpprojNk3a49O4dn+wqZsam1qYQ= -github.com/chromedp/cdproto v0.0.0-20231114014204-3e458d5176f9/go.mod h1:GKljq0VrfU4D5yc+2qA6OVr8pmO/MBbPEWqWQ/oqGEs= +github.com/chromedp/cdproto v0.0.0-20231205062650-00455a960d61 h1:XD280QPATe9jaz20dylKe3vBsNcH1w3mkssGY0lidn8= +github.com/chromedp/cdproto v0.0.0-20231205062650-00455a960d61/go.mod h1:GKljq0VrfU4D5yc+2qA6OVr8pmO/MBbPEWqWQ/oqGEs= github.com/chromedp/chromedp v0.9.3 h1:Wq58e0dZOdHsxaj9Owmfcf+ibtpYN1N0FWVbaxa/esg= github.com/chromedp/chromedp v0.9.3/go.mod h1:NipeUkUcuzIdFbBP8eNNvl9upcceOfWzoJn6cRe4ksA= github.com/chromedp/sysutil v1.0.0 h1:+ZxhTpfpZlmchB58ih/LBHX52ky7w2VhQVKQMucy3Ic= @@ -53,8 +53,8 @@ github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8Hm github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/klauspost/compress v1.4.1/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A= github.com/klauspost/compress v1.11.4/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs= -github.com/klauspost/compress v1.17.3 h1:qkRjuerhUU1EmXLYGkSH6EZL+vPSxIrYjLNAK4slzwA= -github.com/klauspost/compress v1.17.3/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM= +github.com/klauspost/compress v1.17.4 h1:Ej5ixsIri7BrIjBkRZLTo6ghwrEtHFk7ijlczPW4fZ4= +github.com/klauspost/compress v1.17.4/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM= github.com/klauspost/cpuid v1.2.0/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek= github.com/klauspost/pgzip v1.2.5/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQKQE9RUs= github.com/klauspost/pgzip v1.2.6 h1:8RXeL5crjEUFnR2/Sn6GJNWtSQ3Dk8pq4CL3jvdDyjU= @@ -136,20 +136,20 @@ go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/zap v1.26.0 h1:sI7k6L95XOKS281NhVKOFCUNIvv9e0w4BF8N3u+tCRo= go.uber.org/zap v1.26.0/go.mod h1:dtElttAiwGvoJ/vj4IwHBS/gXsEu/pZ50mUIRWuG0so= -golang.org/x/crypto v0.15.0 h1:frVn1TEaCEaZcn3Tmd7Y2b5KKPaZ+I32Q2OA3kYp5TA= -golang.org/x/crypto v0.15.0/go.mod h1:4ChreQoLWfG3xLDer1WdlH5NdlQ3+mwnQq1YTKY+72g= +golang.org/x/crypto v0.16.0 h1:mMMrFzRSCF0GvB7Ne27XVtVAaXLrPmgPC7/v0tkwHaY= +golang.org/x/crypto v0.16.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= golang.org/x/image v0.14.0 h1:tNgSxAFe3jC4uYqvZdTr84SZoM1KfwdC9SKIFrLjFn4= golang.org/x/image v0.14.0/go.mod h1:HUYqC05R2ZcZ3ejNQsIHQDQiwWM4JBqmm6MKANTp4LE= -golang.org/x/net v0.18.0 h1:mIYleuAkSbHh0tCv7RvjL3F6ZVbLjq4+R7zbOn3Kokg= -golang.org/x/net v0.18.0/go.mod h1:/czyP5RqHAH4odGYxBJ1qz0+CE5WZ+2j1YgoEo8F2jQ= +golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c= +golang.org/x/net v0.19.0/go.mod h1:CfAk/cbD4CthTvqiEl8NpboMuiuOYsAr/7NOjZJtv1U= golang.org/x/sync v0.5.0 h1:60k92dhOjHxJkrqnwsfl8KuaHbn/5dl0lUPUklKo3qE= golang.org/x/sync v0.5.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.14.0 h1:Vz7Qs629MkJkGyHxUlRHizWJRG2j8fbQKjELVSNhy7Q= -golang.org/x/sys v0.14.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/term v0.14.0 h1:LGK9IlZ8T9jvdy6cTdfKUCltatMFOehAQo9SRC46UQ8= -golang.org/x/term v0.14.0/go.mod h1:TySc+nGkYR6qt8km8wUhuFRTVSMIX3XPR58y2lC8vww= +golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= +golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/term v0.15.0 h1:y/Oo/a/q3IXu26lQgl04j/gjuBDOBlx7X6Om1j2CPW4= +golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0= golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/pkg/modules/api/context.go b/pkg/modules/api/context.go index 48afed70..ac79801e 100644 --- a/pkg/modules/api/context.go +++ b/pkg/modules/api/context.go @@ -12,14 +12,11 @@ import ( "path/filepath" "strings" "time" - "unicode" "github.com/google/uuid" "github.com/labstack/echo/v4" "github.com/mholt/archiver/v3" "go.uber.org/zap" - "golang.org/x/text/runes" - "golang.org/x/text/transform" "golang.org/x/text/unicode/norm" "github.com/gotenberg/gotenberg/v7/pkg/gotenberg" @@ -124,15 +121,6 @@ func newContext(echoCtx echo.Context, logger *zap.Logger, fs *gotenberg.FileSyst ctx.files = make(map[string]string) copyToDisk := func(fh *multipart.FileHeader) error { - // Avoid directory traversal and normalize filename. - // See https://github.com/gotenberg/gotenberg/issues/104. - t := transform.Chain(norm.NFD, runes.Remove(runes.In(unicode.Mn)), norm.NFC) - - filename, _, err := transform.String(t, filepath.Base(fh.Filename)) - if err != nil { - return fmt.Errorf("transform filename: %w", err) - } - in, err := fh.Open() if err != nil { return fmt.Errorf("open multipart file: %w", err) @@ -145,6 +133,10 @@ func newContext(echoCtx echo.Context, logger *zap.Logger, fs *gotenberg.FileSyst } }() + // Avoid directory traversal and make sure filename characters are + // normalized. + // See: https://github.com/gotenberg/gotenberg/issues/662. + filename := norm.NFC.String(filepath.Base(fh.Filename)) path := fmt.Sprintf("%s/%s", ctx.dirPath, filename) out, err := os.Create(path) diff --git a/pkg/modules/libreoffice/api/libreoffice.go b/pkg/modules/libreoffice/api/libreoffice.go index d1c4267a..8ceaa416 100644 --- a/pkg/modules/libreoffice/api/libreoffice.go +++ b/pkg/modules/libreoffice/api/libreoffice.go @@ -4,12 +4,15 @@ import ( "context" "errors" "fmt" + "io" "net" "os" + "path/filepath" "sync" "sync/atomic" "time" + "github.com/google/uuid" "go.uber.org/zap" "github.com/gotenberg/gotenberg/v7/pkg/gotenberg" @@ -290,6 +293,11 @@ func (p *libreOfficeProcess) pdf(ctx context.Context, logger *zap.Logger, inputP ) } + inputPath, err := nonBasicLatinCharactersGuard(logger, inputPath) + if err != nil { + return fmt.Errorf("non-basic latin characters guard: %w", err) + } + args = append(args, "--output", outputPath, inputPath) cmd, err := gotenberg.CommandContext(ctx, logger, p.arguments.unoBinPath, args...) @@ -321,6 +329,65 @@ func (p *libreOfficeProcess) pdf(ctx context.Context, logger *zap.Logger, inputP return fmt.Errorf("convert to PDF: %w", err) } +// LibreOffice cannot convert a file with a name containing non-basic Latin +// characters. +// See: +// https://github.com/gotenberg/gotenberg/issues/104 +// https://github.com/gotenberg/gotenberg/issues/730 +func nonBasicLatinCharactersGuard(logger *zap.Logger, inputPath string) (string, error) { + hasNonBasicLatinChars := func(str string) bool { + for _, r := range str { + // Check if the character is outside basic Latin. + if r != '.' && (r < ' ' || r > '~') { + return true + } + } + return false + } + + filename := filepath.Base(inputPath) + if !hasNonBasicLatinChars(filename) { + logger.Debug("no non-basic latin characters in filename, skip copy") + return inputPath, nil + } + + logger.Warn("non-basic latin characters in filename, copy to a file with a valid filename") + basePath := filepath.Dir(inputPath) + ext := filepath.Ext(inputPath) + newInputPath := filepath.Join(basePath, fmt.Sprintf("%s%s", uuid.NewString(), ext)) + + in, err := os.Open(inputPath) + if err != nil { + return "", fmt.Errorf("open file: %w", err) + } + + defer func() { + err := in.Close() + if err != nil { + logger.Error(fmt.Sprintf("close file: %s", err)) + } + }() + + out, err := os.Create(newInputPath) + if err != nil { + return "", fmt.Errorf("create new file: %w", err) + } + + defer func() { + err := out.Close() + if err != nil { + logger.Error(fmt.Sprintf("close new file: %s", err)) + } + }() + + _, err = io.Copy(out, in) + if err != nil { + return "", fmt.Errorf("copy file to new file: %w", err) + } + + return newInputPath, nil +} + // Interface guards. var ( _ gotenberg.Process = (*libreOfficeProcess)(nil) diff --git a/pkg/modules/libreoffice/api/libreoffice_test.go b/pkg/modules/libreoffice/api/libreoffice_test.go index 4a205c99..b69ac414 100644 --- a/pkg/modules/libreoffice/api/libreoffice_test.go +++ b/pkg/modules/libreoffice/api/libreoffice_test.go @@ -545,7 +545,7 @@ func TestLibreOfficeProcess_pdf(t *testing.T) { err := tc.libreOffice.pdf( ctx, logger, - fmt.Sprintf("file://%s/document.txt", tc.fs.WorkingDirPath()), + fmt.Sprintf("%s/document.txt", tc.fs.WorkingDirPath()), fmt.Sprintf("%s/%s.pdf", tc.fs.WorkingDirPath(), uuid.NewString()), tc.options, ) @@ -564,3 +564,87 @@ func TestLibreOfficeProcess_pdf(t *testing.T) { }) } } + +func TestNonBasicLatinCharactersGuard(t *testing.T) { + for _, tc := range []struct { + scenario string + fs *gotenberg.FileSystem + filename string + expectSameInputPath bool + expectError bool + }{ + { + scenario: "basic latin characters", + fs: func() *gotenberg.FileSystem { + fs := gotenberg.NewFileSystem() + + err := os.MkdirAll(fs.WorkingDirPath(), 0o755) + if err != nil { + t.Fatalf(fmt.Sprintf("expected no error but got: %v", err)) + } + + err = os.WriteFile(fmt.Sprintf("%s/document.txt", fs.WorkingDirPath()), []byte("Basic latin characters"), 0o755) + if err != nil { + t.Fatalf("expected no error but got: %v", err) + } + + return fs + }(), + filename: "document.txt", + expectSameInputPath: true, + expectError: false, + }, + { + scenario: "non-basic latin characters", + fs: func() *gotenberg.FileSystem { + fs := gotenberg.NewFileSystem() + + err := os.MkdirAll(fs.WorkingDirPath(), 0o755) + if err != nil { + t.Fatalf(fmt.Sprintf("expected no error but got: %v", err)) + } + + err = os.WriteFile(fmt.Sprintf("%s/éèßàùä.txt", fs.WorkingDirPath()), []byte("Non-basic latin characters"), 0o755) + if err != nil { + t.Fatalf("expected no error but got: %v", err) + } + + return fs + }(), + filename: "éèßàùä.txt", + expectSameInputPath: false, + expectError: false, + }, + } { + t.Run(tc.scenario, func(t *testing.T) { + defer func() { + err := os.RemoveAll(tc.fs.WorkingDirPath()) + if err != nil { + t.Fatalf("expected no error while cleaning up, but got: %v", err) + } + }() + + inputPath := fmt.Sprintf("%s/%s", tc.fs.WorkingDirPath(), tc.filename) + newInputPath, err := nonBasicLatinCharactersGuard( + zap.NewNop(), + inputPath, + ) + + if !tc.expectError && err != nil { + t.Fatalf("expected no error but got: %v", err) + } + + if tc.expectError && err == nil { + t.Fatal("expected error but got none") + } + + if tc.expectSameInputPath && newInputPath != inputPath { + t.Fatalf("expected same input path, but got '%s'", newInputPath) + } + + if !tc.expectSameInputPath && newInputPath == inputPath { + t.Fatalf("expected different input path, but got same '%s'", newInputPath) + } + }) + } +}