fix: file rename now correctly works for zip archive

pkg/gotenberg/fs.go

@@ -51,3 +51,11 @@ func (fs *FileSystem) MkdirAll() (string, error) {
 
 	return path, nil
 }
+
+// PathRename defines the method signature for renaming files. Implement this
+// interface if you don't want to rely on [os.Rename], notably for testing
+// purpose.
+type PathRename interface {
+	// Rename uses the same signature as [os.Rename].
+	Rename(oldpath, newpath string) error
+}

pkg/gotenberg/mocks.go

@@ -127,6 +127,15 @@ func (provider *MetricsProviderMock) Metrics() ([]Metric, error) {
 	return provider.MetricsMock()
 }
 
+// PathRenameMock is a mock for the [PathRename] interface.
+type PathRenameMock struct {
+	RenameMock func(oldpath, newpath string) error
+}
+
+func (rename *PathRenameMock) Rename(oldpath, newpath string) error {
+	return rename.RenameMock(oldpath, newpath)
+}
+
 // Interface guards.
 var (
 	_ Module            = (*ModuleMock)(nil)

pkg/gotenberg/mocks_test.go

@@ -188,3 +188,16 @@ func TestMetricsProviderMock(t *testing.T) {
 		t.Errorf("expected no error from MetricsProviderMock.Metrics, but got: %v", err)
 	}
 }
+
+func TestPathRenameMock(t *testing.T) {
+	mock := &PathRenameMock{
+		RenameMock: func(oldpath, newpath string) error {
+			return nil
+		},
+	}
+
+	err := mock.Rename("", "")
+	if err != nil {
+		t.Errorf("expected no error from PathRenameMock.Rename, but got: %v", err)
+	}
+}

pkg/modules/api/context.go

