fix: stage zip downloads to temp file and improve file serving headers
Write zip to a temporary file before serving to enable correct content-length, range requests, and proper cache-control headers. Additionally, handle negative object sizes by falling back to file metadata for content-length.
This commit is contained in:
@@ -626,6 +626,7 @@ func (a *App) serveFileContent(w http.ResponseWriter, r *http.Request, box servi
|
||||
defer object.Body.Close()
|
||||
|
||||
w.Header().Set("Content-Type", file.ContentType)
|
||||
w.Header().Set("Cache-Control", "no-transform")
|
||||
disposition := "inline"
|
||||
if attachment {
|
||||
disposition = "attachment"
|
||||
@@ -634,8 +635,12 @@ func (a *App) serveFileContent(w http.ResponseWriter, r *http.Request, box servi
|
||||
if seeker, ok := object.Body.(io.ReadSeeker); ok {
|
||||
http.ServeContent(w, r, file.Name, object.ModTime, seeker)
|
||||
} else {
|
||||
if object.Size > 0 {
|
||||
w.Header().Set("Content-Length", fmt.Sprintf("%d", object.Size))
|
||||
size := object.Size
|
||||
if size < 0 {
|
||||
size = file.Size
|
||||
}
|
||||
if size >= 0 {
|
||||
w.Header().Set("Content-Length", fmt.Sprintf("%d", size))
|
||||
}
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = io.Copy(w, object.Body)
|
||||
@@ -722,14 +727,44 @@ func (a *App) DownloadZip(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
w.Header().Set("Content-Type", "application/zip")
|
||||
w.Header().Set("Content-Disposition", contentDisposition("attachment", "warpbox-"+box.ID+".zip"))
|
||||
w.Header().Set("Last-Modified", time.Now().UTC().Format(http.TimeFormat))
|
||||
|
||||
if err := a.uploadService.WriteZip(w, box); err != nil {
|
||||
a.logger.Error("zip download failed", "source", "download", "severity", "error", "code", 5002, "box_id", box.ID, "error", err.Error())
|
||||
tempDir := filepath.Join(a.cfg.DataDir, "tmp", "downloads")
|
||||
if err := os.MkdirAll(tempDir, 0o700); err != nil {
|
||||
a.logger.Error("zip staging directory creation failed", "source", "download", "severity", "error", "code", 5002, "box_id", box.ID, "error", err.Error())
|
||||
http.Error(w, "failed to prepare zip download", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
archive, err := os.CreateTemp(tempDir, "warpbox-*.zip")
|
||||
if err != nil {
|
||||
a.logger.Error("zip staging file creation failed", "source", "download", "severity", "error", "code", 5002, "box_id", box.ID, "error", err.Error())
|
||||
http.Error(w, "failed to prepare zip download", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
archivePath := archive.Name()
|
||||
defer func() {
|
||||
archive.Close()
|
||||
if err := os.Remove(archivePath); err != nil && !errors.Is(err, os.ErrNotExist) {
|
||||
a.logger.Warn("failed to remove staged zip", "source", "download", "severity", "warn", "box_id", box.ID, "error", err.Error())
|
||||
}
|
||||
}()
|
||||
|
||||
if err := a.uploadService.WriteZip(archive, box); err != nil {
|
||||
a.logger.Error("zip download failed", "source", "download", "severity", "error", "code", 5002, "box_id", box.ID, "error", err.Error())
|
||||
http.Error(w, "failed to prepare zip download", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
stat, err := archive.Stat()
|
||||
if err != nil {
|
||||
a.logger.Error("staged zip stat failed", "source", "download", "severity", "error", "code", 5002, "box_id", box.ID, "error", err.Error())
|
||||
http.Error(w, "failed to prepare zip download", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
name := "warpbox-" + box.ID + ".zip"
|
||||
w.Header().Set("Content-Type", "application/zip")
|
||||
w.Header().Set("Cache-Control", "no-transform")
|
||||
w.Header().Set("Content-Disposition", contentDisposition("attachment", name))
|
||||
http.ServeContent(w, r, name, stat.ModTime(), archive)
|
||||
|
||||
if err := a.uploadService.RecordDownload(box.ID); err != nil && !errors.Is(err, os.ErrNotExist) {
|
||||
a.logger.Warn("failed to record zip download", "source", "download", "severity", "warn", "code", 4003, "box_id", box.ID, "error", err.Error())
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"archive/zip"
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
@@ -284,6 +285,40 @@ func TestFileDownloadUsesOriginalFilename(t *testing.T) {
|
||||
if response.Body.String() != "hello" {
|
||||
t.Fatalf("body = %q", response.Body.String())
|
||||
}
|
||||
if got := response.Header().Get("Content-Length"); got != "5" {
|
||||
t.Fatalf("Content-Length = %q, want 5", got)
|
||||
}
|
||||
if got := response.Header().Get("Cache-Control"); got != "no-transform" {
|
||||
t.Fatalf("Cache-Control = %q, want no-transform", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestZipDownloadIncludesExactContentLength(t *testing.T) {
|
||||
app, cleanup := newTestApp(t)
|
||||
defer cleanup()
|
||||
payload := uploadNamedFileThroughApp(t, app, "report.txt", "hello zip")
|
||||
|
||||
request := httptest.NewRequest(http.MethodGet, "/d/"+payload.BoxID+"/zip", nil)
|
||||
request.SetPathValue("boxID", payload.BoxID)
|
||||
response := httptest.NewRecorder()
|
||||
app.DownloadZip(response, request)
|
||||
|
||||
if response.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, body = %s", response.Code, response.Body.String())
|
||||
}
|
||||
if got, want := response.Header().Get("Content-Length"), strconv.Itoa(response.Body.Len()); got != want {
|
||||
t.Fatalf("Content-Length = %q, want %s", got, want)
|
||||
}
|
||||
if got := response.Header().Get("Cache-Control"); got != "no-transform" {
|
||||
t.Fatalf("Cache-Control = %q, want no-transform", got)
|
||||
}
|
||||
archive, err := zip.NewReader(bytes.NewReader(response.Body.Bytes()), int64(response.Body.Len()))
|
||||
if err != nil {
|
||||
t.Fatalf("zip.NewReader returned error: %v", err)
|
||||
}
|
||||
if len(archive.File) != 1 || archive.File[0].Name != "report.txt" {
|
||||
t.Fatalf("unexpected zip files: %+v", archive.File)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInlineFileDownloadKeepsOriginalFilename(t *testing.T) {
|
||||
|
||||
@@ -60,6 +60,9 @@ func shouldSkipGzip(r *http.Request) bool {
|
||||
}
|
||||
|
||||
path := r.URL.Path
|
||||
if strings.HasPrefix(path, "/d/") && (strings.HasSuffix(path, "/zip") || strings.HasSuffix(path, "/download")) {
|
||||
return true
|
||||
}
|
||||
switch ext := strings.ToLower(path[strings.LastIndex(path, ".")+1:]); ext {
|
||||
case "br", "gz", "zip", "7z", "rar", "jpg", "jpeg", "png", "gif", "webp", "avif", "mp4", "webm", "mov", "m4v", "mp3", "ogg", "woff", "woff2", "ttf", "otf":
|
||||
return true
|
||||
|
||||
@@ -61,3 +61,30 @@ func TestGzipSkipsRangeAndHeadRequests(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGzipSkipsDownloadEndpoints(t *testing.T) {
|
||||
handler := Gzip(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Length", "11")
|
||||
_, _ = io.WriteString(w, "hello world")
|
||||
}))
|
||||
|
||||
for _, path := range []string{
|
||||
"/d/box/f/file/download",
|
||||
"/d/box/zip",
|
||||
} {
|
||||
t.Run(path, func(t *testing.T) {
|
||||
request := httptest.NewRequest(http.MethodGet, path, nil)
|
||||
request.Header.Set("Accept-Encoding", "gzip")
|
||||
response := httptest.NewRecorder()
|
||||
|
||||
handler.ServeHTTP(response, request)
|
||||
|
||||
if got := response.Header().Get("Content-Encoding"); got != "" {
|
||||
t.Fatalf("Content-Encoding = %q, want empty", got)
|
||||
}
|
||||
if got := response.Header().Get("Content-Length"); got != "11" {
|
||||
t.Fatalf("Content-Length = %q, want 11", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1028,9 +1028,13 @@ func (s *UploadService) RecordDownload(boxID string) error {
|
||||
})
|
||||
}
|
||||
|
||||
func (s *UploadService) WriteZip(w io.Writer, box Box) error {
|
||||
func (s *UploadService) WriteZip(w io.Writer, box Box) (err error) {
|
||||
archive := zip.NewWriter(w)
|
||||
defer archive.Close()
|
||||
defer func() {
|
||||
if closeErr := archive.Close(); err == nil {
|
||||
err = closeErr
|
||||
}
|
||||
}()
|
||||
|
||||
for _, file := range box.Files {
|
||||
object, err := s.OpenFileObject(context.Background(), box, file)
|
||||
|
||||
Reference in New Issue
Block a user