From 31fa392db270defa572dc6d629a56362827e2798 Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Fri, 7 Aug 2026 15:33:44 +0200 Subject: [PATCH] fix(libreoffice)!: return 500 when a failure is not the client's fault --- pkg/modules/libreoffice/api/api.go | 28 +- pkg/modules/libreoffice/api/errortype_test.go | 3 + pkg/modules/libreoffice/api/libreoffice.go | 22 +- pkg/modules/libreoffice/api/protection.go | 129 ++++++++++ .../libreoffice/api/protection_test.go | 193 ++++++++++++++ pkg/modules/libreoffice/routes.go | 45 +++- pkg/modules/libreoffice/routes_test.go | 241 ++++++++++++++++++ .../features/libreoffice_convert.feature | 8 +- 8 files changed, 651 insertions(+), 18 deletions(-) create mode 100644 pkg/modules/libreoffice/api/protection.go create mode 100644 pkg/modules/libreoffice/api/protection_test.go create mode 100644 pkg/modules/libreoffice/routes_test.go diff --git a/pkg/modules/libreoffice/api/api.go b/pkg/modules/libreoffice/api/api.go index 93f9fba5..ca6c9785 100644 --- a/pkg/modules/libreoffice/api/api.go +++ b/pkg/modules/libreoffice/api/api.go @@ -33,12 +33,33 @@ var ( // formats option. ErrInvalidPdfFormats = errors.New("invalid PDF formats") - // ErrUnoException happens when unoconverter returns exit code 5. + // ErrUnoException happens when unoconverter returns exit code 5. That code + // is the residual bucket of unoconverter's catch-all UNO exception handler: + // it covers a malformed page range, a password supplied to a document that + // does not need one, a failure to open the document and a failure to write + // the output alike. It names the exception class that was caught, not a + // cause. See https://github.com/gotenberg/gotenberg/issues/1588. ErrUnoException = errors.New("uno exception") // ErrRuntimeException happens when unoconverter returns exit code 6. + // unoconverter's own message for it reads "Office probably died", yet a + // wrong or missing password also surfaces there. Like [ErrUnoException], it + // does not establish who is at fault. ErrRuntimeException = errors.New("runtime exception") + // ErrIoException happens when unoconverter returns exit code 3. LibreOffice + // could not read the source document. + ErrIoException = errors.New("io exception") + + // ErrCannotConvertException happens when unoconverter returns exit code 4. + // LibreOffice read the document but could not convert it to PDF. + ErrCannotConvertException = errors.New("cannot convert exception") + + // ErrIllegalArgumentException happens when unoconverter returns exit code + // 8. LibreOffice rejected the source document, usually because its contents + // do not match its extension. + ErrIllegalArgumentException = errors.New("illegal argument exception") + // ErrCoreDumped happens randomly; sometimes a conversion will work as // expected, and some other time the same conversion will fail. // See https://github.com/gotenberg/gotenberg/issues/639. @@ -767,7 +788,10 @@ func conversionRequestAttributes(inputPath string, options Options) []attribute. // [gotenberg.ClassifyError]. func libreofficeErrorType(err error) string { switch { - case errors.Is(err, ErrInvalidPdfFormats): + case errors.Is(err, ErrInvalidPdfFormats), + errors.Is(err, ErrIoException), + errors.Is(err, ErrCannotConvertException), + errors.Is(err, ErrIllegalArgumentException): return gotenberg.ErrorTypeInvalidInput case errors.Is(err, ErrUnoException), errors.Is(err, ErrRuntimeException): return "libreoffice_exception" diff --git a/pkg/modules/libreoffice/api/errortype_test.go b/pkg/modules/libreoffice/api/errortype_test.go index 79a0fe60..62e94ff4 100644 --- a/pkg/modules/libreoffice/api/errortype_test.go +++ b/pkg/modules/libreoffice/api/errortype_test.go @@ -17,6 +17,9 @@ func TestLibreofficeErrorType(t *testing.T) { {"deadline", context.DeadlineExceeded, "timeout"}, {"canceled", context.Canceled, "context_cancelled"}, {"invalid pdf formats", ErrInvalidPdfFormats, "invalid_input"}, + {"io exception", ErrIoException, "invalid_input"}, + {"cannot convert exception", ErrCannotConvertException, "invalid_input"}, + {"illegal argument exception", ErrIllegalArgumentException, "invalid_input"}, {"uno exception", ErrUnoException, "libreoffice_exception"}, {"runtime exception", ErrRuntimeException, "libreoffice_exception"}, {"queue size exceeded", gotenberg.ErrMaximumQueueSizeExceeded, "libreoffice_unavailable"}, diff --git a/pkg/modules/libreoffice/api/libreoffice.go b/pkg/modules/libreoffice/api/libreoffice.go index a04a950a..d722b90f 100644 --- a/pkg/modules/libreoffice/api/libreoffice.go +++ b/pkg/modules/libreoffice/api/libreoffice.go @@ -435,9 +435,11 @@ func (p *libreOfficeProcess) pdf(ctx context.Context, logger *slog.Logger, input return nil } - // LibreOffice's errors are not explicit. - // For instance, exit code 5 may be explained by a malformed page range - // but also by a not required password. + // LibreOffice's errors are not explicit: unoconverter derives its exit code + // from the UNO exception class it caught, not from a diagnosis. Exit codes + // 5 and 6 are ambiguous in particular, so the route decides the HTTP status + // from the request and the document rather than from the code alone. + // See https://github.com/gotenberg/gotenberg/issues/1588. // We may want to retry in case of a core-dumped event. // See https://github.com/gotenberg/gotenberg/issues/639. @@ -445,13 +447,17 @@ func (p *libreOfficeProcess) pdf(ctx context.Context, logger *slog.Logger, input return ErrCoreDumped } - if exitCode == 5 { - // Potentially malformed page ranges or password not required. + switch exitCode { + case 3: + return ErrIoException + case 4: + return ErrCannotConvertException + case 5: return ErrUnoException - } - if exitCode == 6 { - // Password potentially required or invalid. + case 6: return ErrRuntimeException + case 8: + return ErrIllegalArgumentException } return fmt.Errorf("convert to PDF: %w", err) diff --git a/pkg/modules/libreoffice/api/protection.go b/pkg/modules/libreoffice/api/protection.go new file mode 100644 index 00000000..9ada1596 --- /dev/null +++ b/pkg/modules/libreoffice/api/protection.go @@ -0,0 +1,129 @@ +package api + +import ( + "archive/zip" + "bytes" + "io" + "os" + "path/filepath" + "strings" +) + +// PasswordProtection describes whether a document requires a password to open. +type PasswordProtection int + +const ( + // PasswordProtectionUnknown means the document's encryption state could not + // be determined. + PasswordProtectionUnknown PasswordProtection = iota + + // PasswordProtectionNone means the document opens without a password. + PasswordProtectionNone + + // PasswordProtectionRequired means the document is encrypted. + PasswordProtectionRequired +) + +var ( + // Compound File Binary magic. An encrypted OOXML document is an + // MS-OFFCRYPTO container, which is a compound file. Per MS-CFB 2.2, the + // header signature is fixed. + ole2Magic = []byte{0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1} + + // Local file header signature. Per APPNOTE.TXT 4.3.7, every ZIP entry + // starts with it, so an intact package starts with it too. + zipMagic = []byte{0x50, 0x4b, 0x03, 0x04} + + // An unencrypted OOXML document is always a ZIP package, so any of these + // extensions over a compound file means the payload is encrypted. Legacy + // binary formats (.doc, .xls, .ppt) are compound files either way and are + // deliberately absent. + ooxmlExtensions = map[string]struct{}{ + ".docx": {}, ".docm": {}, ".dotx": {}, ".dotm": {}, + ".xlsx": {}, ".xlsm": {}, ".xltx": {}, ".xltm": {}, + ".pptx": {}, ".pptm": {}, ".potx": {}, ".potm": {}, + ".ppsx": {}, ".ppsm": {}, + } +) + +// odfManifestSizeLimit caps how much of an ODF manifest is read. The manifest +// is a few kilobytes in practice; the cap stops a crafted archive from +// exhausting memory through its decompressed size. +const odfManifestSizeLimit = 1 << 20 + +// DetectPasswordProtection reports whether the document at path is encrypted. +// +// Detection is advisory and never fails: an unreadable file, an unknown format +// or a malformed archive all yield [PasswordProtectionUnknown]. It exists to +// refine the diagnosis of a conversion that already failed, since LibreOffice's +// exit codes do not distinguish a missing password from a crash. +func DetectPasswordProtection(path string) PasswordProtection { + f, err := os.Open(path) + if err != nil { + return PasswordProtectionUnknown + } + defer func() { + _ = f.Close() + }() + + magic := make([]byte, 8) + n, err := io.ReadFull(f, magic) + if err != nil && n < len(zipMagic) { + return PasswordProtectionUnknown + } + magic = magic[:n] + + switch { + case bytes.HasPrefix(magic, ole2Magic): + if _, ok := ooxmlExtensions[strings.ToLower(filepath.Ext(path))]; ok { + return PasswordProtectionRequired + } + // A legacy binary document is a compound file whether or not it is + // encrypted; its encryption lives in a stream this cannot cheaply read. + return PasswordProtectionUnknown + case bytes.HasPrefix(magic, zipMagic): + return detectZipPasswordProtection(f) + default: + // Flat XML (.fodt), RTF, CSV and everything else carry no encryption. + return PasswordProtectionUnknown + } +} + +// detectZipPasswordProtection inspects a ZIP package. ODF keeps META-INF/manifest.xml +// in cleartext even when encrypted, declaring each encrypted entry. An OOXML +// package has no manifest, and reaching this point already proves it is not an +// MS-OFFCRYPTO container, so it opens without a password. +func detectZipPasswordProtection(f *os.File) PasswordProtection { + size, err := f.Seek(0, io.SeekEnd) + if err != nil { + return PasswordProtectionUnknown + } + + r, err := zip.NewReader(f, size) + if err != nil { + return PasswordProtectionUnknown + } + + manifest, err := r.Open("META-INF/manifest.xml") + if err != nil { + // No manifest: an OOXML package, or a ZIP that is not an office + // document at all. Neither is encrypted. + return PasswordProtectionNone + } + defer func() { + _ = manifest.Close() + }() + + content, err := io.ReadAll(io.LimitReader(manifest, odfManifestSizeLimit)) + if err != nil { + return PasswordProtectionUnknown + } + + // Per OpenDocument 1.3 part 3, section 4.16, an encrypted entry carries a + // child. + if bytes.Contains(content, []byte("encryption-data")) { + return PasswordProtectionRequired + } + + return PasswordProtectionNone +} diff --git a/pkg/modules/libreoffice/api/protection_test.go b/pkg/modules/libreoffice/api/protection_test.go new file mode 100644 index 00000000..88fa88df --- /dev/null +++ b/pkg/modules/libreoffice/api/protection_test.go @@ -0,0 +1,193 @@ +package api + +import ( + "archive/zip" + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +// writeFile writes content to a new file named name inside dir and returns its +// path. +func writeFile(t *testing.T, dir, name string, content []byte) string { + t.Helper() + + path := filepath.Join(dir, name) + err := os.WriteFile(path, content, 0o600) + if err != nil { + t.Fatalf("write %s: %v", path, err) + } + + return path +} + +// writeZip builds a ZIP archive from entries and returns its path. +func writeZip(t *testing.T, dir, name string, entries map[string]string) string { + t.Helper() + + buf := new(bytes.Buffer) + w := zip.NewWriter(buf) + + for entryName, content := range entries { + f, err := w.Create(entryName) + if err != nil { + t.Fatalf("create zip entry %s: %v", entryName, err) + } + _, err = f.Write([]byte(content)) + if err != nil { + t.Fatalf("write zip entry %s: %v", entryName, err) + } + } + + err := w.Close() + if err != nil { + t.Fatalf("close zip writer: %v", err) + } + + return writeFile(t, dir, name, buf.Bytes()) +} + +func TestDetectPasswordProtection(t *testing.T) { + dir := t.TempDir() + + ole2 := func(name string) string { + return writeFile(t, dir, name, append(ole2Magic, bytes.Repeat([]byte{0x00}, 64)...)) + } + + for _, tc := range []struct { + name string + path string + want PasswordProtection + }{ + { + name: "encrypted OOXML is a compound file", + path: ole2("encrypted.docx"), + want: PasswordProtectionRequired, + }, + { + name: "extension casing is ignored", + path: ole2("encrypted.DOCX"), + want: PasswordProtectionRequired, + }, + { + name: "encrypted spreadsheet", + path: ole2("encrypted.xlsx"), + want: PasswordProtectionRequired, + }, + { + name: "legacy binary document is inconclusive", + path: ole2("legacy.doc"), + want: PasswordProtectionUnknown, + }, + { + name: "plain OOXML package", + path: writeZip(t, dir, "plain.docx", map[string]string{ + "[Content_Types].xml": "", + "word/document.xml": "", + }), + want: PasswordProtectionNone, + }, + { + name: "encrypted ODF declares encryption-data in its manifest", + path: writeZip(t, dir, "encrypted.odt", map[string]string{ + "mimetype": "application/vnd.oasis.opendocument.text", + "META-INF/manifest.xml": ``, + "content.xml": "", + }), + want: PasswordProtectionRequired, + }, + { + name: "plain ODF has a manifest without encryption-data", + path: writeZip(t, dir, "plain.odt", map[string]string{ + "mimetype": "application/vnd.oasis.opendocument.text", + "META-INF/manifest.xml": ``, + "content.xml": "", + }), + want: PasswordProtectionNone, + }, + { + name: "flat XML carries no encryption", + path: writeFile(t, dir, "flat.fodt", []byte("")), + want: PasswordProtectionUnknown, + }, + { + name: "plain text", + path: writeFile(t, dir, "notes.txt", []byte("hello")), + want: PasswordProtectionUnknown, + }, + { + name: "file shorter than any magic", + path: writeFile(t, dir, "tiny.docx", []byte{0x50}), + want: PasswordProtectionUnknown, + }, + { + name: "empty file", + path: writeFile(t, dir, "empty.docx", nil), + want: PasswordProtectionUnknown, + }, + { + name: "truncated archive", + path: writeFile(t, dir, "truncated.docx", append(zipMagic, bytes.Repeat([]byte{0x00}, 32)...)), + want: PasswordProtectionUnknown, + }, + { + name: "non-existent path", + path: filepath.Join(dir, "does-not-exist.docx"), + want: PasswordProtectionUnknown, + }, + { + name: "directory", + path: dir, + want: PasswordProtectionUnknown, + }, + } { + t.Run(tc.name, func(t *testing.T) { + if got := DetectPasswordProtection(tc.path); got != tc.want { + t.Errorf("DetectPasswordProtection(%s) = %d, want %d", tc.path, got, tc.want) + } + }) + } +} + +// TestDetectPasswordProtection_Fixtures anchors detection to the same documents +// the integration scenarios upload, so a fixture swap cannot silently flip a +// status code. +func TestDetectPasswordProtection_Fixtures(t *testing.T) { + for _, tc := range []struct { + path string + want PasswordProtection + }{ + {"../../../../test/integration/testdata/protected_page_1.docx", PasswordProtectionRequired}, + {"../../../../test/integration/testdata/page_1.docx", PasswordProtectionNone}, + } { + t.Run(filepath.Base(tc.path), func(t *testing.T) { + if _, err := os.Stat(tc.path); err != nil { + t.Skipf("fixture unavailable: %v", err) + } + if got := DetectPasswordProtection(tc.path); got != tc.want { + t.Errorf("DetectPasswordProtection(%s) = %d, want %d", tc.path, got, tc.want) + } + }) + } +} + +// TestDetectPasswordProtection_OversizedManifest verifies that a manifest far +// larger than the cap still yields a verdict through a bounded read. +func TestDetectPasswordProtection_OversizedManifest(t *testing.T) { + dir := t.TempDir() + + // Well past odfManifestSizeLimit, and highly compressible, so the archive + // on disk stays small. + filler := strings.Repeat("", 200_000) + + path := writeZip(t, dir, "oversized.odt", map[string]string{ + "mimetype": "application/vnd.oasis.opendocument.text", + "META-INF/manifest.xml": "" + filler + "", + }) + + if got := DetectPasswordProtection(path); got != PasswordProtectionNone { + t.Errorf("DetectPasswordProtection(oversized) = %d, want %d", got, PasswordProtectionNone) + } +} diff --git a/pkg/modules/libreoffice/routes.go b/pkg/modules/libreoffice/routes.go index 83ce0576..38562f33 100644 --- a/pkg/modules/libreoffice/routes.go +++ b/pkg/modules/libreoffice/routes.go @@ -15,6 +15,11 @@ import ( "github.com/gotenberg/gotenberg/v8/pkg/modules/pdfengines" ) +// unattributableFailureMessage is returned when LibreOffice fails and no +// client-supplied input is implicated. Its only format verb is the original +// filename. +const unattributableFailureMessage = "LibreOffice failed to convert the document '%s'. This is usually a resource issue: increase the container's memory and CPU, or reduce the document's size. The request is valid and may be retried." + // convertRoute returns an [api.Route] which can convert LibreOffice documents // to PDF. func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) api.Route { @@ -405,20 +410,52 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap ) } - if errors.Is(err, libreofficeapi.ErrUnoException) { + filename := ctx.OriginalFilename(inputPath) + + if errors.Is(err, libreofficeapi.ErrIoException) || errors.Is(err, libreofficeapi.ErrIllegalArgumentException) { return api.WrapError( fmt.Errorf("convert to PDF: %w", err), - api.NewSentinelHttpError(http.StatusBadRequest, fmt.Sprintf("LibreOffice failed to process a document: possible causes include malformed page ranges '%s' (nativePageRanges), or, if a password has been provided, it may not be required. In any case, the exact cause is uncertain.", options.PageRanges)), + api.NewSentinelHttpError(http.StatusBadRequest, fmt.Sprintf("LibreOffice could not read the document '%s'. Ensure the file is not corrupted and that its extension matches its actual format.", filename)), ) } - if errors.Is(err, libreofficeapi.ErrRuntimeException) { + if errors.Is(err, libreofficeapi.ErrCannotConvertException) { return api.WrapError( fmt.Errorf("convert to PDF: %w", err), - api.NewSentinelHttpError(http.StatusBadRequest, "LibreOffice failed to process a document: a password may be required, or, if one has been given, it is invalid. In any case, the exact cause is uncertain."), + api.NewSentinelHttpError(http.StatusBadRequest, fmt.Sprintf("LibreOffice read the document '%s' but could not convert it to PDF. The document may be corrupted or rely on an unsupported feature.", filename)), ) } + // Exit codes 5 and 6 name the UNO exception class that was + // caught, not a cause: both cover a client mistake and a + // LibreOffice crash. Blame the client only when one of its + // inputs is actually implicated, since the server is the + // only remaining explanation otherwise. Password evidence + // outranks page ranges: a password failure aborts on import, + // before the export filter applies any page range. + // See https://github.com/gotenberg/gotenberg/issues/1588. + if errors.Is(err, libreofficeapi.ErrUnoException) || errors.Is(err, libreofficeapi.ErrRuntimeException) { + protection := libreofficeapi.DetectPasswordProtection(inputPath) + + var sentinel api.SentinelHttpError + switch { + case protection == libreofficeapi.PasswordProtectionRequired && options.Password == "": + sentinel = api.NewSentinelHttpError(http.StatusBadRequest, fmt.Sprintf("The document '%s' is password-protected. Provide its password in the 'password' form field.", filename)) + case protection == libreofficeapi.PasswordProtectionRequired: + sentinel = api.NewSentinelHttpError(http.StatusBadRequest, fmt.Sprintf("The password for the document '%s' is incorrect. Check the 'password' form field.", filename)) + case protection == libreofficeapi.PasswordProtectionNone && options.Password != "": + sentinel = api.NewSentinelHttpError(http.StatusBadRequest, fmt.Sprintf("The document '%s' is not password-protected. Remove the 'password' form field.", filename)) + case options.Password != "": + sentinel = api.NewSentinelHttpError(http.StatusBadRequest, fmt.Sprintf("LibreOffice could not open the document '%s' with the given password. Check the 'password' form field, and omit it if the document is not password-protected.", filename)) + case errors.Is(err, libreofficeapi.ErrUnoException) && options.PageRanges != "": + sentinel = api.NewSentinelHttpError(http.StatusBadRequest, fmt.Sprintf("LibreOffice could not apply the page ranges '%s' to the document '%s'. Check the 'nativePageRanges' form field; valid values look like '1-4', '2' or '1,3,5-7'.", options.PageRanges, filename)) + default: + sentinel = api.NewSentinelHttpError(http.StatusInternalServerError, fmt.Sprintf(unattributableFailureMessage, filename)) + } + + return api.WrapError(fmt.Errorf("convert to PDF: %w", err), sentinel) + } + return fmt.Errorf("convert to PDF: %w", err) } } diff --git a/pkg/modules/libreoffice/routes_test.go b/pkg/modules/libreoffice/routes_test.go new file mode 100644 index 00000000..f67b801b --- /dev/null +++ b/pkg/modules/libreoffice/routes_test.go @@ -0,0 +1,241 @@ +package libreoffice + +import ( + "archive/zip" + "bytes" + "context" + "fmt" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/labstack/echo/v4" + + "github.com/gotenberg/gotenberg/v8/pkg/gotenberg" + "github.com/gotenberg/gotenberg/v8/pkg/modules/api" + libreofficeapi "github.com/gotenberg/gotenberg/v8/pkg/modules/libreoffice/api" +) + +// compoundFile writes a document whose header marks it as a compound file. Over +// an OOXML extension, that means an encrypted payload. +func compoundFile(t *testing.T, dir, name string) string { + t.Helper() + + content := append( + []byte{0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1}, + bytes.Repeat([]byte{0x00}, 64)..., + ) + + return writeTestFile(t, dir, name, content) +} + +// zipPackage writes a minimal, unencrypted OOXML package. +func zipPackage(t *testing.T, dir, name string) string { + t.Helper() + + buf := new(bytes.Buffer) + w := zip.NewWriter(buf) + + f, err := w.Create("[Content_Types].xml") + if err != nil { + t.Fatalf("create zip entry: %v", err) + } + _, err = f.Write([]byte("")) + if err != nil { + t.Fatalf("write zip entry: %v", err) + } + err = w.Close() + if err != nil { + t.Fatalf("close zip writer: %v", err) + } + + return writeTestFile(t, dir, name, buf.Bytes()) +} + +func writeTestFile(t *testing.T, dir, name string, content []byte) string { + t.Helper() + + path := filepath.Join(dir, name) + err := os.WriteFile(path, content, 0o600) + if err != nil { + t.Fatalf("write %s: %v", path, err) + } + + return path +} + +// TestConvertRoute_FailureStatus pins the branch table that decides whether a +// LibreOffice failure is the client's fault. See +// https://github.com/gotenberg/gotenberg/issues/1588. +func TestConvertRoute_FailureStatus(t *testing.T) { + dir := t.TempDir() + + var ( + protected = compoundFile(t, dir, "protected_page_1.docx") + plain = zipPackage(t, dir, "page_1.docx") + legacy = compoundFile(t, dir, "legacy.doc") + corrupted = writeTestFile(t, dir, "corrupted.docx", []byte("not a document")) + unreachable = filepath.Join(dir, "vanished.docx") + ) + + for _, tc := range []struct { + name string + inputPath string + values map[string][]string + err error + wantStatus int + wantBody string + }{ + { + name: "encrypted document, no password", + inputPath: protected, + err: libreofficeapi.ErrRuntimeException, + wantStatus: http.StatusBadRequest, + wantBody: "The document 'protected_page_1.docx' is password-protected. Provide its password in the 'password' form field.", + }, + { + name: "encrypted document, wrong password", + inputPath: protected, + values: map[string][]string{"password": {"bar"}}, + err: libreofficeapi.ErrRuntimeException, + wantStatus: http.StatusBadRequest, + wantBody: "The password for the document 'protected_page_1.docx' is incorrect. Check the 'password' form field.", + }, + { + name: "unencrypted document, password supplied", + inputPath: plain, + values: map[string][]string{"password": {"foo"}}, + err: libreofficeapi.ErrUnoException, + wantStatus: http.StatusBadRequest, + wantBody: "The document 'page_1.docx' is not password-protected. Remove the 'password' form field.", + }, + { + name: "inconclusive document, password supplied", + inputPath: legacy, + values: map[string][]string{"password": {"foo"}}, + err: libreofficeapi.ErrUnoException, + wantStatus: http.StatusBadRequest, + wantBody: "LibreOffice could not open the document 'legacy.doc' with the given password. Check the 'password' form field, and omit it if the document is not password-protected.", + }, + { + name: "malformed page ranges", + inputPath: plain, + values: map[string][]string{"nativePageRanges": {"foo"}}, + err: libreofficeapi.ErrUnoException, + wantStatus: http.StatusBadRequest, + wantBody: "LibreOffice could not apply the page ranges 'foo' to the document 'page_1.docx'. Check the 'nativePageRanges' form field; valid values look like '1-4', '2' or '1,3,5-7'.", + }, + { + name: "password evidence outranks page ranges", + inputPath: protected, + values: map[string][]string{"nativePageRanges": {"1-2"}}, + err: libreofficeapi.ErrUnoException, + wantStatus: http.StatusBadRequest, + wantBody: "The document 'protected_page_1.docx' is password-protected. Provide its password in the 'password' form field.", + }, + { + name: "page ranges do not excuse a runtime exception", + inputPath: plain, + values: map[string][]string{"nativePageRanges": {"1-2"}}, + err: libreofficeapi.ErrRuntimeException, + wantStatus: http.StatusInternalServerError, + wantBody: fmt.Sprintf(unattributableFailureMessage, "page_1.docx"), + }, + { + name: "nothing implicated, uno exception", + inputPath: plain, + err: libreofficeapi.ErrUnoException, + wantStatus: http.StatusInternalServerError, + wantBody: fmt.Sprintf(unattributableFailureMessage, "page_1.docx"), + }, + { + name: "nothing implicated, runtime exception", + inputPath: plain, + err: libreofficeapi.ErrRuntimeException, + wantStatus: http.StatusInternalServerError, + wantBody: fmt.Sprintf(unattributableFailureMessage, "page_1.docx"), + }, + { + name: "detection cannot read the document", + inputPath: unreachable, + err: libreofficeapi.ErrUnoException, + wantStatus: http.StatusInternalServerError, + wantBody: fmt.Sprintf(unattributableFailureMessage, "vanished.docx"), + }, + { + name: "unreadable source", + inputPath: corrupted, + err: libreofficeapi.ErrIoException, + wantStatus: http.StatusBadRequest, + wantBody: "LibreOffice could not read the document 'corrupted.docx'. Ensure the file is not corrupted and that its extension matches its actual format.", + }, + { + name: "rejected source", + inputPath: corrupted, + err: libreofficeapi.ErrIllegalArgumentException, + wantStatus: http.StatusBadRequest, + wantBody: "LibreOffice could not read the document 'corrupted.docx'. Ensure the file is not corrupted and that its extension matches its actual format.", + }, + { + name: "unconvertible document", + inputPath: corrupted, + err: libreofficeapi.ErrCannotConvertException, + wantStatus: http.StatusBadRequest, + wantBody: "LibreOffice read the document 'corrupted.docx' but could not convert it to PDF. The document may be corrupted or rely on an unsupported feature.", + }, + { + name: "core dumped past the retry cap", + inputPath: plain, + err: libreofficeapi.ErrCoreDumped, + wantStatus: http.StatusInternalServerError, + wantBody: http.StatusText(http.StatusInternalServerError), + }, + { + name: "unmapped exit code", + inputPath: plain, + err: fmt.Errorf("convert to PDF: exit status 7"), + wantStatus: http.StatusInternalServerError, + wantBody: http.StatusText(http.StatusInternalServerError), + }, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := &api.ContextMock{Context: new(api.Context)} + ctx.SetDirPath(dir) + ctx.SetFiles(map[string]string{filepath.Base(tc.inputPath): tc.inputPath}) + ctx.SetValues(tc.values) + ctx.SetLogger(slog.New(slog.DiscardHandler)) + + uno := &libreofficeapi.ApiMock{ + ExtensionsMock: func() []string { + return []string{".docx", ".doc"} + }, + PdfMock: func(_ context.Context, _ *slog.Logger, _, _ string, _ libreofficeapi.Options) error { + // Mirror the wrapping done by [libreofficeapi.Api.Pdf]. + return fmt.Errorf("supervisor run task: %w", tc.err) + }, + } + + c := echo.New().NewContext( + httptest.NewRequest(http.MethodPost, "/forms/libreoffice/convert", nil), + httptest.NewRecorder(), + ) + c.Set("context", ctx.Context) + + err := convertRoute(uno, new(gotenberg.PdfEngineMock)).Handler(c) + if err == nil { + t.Fatal("expected an error, got none") + } + + status, message := api.ParseError(err) + if status != tc.wantStatus { + t.Errorf("status = %d, want %d (message: %s)", status, tc.wantStatus, message) + } + if message != tc.wantBody { + t.Errorf("message =\n%s\nwant\n%s", message, tc.wantBody) + } + }) + } +} diff --git a/test/integration/features/libreoffice_convert.feature b/test/integration/features/libreoffice_convert.feature index d659347f..cc9c4fc7 100644 --- a/test/integration/features/libreoffice_convert.feature +++ b/test/integration/features/libreoffice_convert.feature @@ -88,7 +88,7 @@ Feature: /forms/libreoffice/convert Then the response header "Content-Type" should be "text/plain; charset=UTF-8" Then the response body should match string: """ - LibreOffice failed to process a document: a password may be required, or, if one has been given, it is invalid. In any case, the exact cause is uncertain. + The document 'protected_page_1.docx' is password-protected. Provide its password in the 'password' form field. """ When I make a "POST" request to Gotenberg at the "/forms/libreoffice/convert" endpoint with the following form data and header(s): | files | testdata/protected_page_1.docx | file | @@ -255,7 +255,7 @@ Feature: /forms/libreoffice/convert Then the response header "Content-Type" should be "text/plain; charset=UTF-8" Then the response body should match string: """ - LibreOffice failed to process a document: possible causes include malformed page ranges 'foo' (nativePageRanges), or, if a password has been provided, it may not be required. In any case, the exact cause is uncertain. + LibreOffice could not apply the page ranges 'foo' to the document 'page_1.docx'. Check the 'nativePageRanges' form field; valid values look like '1-4', '2' or '1,3,5-7'. """ When I make a "POST" request to Gotenberg at the "/forms/libreoffice/convert" endpoint with the following form data and header(s): | files | testdata/page_1.docx | file | @@ -264,7 +264,7 @@ Feature: /forms/libreoffice/convert Then the response header "Content-Type" should be "text/plain; charset=UTF-8" Then the response body should match string: """ - LibreOffice failed to process a document: possible causes include malformed page ranges '' (nativePageRanges), or, if a password has been provided, it may not be required. In any case, the exact cause is uncertain. + The document 'page_1.docx' is not password-protected. Remove the 'password' form field. """ When I make a "POST" request to Gotenberg at the "/forms/libreoffice/convert" endpoint with the following form data and header(s): | files | testdata/protected_page_1.docx | file | @@ -273,7 +273,7 @@ Feature: /forms/libreoffice/convert Then the response header "Content-Type" should be "text/plain; charset=UTF-8" Then the response body should match string: """ - LibreOffice failed to process a document: a password may be required, or, if one has been given, it is invalid. In any case, the exact cause is uncertain. + The password for the document 'protected_page_1.docx' is incorrect. Check the 'password' form field. """ When I make a "POST" request to Gotenberg at the "/forms/libreoffice/convert" endpoint with the following form data and header(s): | files | testdata/page_1.docx | file |