@@ -34,18 +34,24 @@ var (
 
 // Context is the request context for a "multipart/form-data" requests.
 type Context struct {
-	dirPath string
-	values  map[string][]string
-	files   map[string]string
-
+	dirPath     string
+	values      map[string][]string
+	files       map[string]string
 	outputPaths []string
+	cancelled   bool
 
-	cancelled bool
-	logger    *zap.Logger
-	echoCtx   echo.Context
+	logger     *zap.Logger
+	echoCtx    echo.Context
+	pathRename gotenberg.PathRename
 	context.Context
 }
 
+type osPathRename struct{}
+
+func (o *osPathRename) Rename(oldpath, newpath string) error {
+	return os.Rename(oldpath, newpath)
+}
+
 // newContext returns a [Context] by parsing a "multipart/form-data" request.
 func newContext(echoCtx echo.Context, logger *zap.Logger, fs *gotenberg.FileSystem, timeout time.Duration) (*Context, context.CancelFunc, error) {
 	processCtx, processCancel := context.WithTimeout(context.Background(), timeout)
@@ -55,6 +61,7 @@ func newContext(echoCtx echo.Context, logger *zap.Logger, fs *gotenberg.FileSyst
 		cancelled:   false,
 		logger:      logger,
 		echoCtx:     echoCtx,
+		pathRename:  new(osPathRename),
 		Context:     processCtx,
 	}
 
@@ -191,14 +198,19 @@ func (ctx *Context) FormData() *FormData {
 }
 
 // GeneratePath generates a path within the context's working directory.
-// It either generates a new UUID-based filename or uses the provided filename.
-// It does not create a file.
-func (ctx *Context) GeneratePath(filename, extension string) string {
-	if filename == "" {
-		// Generate a new UUID-based filename
-		filename = uuid.New().String()
+// It generates a new UUID-based filename. It does not create a file.
+func (ctx *Context) GeneratePath(extension string) string {
+	return fmt.Sprintf("%s/%s%s", ctx.dirPath, uuid.New().String(), extension)
+}
+
+// Rename is just a wrapper around [os.Rename], as we need to mock this
+// behavior in our tests.
+func (ctx *Context) Rename(oldpath, newpath string) error {
+	err := ctx.pathRename.Rename(oldpath, newpath)
+	if err != nil {
+		return fmt.Errorf("rename path: %w", err)
 	}
-	return fmt.Sprintf("%s/%s%s", ctx.dirPath, filename, extension)
+	return nil
 }
 
 // AddOutputPaths adds the given paths. Those paths will be used later to build
@@ -250,7 +262,7 @@ func (ctx *Context) BuildOutputFile() (string, error) {
 		ImplicitTopLevelFolder: false,
 	}
 
-	archivePath := ctx.GeneratePath("", ".zip")
+	archivePath := ctx.GeneratePath(".zip")
 
 	err := z.Archive(ctx.outputPaths, archivePath)
 	if err != nil {
@@ -273,3 +285,8 @@ func (ctx *Context) OutputFilename(outputPath string) string {
 
 	return fmt.Sprintf("%s%s", filename, filepath.Ext(outputPath))
 }
+
+// Interface guard.
+var (
+	_ gotenberg.PathRename = (*osPathRename)(nil)
+)

pkg/modules/api/context_test.go

@@ -3,21 +3,77 @@ package api
 import (
 	"bytes"
 	"errors"
+	"fmt"
+	"io"
 	"mime/multipart"
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"path/filepath"
 	"reflect"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/google/uuid"
 	"github.com/labstack/echo/v4"
 	"go.uber.org/zap"
 
 	"github.com/gotenberg/gotenberg/v8/pkg/gotenberg"
 )
 
+func TestOsPathRename_Rename(t *testing.T) {
+	dirPath, err := gotenberg.NewFileSystem().MkdirAll()
+	if err != nil {
+		t.Fatalf("create working directory: %v", err)
+	}
+
+	path := "/tests/test/testdata/api/sample1.txt"
+	copyPath := filepath.Join(dirPath, fmt.Sprintf("%s.txt", uuid.NewString()))
+
+	in, err := os.Open(path)
+	if err != nil {
+		t.Fatalf("open file: %v", err)
+	}
+
+	defer func() {
+		err := in.Close()
+		if err != nil {
+			t.Fatalf("close file: %v", err)
+		}
+	}()
+
+	out, err := os.Create(copyPath)
+	if err != nil {
+		t.Fatalf("create new file: %v", err)
+	}
+
+	defer func() {
+		err := out.Close()
+		if err != nil {
+			t.Fatalf("close new file: %v", err)
+		}
+	}()
+
+	_, err = io.Copy(out, in)
+	if err != nil {
+		t.Fatalf("copy file to new file: %v", err)
+	}
+
+	rename := new(osPathRename)
+	newPath := filepath.Join(dirPath, fmt.Sprintf("%s.txt", uuid.NewString()))
+
+	err = rename.Rename(copyPath, newPath)
+	if err != nil {
+		t.Errorf("expected no error but got: %v", err)
+	}
+
+	err = os.RemoveAll(dirPath)
+	if err != nil {
+		t.Fatalf("remove working directory: %v", err)
+	}
+}
+
 func TestNewContext(t *testing.T) {
 	for _, tc := range []struct {
 		scenario         string
@@ -190,14 +246,44 @@ func TestContext_GeneratePath(t *testing.T) {
 		dirPath: "/foo",
 	}
 
-	path := ctx.GeneratePath("", ".pdf")
+	path := ctx.GeneratePath(".pdf")
 	if !strings.HasPrefix(path, ctx.dirPath) {
 		t.Errorf("expected '%s' to start with '%s'", path, ctx.dirPath)
 	}
+}
 
-	path = ctx.GeneratePath("foo.txt", ".pdf")
-	if !strings.Contains(path, "foo.txt.pdf") {
-		t.Errorf("expected '%s' to start with '%s'", path, ctx.dirPath)
+func TestContext_Rename(t *testing.T) {
+	for _, tc := range []struct {
+		scenario    string
+		ctx         *Context
+		expectError bool
+	}{
+		{
+			scenario: "failure",
+			ctx: &Context{pathRename: &gotenberg.PathRenameMock{RenameMock: func(oldpath, newpath string) error {
+				return errors.New("cannot rename")
+			}}},
+			expectError: true,
+		},
+		{
+			scenario: "success",
+			ctx: &Context{pathRename: &gotenberg.PathRenameMock{RenameMock: func(oldpath, newpath string) error {
+				return nil
+			}}},
+			expectError: false,
+		},
+	} {
+		t.Run(tc.scenario, func(t *testing.T) {
+			err := tc.ctx.Rename("", "")
+
+			if tc.expectError && err == nil {
+				t.Fatal("expected error but got none", err)
+			}
+
+			if !tc.expectError && err != nil {
+				t.Fatalf("expected no error but got: %v", err)
+			}
+		})
 	}
 }
 

pkg/modules/api/mocks.go

@@ -4,6 +4,8 @@ import (
 	"github.com/alexliesenfeld/health"
 	"github.com/labstack/echo/v4"
 	"go.uber.org/zap"
+
+	"github.com/gotenberg/gotenberg/v8/pkg/gotenberg"
 )
 
 // ContextMock is a helper for tests.
@@ -76,7 +78,7 @@ func (ctx *ContextMock) SetLogger(logger *zap.Logger) {
 	ctx.logger = logger
 }
 
-// SetEchoContext sets the echo.Context.
+// SetEchoContext sets the [echo.Context].
 //
 //	ctx := &api.ContextMock{Context: &api.Context{}}
 //	ctx.setEchoContext(c)
@@ -84,6 +86,14 @@ func (ctx *ContextMock) SetEchoContext(c echo.Context) {
 	ctx.Context.echoCtx = c
 }
 
+// SetPathRename sets the [gotenberg.PathRename].
+//
+//	ctx := &api.ContextMock{Context: &api.Context{}}
+//	ctx.setPathRename(rename)
+func (ctx *ContextMock) SetPathRename(rename gotenberg.PathRename) {
+	ctx.Context.pathRename = rename
+}
+
 // RouterMock is a mock for the [Router] interface.
 type RouterMock struct {
 	RoutesMock func() ([]Route, error)

pkg/modules/api/mocks_test.go

@@ -117,6 +117,19 @@ func TestContextMock_SetEchoContext(t *testing.T) {
 	}
 }
 
+func TestContextMock_SetPathRename(t *testing.T) {
+	mock := ContextMock{&Context{}}
+
+	expect := new(osPathRename)
+	mock.SetPathRename(expect)
+
+	actual := mock.pathRename
+
+	if actual != expect {
+		t.Errorf("expected %v but got %v", expect, actual)
+	}
+}
+
 func TestRouterMock(t *testing.T) {
 	mock := &RouterMock{
 		RoutesMock: func() ([]Route, error) {

pkg/modules/chromium/routes.go

@@ -496,7 +496,7 @@ func markdownToHtml(ctx *api.Context, inputPath string, markdownPaths []string)
 		)
 	}
 
-	inputPath = ctx.GeneratePath("", ".html")
+	inputPath = ctx.GeneratePath(".html")
 
 	err = os.WriteFile(inputPath, buffer.Bytes(), 0o600)
 	if err != nil {
@@ -507,7 +507,7 @@ func markdownToHtml(ctx *api.Context, inputPath string, markdownPaths []string)
 }
 
 func convertUrl(ctx *api.Context, chromium Api, engine gotenberg.PdfEngine, url string, pdfFormats gotenberg.PdfFormats, options PdfOptions) error {
-	outputPath := ctx.GeneratePath("", ".pdf")
+	outputPath := ctx.GeneratePath(".pdf")
 
 	err := chromium.Pdf(ctx, ctx.Log(), url, outputPath, options)
 	err = handleChromiumError(err, options.Options)
@@ -551,7 +551,7 @@ func convertUrl(ctx *api.Context, chromium Api, engine gotenberg.PdfEngine, url
 	zeroValued := gotenberg.PdfFormats{}
 	if pdfFormats != zeroValued {
 		convertInputPath := outputPath
-		convertOutputPath := ctx.GeneratePath("", ".pdf")
+		convertOutputPath := ctx.GeneratePath(".pdf")
 
 		err = engine.Convert(ctx, ctx.Log(), pdfFormats, convertInputPath, convertOutputPath)
 		if err != nil {
@@ -572,7 +572,7 @@ func convertUrl(ctx *api.Context, chromium Api, engine gotenberg.PdfEngine, url
 
 func screenshotUrl(ctx *api.Context, chromium Api, url string, options ScreenshotOptions) error {
 	ext := fmt.Sprintf(".%s", options.Format)
-	outputPath := ctx.GeneratePath("", ext)
+	outputPath := ctx.GeneratePath(ext)
 
 	err := chromium.Screenshot(ctx, ctx.Log(), url, outputPath, options)
 	err = handleChromiumError(err, options.Options)

pkg/modules/libreoffice/routes.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"path/filepath"
 
 	"github.com/labstack/echo/v4"
 
@@ -55,8 +54,7 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap
 			// Alright, let's convert each document to PDF.
 			outputPaths := make([]string, len(inputPaths))
 			for i, inputPath := range inputPaths {
-				// document.docx -> document.docx.pdf.
-				outputPaths[i] = ctx.GeneratePath(filepath.Base(inputPath), ".pdf")
+				outputPaths[i] = ctx.GeneratePath(".pdf")
 				options := libreofficeapi.Options{
 					Landscape:  landscape,
 					PageRanges: nativePageRanges,
@@ -91,9 +89,8 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap
 
 			// So far so good, let's check if we have to merge the PDFs. Quick
 			// win: if there is only one PDF, skip this step.
-
 			if len(outputPaths) > 1 && merge {
-				outputPath := ctx.GeneratePath("", ".pdf")
+				outputPath := ctx.GeneratePath(".pdf")
 
 				err = engine.Merge(ctx, ctx.Log(), outputPaths, outputPath)
 				if err != nil {
@@ -105,7 +102,7 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap
 				zeroValued := gotenberg.PdfFormats{}
 				if !nativePdfFormats && pdfFormats != zeroValued {
 					convertInputPath := outputPath
-					convertOutputPath := ctx.GeneratePath("", ".pdf")
+					convertOutputPath := ctx.GeneratePath(".pdf")
 
 					err = engine.Convert(ctx, ctx.Log(), pdfFormats, convertInputPath, convertOutputPath)
 					if err != nil {
@@ -117,8 +114,7 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap
 				}
 
 				// Last but not least, add the output path to the context so that
-				// the Uno is able to send it as a response to the client.
-
+				// the API is able to send it as a response to the client.
 				err = ctx.AddOutputPaths(outputPath)
 				if err != nil {
 					return fmt.Errorf("add output path: %w", err)
@@ -135,23 +131,34 @@ func convertRoute(libreOffice libreofficeapi.Uno, engine gotenberg.PdfEngine) ap
 
 				for i, outputPath := range outputPaths {
 					convertInputPath := outputPath
-					// document.docx -> document.docx.pdf.
-					convertOutputPaths[i] = ctx.GeneratePath(filepath.Base(inputPaths[i]), ".pdf")
+					convertOutputPaths[i] = ctx.GeneratePath(".pdf")
 
 					err = engine.Convert(ctx, ctx.Log(), pdfFormats, convertInputPath, convertOutputPaths[i])
 					if err != nil {
 						return fmt.Errorf("convert PDF: %w", err)
 					}
-
 				}
 
 				// Important: the output paths are now the converted files.
 				outputPaths = convertOutputPaths
 			}
 
-			// Last but not least, add the output paths to the context so that
-			// the Uno is able to send them as a response to the client.
+			if len(outputPaths) > 1 {
+				// If .zip archive, document.docx -> document.docx.pdf.
+				for i, inputPath := range inputPaths {
+					outputPath := fmt.Sprintf("%s.pdf", inputPath)
 
+					err = ctx.Rename(outputPaths[i], outputPath)
+					if err != nil {
+						return fmt.Errorf("rename output path: %w", err)
+					}
+
+					outputPaths[i] = outputPath
+				}
+			}
+
+			// Last but not least, add the output paths to the context so that
+			// the API is able to send them as a response to the client.
 			err = ctx.AddOutputPaths(outputPaths...)
 			if err != nil {
 				return fmt.Errorf("add output paths: %w", err)

pkg/modules/libreoffice/routes_test.go

@@ -185,7 +185,32 @@ func TestConvertRoute(t *testing.T) {
 			expectError:            false,
 			expectHttpError:        false,
 			expectOutputPathsCount: 1,
-			expectOutputPaths:      []string{"/document.docx.pdf"},
+		},
+		{
+			scenario: "cannot rename many files",
+			ctx: func() *api.ContextMock {
+				ctx := &api.ContextMock{Context: new(api.Context)}
+				ctx.SetFiles(map[string]string{
+					"document.docx":  "/document.docx",
+					"document2.docx": "/document2.docx",
+					"document2.doc":  "/document2.doc",
+				})
+				ctx.SetPathRename(&gotenberg.PathRenameMock{RenameMock: func(oldpath, newpath string) error {
+					return errors.New("cannot rename")
+				}})
+				return ctx
+			}(),
+			libreOffice: &libreofficeapi.ApiMock{
+				PdfMock: func(ctx context.Context, logger *zap.Logger, inputPath, outputPath string, options libreofficeapi.Options) error {
+					return nil
+				},
+				ExtensionsMock: func() []string {
+					return []string{".docx", ".doc"}
+				},
+			},
+			expectError:            true,
+			expectHttpError:        false,
+			expectOutputPathsCount: 0,
 		},
 		{
 			scenario: "success (many files)",
@@ -196,6 +221,9 @@ func TestConvertRoute(t *testing.T) {
 					"document2.docx": "/document2.docx",
 					"document2.doc":  "/document2.doc",
 				})
+				ctx.SetPathRename(&gotenberg.PathRenameMock{RenameMock: func(oldpath, newpath string) error {
+					return nil
+				}})
 				return ctx
 			}(),
 			libreOffice: &libreofficeapi.ApiMock{
@@ -230,6 +258,9 @@ func TestConvertRoute(t *testing.T) {
 						"false",
 					},
 				})
+				ctx.SetPathRename(&gotenberg.PathRenameMock{RenameMock: func(oldpath, newpath string) error {
+					return nil
+				}})
 				return ctx
 			}(),
 			libreOffice: &libreofficeapi.ApiMock{
@@ -266,6 +297,9 @@ func TestConvertRoute(t *testing.T) {
 						"true",
 					},
 				})
+				ctx.SetPathRename(&gotenberg.PathRenameMock{RenameMock: func(oldpath, newpath string) error {
+					return nil
+				}})
 				return ctx
 			}(),
 			libreOffice: &libreofficeapi.ApiMock{

pkg/modules/pdfengines/routes.go

@@ -4,8 +4,6 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"path/filepath"
-	"strings"
 
 	"github.com/labstack/echo/v4"
 
@@ -44,8 +42,7 @@ func mergeRoute(engine gotenberg.PdfEngine) api.Route {
 			}
 
 			// Alright, let's merge the PDFs.
-
-			outputPath := ctx.GeneratePath("", ".pdf")
+			outputPath := ctx.GeneratePath(".pdf")
 
 			err = engine.Merge(ctx, ctx.Log(), inputPaths, outputPath)
 			if err != nil {
@@ -58,7 +55,7 @@ func mergeRoute(engine gotenberg.PdfEngine) api.Route {
 			zeroValued := gotenberg.PdfFormats{}
 			if pdfFormats != zeroValued {
 				convertInputPath := outputPath
-				convertOutputPath := ctx.GeneratePath("", ".pdf")
+				convertOutputPath := ctx.GeneratePath(".pdf")
 
 				err = engine.Convert(ctx, ctx.Log(), pdfFormats, convertInputPath, convertOutputPath)
 				if err != nil {
@@ -71,7 +68,6 @@ func mergeRoute(engine gotenberg.PdfEngine) api.Route {
 
 			// Last but not least, add the output path to the context so that
 			// the API is able to send it as a response to the client.
-
 			err = ctx.AddOutputPaths(outputPath)
 			if err != nil {
 				return fmt.Errorf("add output path: %w", err)
@@ -124,26 +120,29 @@ func convertRoute(engine gotenberg.PdfEngine) api.Route {
 				)
 			}
 
-			// Alright, let's convert the PDFs.s
+			// Alright, let's convert the PDFs.
 			outputPaths := make([]string, len(inputPaths))
-
 			for i, inputPath := range inputPaths {
-				if len(outputPaths) > 1 {
-					// If .zip archive, keep the original filenames.
-					outputPaths[i] = ctx.GeneratePath(strings.TrimSuffix(filepath.Base(inputPath), filepath.Ext(inputPath)), ".pdf")
-				} else {
-					outputPaths[i] = ctx.GeneratePath("", ".pdf")
-				}
+				outputPaths[i] = ctx.GeneratePath(".pdf")
 
 				err = engine.Convert(ctx, ctx.Log(), pdfFormats, inputPath, outputPaths[i])
 				if err != nil {
 					return fmt.Errorf("convert PDF: %w", err)
 				}
+
+				if len(outputPaths) > 1 {
+					// If .zip archive, keep the original filename.
+					err = ctx.Rename(outputPaths[i], inputPath)
+					if err != nil {
+						return fmt.Errorf("rename output path: %w", err)
+					}
+
+					outputPaths[i] = inputPath
+				}
 			}
 
 			// Last but not least, add the output paths to the context so that
 			// the API is able to send them as a response to the client.
-
 			err = ctx.AddOutputPaths(outputPaths...)
 			if err != nil {
 				return fmt.Errorf("add output paths: %w", err)

pkg/modules/pdfengines/routes_test.go

@@ -294,6 +294,36 @@ func TestConvertHandler(t *testing.T) {
 			expectHttpError:        false,
 			expectOutputPathsCount: 1,
 		},
+		{
+			scenario: "cannot rename many files",
+			ctx: func() *api.ContextMock {
+				ctx := &api.ContextMock{Context: new(api.Context)}
+				ctx.SetFiles(map[string]string{
+					"file.pdf":  "/file.pdf",
+					"file2.pdf": "/file2.pdf",
+				})
+				ctx.SetValues(map[string][]string{
+					"pdfa": {
+						gotenberg.PdfA1b,
+					},
+					"pdfua": {
+						"true",
+					},
+				})
+				ctx.SetPathRename(&gotenberg.PathRenameMock{RenameMock: func(oldpath, newpath string) error {
+					return errors.New("cannot rename")
+				}})
+				return ctx
+			}(),
+			engine: &gotenberg.PdfEngineMock{
+				ConvertMock: func(ctx context.Context, logger *zap.Logger, formats gotenberg.PdfFormats, inputPath, outputPath string) error {
+					return nil
+				},
+			},
+			expectError:            true,
+			expectHttpError:        false,
+			expectOutputPathsCount: 0,
+		},
 		{
 			scenario: "success with PDF/A & PDF/UA form fields (many files)",
 			ctx: func() *api.ContextMock {
@@ -310,6 +340,9 @@ func TestConvertHandler(t *testing.T) {
 						"true",
 					},
 				})
+				ctx.SetPathRename(&gotenberg.PathRenameMock{RenameMock: func(oldpath, newpath string) error {
+					return nil
+				}})
 				return ctx
 			}(),
 			engine: &gotenberg.PdfEngineMock{