From b8926350b9ea4a7cc624bde5a97350c9128a677f Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Thu, 18 Jan 2024 12:00:32 +0100 Subject: [PATCH] fix(libreoffice): wrong HTTP status codes if invalid PDF formats --- pkg/modules/libreoffice/api/api.go | 4 +-- pkg/modules/libreoffice/api/libreoffice.go | 2 +- .../libreoffice/api/libreoffice_test.go | 4 +-- .../libreoffice/pdfengine/pdfengine.go | 2 +- .../libreoffice/pdfengine/pdfengine_test.go | 2 +- pkg/modules/libreoffice/routes.go | 10 +++++++ pkg/modules/libreoffice/routes_test.go | 27 +++++++++++++++++++ 7 files changed, 44 insertions(+), 7 deletions(-) diff --git a/pkg/modules/libreoffice/api/api.go b/pkg/modules/libreoffice/api/api.go index c2c07796..d3b82128 100644 --- a/pkg/modules/libreoffice/api/api.go +++ b/pkg/modules/libreoffice/api/api.go @@ -21,9 +21,9 @@ func init() { } var ( - // ErrInvalidPdfFormat happens if the PDF format option cannot be handled + // ErrInvalidPdfFormats happens if the PDF formats option cannot be handled // by LibreOffice. - ErrInvalidPdfFormat = errors.New("invalid PDF format") + ErrInvalidPdfFormats = errors.New("invalid PDF formats") // ErrMalformedPageRanges happens if the page ranges option cannot be // interpreted by LibreOffice. diff --git a/pkg/modules/libreoffice/api/libreoffice.go b/pkg/modules/libreoffice/api/libreoffice.go index 6f82e0d7..c8a426eb 100644 --- a/pkg/modules/libreoffice/api/libreoffice.go +++ b/pkg/modules/libreoffice/api/libreoffice.go @@ -282,7 +282,7 @@ func (p *libreOfficeProcess) pdf(ctx context.Context, logger *zap.Logger, inputP case gotenberg.PdfA3b: args = append(args, "--export", "SelectPdfVersion=3") default: - return ErrInvalidPdfFormat + return ErrInvalidPdfFormats } if options.PdfFormats.PdfUa { diff --git a/pkg/modules/libreoffice/api/libreoffice_test.go b/pkg/modules/libreoffice/api/libreoffice_test.go index 554b3629..6347072a 100644 --- a/pkg/modules/libreoffice/api/libreoffice_test.go +++ b/pkg/modules/libreoffice/api/libreoffice_test.go @@ -235,7 +235,7 @@ func TestLibreOfficeProcess_pdf(t *testing.T) { expectError: true, }, { - scenario: "ErrInvalidPdfFormat", + scenario: "ErrInvalidPdfFormats", libreOffice: func() libreOffice { p := new(libreOfficeProcess) p.socketPort = 12345 @@ -247,7 +247,7 @@ func TestLibreOfficeProcess_pdf(t *testing.T) { cancelledCtx: false, start: false, expectError: true, - expectedError: ErrInvalidPdfFormat, + expectedError: ErrInvalidPdfFormats, }, { scenario: "ErrMalformedPageRanges", diff --git a/pkg/modules/libreoffice/pdfengine/pdfengine.go b/pkg/modules/libreoffice/pdfengine/pdfengine.go index 9de03718..dbc5e648 100644 --- a/pkg/modules/libreoffice/pdfengine/pdfengine.go +++ b/pkg/modules/libreoffice/pdfengine/pdfengine.go @@ -64,7 +64,7 @@ func (engine *LibreOfficePdfEngine) Convert(ctx context.Context, logger *zap.Log return nil } - if errors.Is(err, api.ErrInvalidPdfFormat) { + if errors.Is(err, api.ErrInvalidPdfFormats) { return fmt.Errorf("convert PDF to '%+v' with LibreOffice: %w", formats, gotenberg.ErrPdfFormatNotSupported) } diff --git a/pkg/modules/libreoffice/pdfengine/pdfengine_test.go b/pkg/modules/libreoffice/pdfengine/pdfengine_test.go index 26d60d9c..6772c796 100644 --- a/pkg/modules/libreoffice/pdfengine/pdfengine_test.go +++ b/pkg/modules/libreoffice/pdfengine/pdfengine_test.go @@ -137,7 +137,7 @@ func TestLibreOfficePdfEngine_Convert(t *testing.T) { scenario: "invalid PDF format", api: &api.ApiMock{ PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options api.Options) error { - return api.ErrInvalidPdfFormat + return api.ErrInvalidPdfFormats }, }, expectError: true, diff --git a/pkg/modules/libreoffice/routes.go b/pkg/modules/libreoffice/routes.go index fe336cf8..177308b4 100644 --- a/pkg/modules/libreoffice/routes.go +++ b/pkg/modules/libreoffice/routes.go @@ -67,6 +67,16 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap err = libreOffice.Pdf(ctx, ctx.Log(), inputPath, outputPaths[i], options) if err != nil { + if errors.Is(err, libreofficeapi.ErrInvalidPdfFormats) { + return api.WrapError( + fmt.Errorf("convert to PDF: %w", err), + api.NewSentinelHttpError( + http.StatusBadRequest, + fmt.Sprintf("A PDF format in '%+v' is not supported", pdfFormats), + ), + ) + } + if errors.Is(err, libreofficeapi.ErrMalformedPageRanges) { return api.WrapError( fmt.Errorf("convert to PDF: %w", err), diff --git a/pkg/modules/libreoffice/routes_test.go b/pkg/modules/libreoffice/routes_test.go index 5e71cc11..6470cc6a 100644 --- a/pkg/modules/libreoffice/routes_test.go +++ b/pkg/modules/libreoffice/routes_test.go @@ -37,6 +37,28 @@ func TestConvertRoute(t *testing.T) { expectHttpStatus: http.StatusBadRequest, expectOutputPathsCount: 0, }, + { + scenario: "ErrPdfFormatNotSupported (nativePdfFormats)", + ctx: func() *api.ContextMock { + ctx := &api.ContextMock{Context: new(api.Context)} + ctx.SetFiles(map[string]string{ + "document.docx": "/document.docx", + }) + return ctx + }(), + libreOffice: &libreofficeapi.ApiMock{ + PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error { + return libreofficeapi.ErrInvalidPdfFormats + }, + ExtensionsMock: func() []string { + return []string{".docx"} + }, + }, + expectError: true, + expectHttpError: true, + expectHttpStatus: http.StatusBadRequest, + expectOutputPathsCount: 0, + }, { scenario: "ErrMalformedPageRanges", ctx: func() *api.ContextMock { @@ -44,6 +66,11 @@ func TestConvertRoute(t *testing.T) { ctx.SetFiles(map[string]string{ "document.docx": "/document.docx", }) + ctx.SetValues(map[string][]string{ + "pdfa": { + "foo", + }, + }) return ctx }(), libreOffice: &libreofficeapi.ApiMock{