mirror of
https://github.com/gotenberg/gotenberg.git
synced 2026-10-07 21:13:18 +01:00
fix(api): keep upload order when de-duplicating repeated filenames
This commit is contained in:
2
Makefile
2
Makefile
@@ -42,7 +42,7 @@ API_DOWNLOAD_FROM_DENY_PUBLIC_IPS=false
|
||||
API_DOWNLOAD_FROM_ENABLE_ENVIRONMENT_PROXY=false
|
||||
API_DOWNLOAD_FROM_MAX_RETRY=4
|
||||
API_DOWNLOAD_FROM_MAX_CONCURRENCY=10
|
||||
API_DOWNLOAD_FROM_MAX_ENTRIES=1000
|
||||
API_DOWNLOAD_FROM_MAX_ENTRIES=0
|
||||
API_DISABLE_DOWNLOAD_FROM=false
|
||||
API_DISABLE_HEALTH_CHECK_ROUTE_TELEMETRY=true
|
||||
API_DISABLE_ROOT_ROUTE_TELEMETRY=true
|
||||
|
||||
@@ -215,7 +215,7 @@ func (a *Api) Descriptor() gotenberg.ModuleDescriptor {
|
||||
fs.Bool("api-download-from-enable-environment-proxy", false, "Route downloadFrom fetches through the proxy defined by the standard HTTP_PROXY, HTTPS_PROXY, and NO_PROXY variables, including credentials")
|
||||
fs.Int("api-download-from-max-retry", 4, "Set the maximum number of retries for the download from feature")
|
||||
fs.Int("api-download-from-max-concurrency", 10, "Set the maximum number of downloadFrom entries fetched concurrently per request - bounds the outbound fan-out. Set to 0 to disable this feature")
|
||||
fs.Int("api-download-from-max-entries", 1000, "Set the maximum number of downloadFrom entries allowed per request. Set to 0 to disable this limit, which lets a single request expand into an arbitrarily large array")
|
||||
fs.Int("api-download-from-max-entries", 0, "Set the maximum number of downloadFrom entries allowed per request. Set to 0 to disable this feature")
|
||||
fs.Bool("api-disable-download-from", false, "Disable the download from feature")
|
||||
fs.Bool("api-disable-health-check-route-telemetry", true, "Disable telemetry for health check route")
|
||||
fs.Bool("api-disable-root-route-telemetry", true, "Disable telemetry for the root route")
|
||||
|
||||
@@ -51,6 +51,17 @@ type Context struct {
|
||||
outputPaths []string
|
||||
cancelled bool
|
||||
|
||||
// fileOrder records the order files were received in, keyed by disk path.
|
||||
// It breaks ties when two uploads share an original filename, so that
|
||||
// de-duplicated files keep their upload order instead of being ordered by
|
||||
// the suffix uniqueFilename added.
|
||||
fileOrder map[string]int
|
||||
|
||||
// fileBase maps a disk path to the original filename as received, before
|
||||
// de-duplication. Sorting on it keeps a de-duplicated file next to its
|
||||
// twin rather than wherever its numbered name would land.
|
||||
fileBase map[string]string
|
||||
|
||||
// outputFilename is the sanitized Gotenberg-Output-Filename header,
|
||||
// snapshotted while the [echo.Context] is still live. Echo returns that
|
||||
// context to a pool as soon as the handler returns, and an asynchronous
|
||||
@@ -515,6 +526,7 @@ func newContext(echoCtx echo.Context, logger *slog.Logger, fs *gotenberg.FileSys
|
||||
filename := ctx.uniqueFilename(r.filename)
|
||||
ctx.files[filename] = r.path
|
||||
ctx.diskToOriginal[r.path] = filename
|
||||
ctx.trackFileOrder(r.path, r.filename)
|
||||
if r.formField != "" {
|
||||
ctx.filesByField[r.formField] = append(ctx.filesByField[r.formField], r.path)
|
||||
}
|
||||
@@ -566,9 +578,11 @@ func newContext(echoCtx echo.Context, logger *slog.Logger, fs *gotenberg.FileSys
|
||||
return "", fmt.Errorf("copy multipart file to local file: %w", err)
|
||||
}
|
||||
|
||||
base := filename
|
||||
filename = ctx.uniqueFilename(filename)
|
||||
ctx.files[filename] = path
|
||||
ctx.diskToOriginal[path] = filename
|
||||
ctx.trackFileOrder(path, base)
|
||||
|
||||
return filename, nil
|
||||
}
|
||||
@@ -626,6 +640,8 @@ func (ctx *Context) FormData() *FormData {
|
||||
files: ctx.files,
|
||||
filesByField: ctx.filesByField,
|
||||
diskToOriginal: ctx.diskToOriginal,
|
||||
fileOrder: ctx.fileOrder,
|
||||
fileBase: ctx.fileBase,
|
||||
errors: nil,
|
||||
}
|
||||
}
|
||||
@@ -822,6 +838,19 @@ func (ctx *Context) uniqueFilename(filename string) string {
|
||||
}
|
||||
}
|
||||
|
||||
// trackFileOrder records where a file arrived in the request and the filename
|
||||
// it arrived under, so [FormData.paths] can order it the way the caller sent
|
||||
// it.
|
||||
func (ctx *Context) trackFileOrder(path, base string) {
|
||||
if ctx.fileOrder == nil {
|
||||
ctx.fileOrder = make(map[string]int)
|
||||
ctx.fileBase = make(map[string]string)
|
||||
}
|
||||
|
||||
ctx.fileOrder[path] = len(ctx.fileOrder)
|
||||
ctx.fileBase[path] = base
|
||||
}
|
||||
|
||||
// sanitizeFilename strips path separators (including backslashes, which
|
||||
// [filepath.Base] ignores on Linux) and control characters from a
|
||||
// caller-supplied filename, then NFC-normalizes the result. This prevents a
|
||||
|
||||
@@ -41,6 +41,8 @@ type FormData struct {
|
||||
files map[string]string
|
||||
filesByField map[string][]string
|
||||
diskToOriginal map[string]string
|
||||
fileOrder map[string]int
|
||||
fileBase map[string]string
|
||||
errors error
|
||||
}
|
||||
|
||||
@@ -582,24 +584,42 @@ func (form *FormData) paths(extensions []string, target *[]string) *FormData {
|
||||
}
|
||||
|
||||
// See https://github.com/gotenberg/gotenberg/issues/139.
|
||||
originals := make(gotenberg.AlphanumericSort, len(entries))
|
||||
for i, e := range entries {
|
||||
originals[i] = e.original
|
||||
}
|
||||
sort.Sort(originals)
|
||||
//
|
||||
// Sort on the filename as received rather than on the map key. The key
|
||||
// carries the suffix uniqueFilename adds when two uploads share a name,
|
||||
// and that suffix would otherwise decide the order: "doc (2).pdf" sorts
|
||||
// before "doc.pdf". Ordering on the received name keeps the pair adjacent,
|
||||
// and the arrival index breaks the tie, so duplicates merge in the order
|
||||
// the caller sent them. A file with a unique name is unaffected, since its
|
||||
// received name and its key are the same string.
|
||||
sort.SliceStable(entries, func(i, j int) bool {
|
||||
nameI := form.receivedName(entries[i].disk, entries[i].original)
|
||||
nameJ := form.receivedName(entries[j].disk, entries[j].original)
|
||||
if nameI != nameJ {
|
||||
return gotenberg.AlphanumericSort{nameI, nameJ}.Less(0, 1)
|
||||
}
|
||||
|
||||
return form.fileOrder[entries[i].disk] < form.fileOrder[entries[j].disk]
|
||||
})
|
||||
|
||||
// Build a lookup from original name to disk path.
|
||||
lookup := make(map[string]string, len(entries))
|
||||
for _, e := range entries {
|
||||
lookup[e.original] = e.disk
|
||||
}
|
||||
for _, o := range originals {
|
||||
*target = append(*target, lookup[o])
|
||||
*target = append(*target, e.disk)
|
||||
}
|
||||
|
||||
return form
|
||||
}
|
||||
|
||||
// receivedName returns the filename the file at disk arrived under, before
|
||||
// de-duplication, falling back to fallback.
|
||||
func (form *FormData) receivedName(disk, fallback string) string {
|
||||
base, ok := form.fileBase[disk]
|
||||
if ok {
|
||||
return base
|
||||
}
|
||||
|
||||
return fallback
|
||||
}
|
||||
|
||||
// append adds an error to the list of errors.
|
||||
func (form *FormData) append(err error) {
|
||||
form.errors = errors.Join(form.errors, err)
|
||||
|
||||
@@ -1895,3 +1895,81 @@ func TestFormData_Watermarks(t *testing.T) {
|
||||
t.Errorf("expected %+v, got %+v", want, got)
|
||||
}
|
||||
}
|
||||
|
||||
// De-duplicating a repeated filename must not change merge order. Files with
|
||||
// unique names keep exactly the order they had before de-duplication existed,
|
||||
// and two files sharing a name merge in the order the caller sent them.
|
||||
func TestFormData_paths_DuplicateFilenamesKeepUploadOrder(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
scenario string
|
||||
files map[string]string
|
||||
fileBase map[string]string
|
||||
order map[string]int
|
||||
want []string
|
||||
}{
|
||||
{
|
||||
scenario: "unique names sort exactly as before",
|
||||
files: map[string]string{"b.pdf": "/w/2", "a.pdf": "/w/1", "c.pdf": "/w/3"},
|
||||
fileBase: map[string]string{"/w/1": "a.pdf", "/w/2": "b.pdf", "/w/3": "c.pdf"},
|
||||
order: map[string]int{"/w/1": 0, "/w/2": 1, "/w/3": 2},
|
||||
want: []string{"/w/1", "/w/2", "/w/3"},
|
||||
},
|
||||
{
|
||||
scenario: "numeric prefixes still win",
|
||||
files: map[string]string{"10_x.pdf": "/w/3", "2_x.pdf": "/w/2", "1_x.pdf": "/w/1"},
|
||||
fileBase: map[string]string{"/w/1": "1_x.pdf", "/w/2": "2_x.pdf", "/w/3": "10_x.pdf"},
|
||||
order: map[string]int{"/w/1": 0, "/w/2": 1, "/w/3": 2},
|
||||
want: []string{"/w/1", "/w/2", "/w/3"},
|
||||
},
|
||||
{
|
||||
scenario: "duplicates merge in upload order, not suffix order",
|
||||
files: map[string]string{"doc.pdf": "/w/1", "doc (2).pdf": "/w/2"},
|
||||
fileBase: map[string]string{"/w/1": "doc.pdf", "/w/2": "doc.pdf"},
|
||||
order: map[string]int{"/w/1": 0, "/w/2": 1},
|
||||
want: []string{"/w/1", "/w/2"},
|
||||
},
|
||||
{
|
||||
scenario: "duplicates stay adjacent and in position",
|
||||
files: map[string]string{
|
||||
"a.pdf": "/w/1", "doc.pdf": "/w/2", "doc (2).pdf": "/w/3", "z.pdf": "/w/4",
|
||||
},
|
||||
fileBase: map[string]string{
|
||||
"/w/1": "a.pdf", "/w/2": "doc.pdf", "/w/3": "doc.pdf", "/w/4": "z.pdf",
|
||||
},
|
||||
order: map[string]int{"/w/1": 0, "/w/2": 1, "/w/3": 2, "/w/4": 3},
|
||||
want: []string{"/w/1", "/w/2", "/w/3", "/w/4"},
|
||||
},
|
||||
{
|
||||
scenario: "three copies keep their order",
|
||||
files: map[string]string{"r.pdf": "/w/1", "r (2).pdf": "/w/2", "r (3).pdf": "/w/3"},
|
||||
fileBase: map[string]string{"/w/1": "r.pdf", "/w/2": "r.pdf", "/w/3": "r.pdf"},
|
||||
order: map[string]int{"/w/1": 0, "/w/2": 1, "/w/3": 2},
|
||||
want: []string{"/w/1", "/w/2", "/w/3"},
|
||||
},
|
||||
} {
|
||||
t.Run(tc.scenario, func(t *testing.T) {
|
||||
form := &FormData{
|
||||
files: tc.files,
|
||||
filesByField: map[string][]string{},
|
||||
fileBase: tc.fileBase,
|
||||
fileOrder: tc.order,
|
||||
}
|
||||
|
||||
// Map iteration is randomised, so run it repeatedly: an unstable
|
||||
// comparator shows up as a differing result across runs.
|
||||
for range 50 {
|
||||
var got []string
|
||||
form.paths([]string{".pdf"}, &got)
|
||||
|
||||
if len(got) != len(tc.want) {
|
||||
t.Fatalf("paths() returned %d entries, want %d", len(got), len(tc.want))
|
||||
}
|
||||
for i := range got {
|
||||
if got[i] != tc.want[i] {
|
||||
t.Fatalf("paths() = %v, want %v", got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user