diff --git a/pkg/modules/libreoffice/api/api.go b/pkg/modules/libreoffice/api/api.go index 7de4801f..e32712cf 100644 --- a/pkg/modules/libreoffice/api/api.go +++ b/pkg/modules/libreoffice/api/api.go @@ -25,9 +25,11 @@ var ( // by LibreOffice. ErrInvalidPdfFormats = errors.New("invalid PDF formats") - // ErrMalformedPageRanges happens if the page ranges option cannot be - // interpreted by LibreOffice. - ErrMalformedPageRanges = errors.New("page ranges are malformed") + // ErrUnoException happens when unoconverter returns an exit code 5. + ErrUnoException = errors.New("uno exception") + + // ErrRuntimeException happens when unoconverter returns an exit code 6. + ErrRuntimeException = errors.New("uno exception") // ErrCoreDumped happens randomly; sometime a conversion will work as // expected, and some other time the same conversion will fail. @@ -48,6 +50,9 @@ type Api struct { // Options gathers available options when converting a document to PDF. // See: https://help.libreoffice.org/latest/en-US/text/shared/guide/pdf_params.html. type Options struct { + // Password specifies the password for opening the source file. + Password string + // Landscape allows to change the orientation of the resulting PDF. Landscape bool @@ -141,6 +146,7 @@ type Options struct { // DefaultOptions returns the default values for Options. func DefaultOptions() Options { return Options{ + Password: "", Landscape: false, PageRanges: "", ExportFormFields: true, @@ -380,6 +386,7 @@ func (a *Api) Pdf(ctx context.Context, logger *zap.Logger, inputPath, outputPath // See https://github.com/gotenberg/gotenberg/issues/639. if errors.Is(err, ErrCoreDumped) { + logger.Debug(fmt.Sprintf("got a '%s' error, retry conversion", err)) return a.Pdf(ctx, logger, inputPath, outputPath, options) } diff --git a/pkg/modules/libreoffice/api/libreoffice.go b/pkg/modules/libreoffice/api/libreoffice.go index afc2b705..3eb7796f 100644 --- a/pkg/modules/libreoffice/api/libreoffice.go +++ b/pkg/modules/libreoffice/api/libreoffice.go @@ -266,6 +266,10 @@ func (p *libreOfficeProcess) pdf(ctx context.Context, logger *zap.Logger, inputP args = append(args, "-vvv") } + if options.Password != "" { + args = append(args, "--password", options.Password) + } + if options.Landscape { args = append(args, "--printer", "PaperOrientation=landscape") } @@ -343,11 +347,8 @@ func (p *libreOfficeProcess) pdf(ctx context.Context, logger *zap.Logger, inputP } // LibreOffice's errors are not explicit. - // That's why we have to make an educated guess according to the exit code - // and given inputs. - if exitCode == 5 && options.PageRanges != "" { - return ErrMalformedPageRanges - } + // For instance, an exit code 5 may be explained by a malformed page + // ranges, but also by a not required password. // We may want to retry in case of a core dumped event. // See https://github.com/gotenberg/gotenberg/issues/639. @@ -355,6 +356,15 @@ func (p *libreOfficeProcess) pdf(ctx context.Context, logger *zap.Logger, inputP return ErrCoreDumped } + if exitCode == 5 { + // Potentially malformed page ranges or password not required. + return ErrUnoException + } + if exitCode == 6 { + // Password potentially required or invalid. + return ErrRuntimeException + } + // Possible errors: // 1. LibreOffice failed for some reason. // 2. Context done. diff --git a/pkg/modules/libreoffice/api/libreoffice_test.go b/pkg/modules/libreoffice/api/libreoffice_test.go index a8de5048..953cb908 100644 --- a/pkg/modules/libreoffice/api/libreoffice_test.go +++ b/pkg/modules/libreoffice/api/libreoffice_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "os" "testing" "time" @@ -250,7 +251,7 @@ func TestLibreOfficeProcess_pdf(t *testing.T) { expectedError: ErrInvalidPdfFormats, }, { - scenario: "ErrMalformedPageRanges", + scenario: "ErrUnoException", libreOffice: newLibreOfficeProcess( libreOfficeArguments{ binPath: os.Getenv("LIBREOFFICE_BIN_PATH"), @@ -267,7 +268,7 @@ func TestLibreOfficeProcess_pdf(t *testing.T) { t.Fatalf(fmt.Sprintf("expected no error but got: %v", err)) } - err = os.WriteFile(fmt.Sprintf("%s/document.txt", fs.WorkingDirPath()), []byte("ErrMalformedPageRanges"), 0o755) + err = os.WriteFile(fmt.Sprintf("%s/document.txt", fs.WorkingDirPath()), []byte("Context done"), 0o755) if err != nil { t.Fatalf("expected no error but got: %v", err) } @@ -277,7 +278,61 @@ func TestLibreOfficeProcess_pdf(t *testing.T) { cancelledCtx: false, start: true, expectError: true, - expectedError: ErrMalformedPageRanges, + expectedError: ErrUnoException, + }, + { + scenario: "ErrRuntimeException", + libreOffice: newLibreOfficeProcess( + libreOfficeArguments{ + binPath: os.Getenv("LIBREOFFICE_BIN_PATH"), + unoBinPath: os.Getenv("UNOCONVERTER_BIN_PATH"), + startTimeout: 5 * time.Second, + }, + ), + options: Options{Password: "foo"}, + 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)) + } + + in, err := os.Open("/tests/test/testdata/libreoffice/protected.docx") + if err != nil { + t.Fatalf(fmt.Sprintf("expected no error but got: %v", err)) + } + + defer func() { + err := in.Close() + if err != nil { + t.Fatalf(fmt.Sprintf("expected no error but got: %v", err)) + } + }() + + out, err := os.Create(fmt.Sprintf("%s/protected.docx", fs.WorkingDirPath())) + if err != nil { + t.Fatalf(fmt.Sprintf("expected no error but got: %v", err)) + } + + defer func() { + err := out.Close() + if err != nil { + t.Fatalf(fmt.Sprintf("expected no error but got: %v", err)) + } + }() + + _, err = io.Copy(out, in) + if err != nil { + t.Fatalf(fmt.Sprintf("expected no error but got: %v", err)) + } + + return fs + }(), + cancelledCtx: false, + start: true, + expectError: true, + expectedError: ErrRuntimeException, }, { scenario: "context done", @@ -360,6 +415,7 @@ func TestLibreOfficeProcess_pdf(t *testing.T) { return fs }(), options: Options{ + Password: "", // Ok, the only exception in this list. Landscape: true, PageRanges: "1", ExportFormFields: false, diff --git a/pkg/modules/libreoffice/routes.go b/pkg/modules/libreoffice/routes.go index 1dde35f7..31d22d12 100644 --- a/pkg/modules/libreoffice/routes.go +++ b/pkg/modules/libreoffice/routes.go @@ -29,6 +29,7 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap // Let's get the data from the form and validate them. var ( inputPaths []string + password string landscape bool nativePageRanges string exportFormFields bool @@ -59,6 +60,7 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap err := ctx.FormData(). MandatoryPaths(libreOffice.Extensions(), &inputPaths). + String("password", &password, defaultOptions.Password). Bool("landscape", &landscape, defaultOptions.Landscape). String("nativePageRanges", &nativePageRanges, defaultOptions.PageRanges). Bool("exportFormFields", &exportFormFields, defaultOptions.ExportFormFields). @@ -146,6 +148,7 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap for i, inputPath := range inputPaths { outputPaths[i] = ctx.GeneratePath(".pdf") options := libreofficeapi.Options{ + Password: password, Landscape: landscape, PageRanges: nativePageRanges, ExportFormFields: exportFormFields, @@ -185,10 +188,17 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap ) } - if errors.Is(err, libreofficeapi.ErrMalformedPageRanges) { + if errors.Is(err, libreofficeapi.ErrUnoException) { return api.WrapError( fmt.Errorf("convert to PDF: %w", err), - api.NewSentinelHttpError(http.StatusBadRequest, fmt.Sprintf("Malformed page ranges '%s' (nativePageRanges)", options.PageRanges)), + api.NewSentinelHttpError(http.StatusBadRequest, fmt.Sprintf("LibreOffice failed to process the document: possible causes include malformed page ranges '%s' (nativePageRanges) or the document might not be password-protected, but the exact cause is uncertain", options.PageRanges)), + ) + } + + if errors.Is(err, libreofficeapi.ErrRuntimeException) { + return api.WrapError( + fmt.Errorf("convert to PDF: %w", err), + api.NewSentinelHttpError(http.StatusBadRequest, "LibreOffice failed to process a document: a password may be invalid or required, but the exact cause is uncertain"), ) } diff --git a/pkg/modules/libreoffice/routes_test.go b/pkg/modules/libreoffice/routes_test.go index 56af4734..041e4165 100644 --- a/pkg/modules/libreoffice/routes_test.go +++ b/pkg/modules/libreoffice/routes_test.go @@ -194,14 +194,14 @@ func TestConvertRoute(t *testing.T) { expectOutputPathsCount: 0, }, { - scenario: "ErrMalformedPageRanges", + scenario: "ErrUnoException", ctx: func() *api.ContextMock { ctx := &api.ContextMock{Context: new(api.Context)} ctx.SetFiles(map[string]string{ "document.docx": "/document.docx", }) ctx.SetValues(map[string][]string{ - "pdfa": { + "nativePageRanges": { "foo", }, }) @@ -209,7 +209,34 @@ func TestConvertRoute(t *testing.T) { }(), libreOffice: &libreofficeapi.ApiMock{ PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { - return libreofficeapi.ErrMalformedPageRanges + return libreofficeapi.ErrUnoException + }, + ExtensionsMock: func() []string { + return []string{".docx"} + }, + }, + expectError: true, + expectHttpError: true, + expectHttpStatus: http.StatusBadRequest, + expectOutputPathsCount: 0, + }, + { + scenario: "ErrRuntimeException", + ctx: func() *api.ContextMock { + ctx := &api.ContextMock{Context: new(api.Context)} + ctx.SetFiles(map[string]string{ + "document.docx": "/document.docx", + }) + ctx.SetValues(map[string][]string{ + "password": { + "invalid", + }, + }) + return ctx + }(), + libreOffice: &libreofficeapi.ApiMock{ + PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { + return libreofficeapi.ErrRuntimeException }, ExtensionsMock: func() []string { return []string{".docx"} diff --git a/test/testdata/libreoffice/protected.docx b/test/testdata/libreoffice/protected.docx new file mode 100644 index 00000000..840d00d5 Binary files /dev/null and b/test/testdata/libreoffice/protected.docx differ