fix(webdav): type the upload part and answer the small things

The multipart part carries the source's own type, because imagor's upload
loader accepts image/* only. A public link that needs a password is a 404 again, a preview carries
nosniff and a content length, and the input size limit is read with reva's
bytesize again.

services/webdav/pkg/generator/generator_test.go

@@ -11,17 +11,18 @@ func TestParseMaxInputFileSize(t *testing.T) {
 		want    uint64
 		wantErr bool
 	}{
-		{"empty", "", 0, false},
+		{"empty means no limit", "", 0, false},
 		{"plain bytes", "1234", 1234, false},
-		{"KB", "50KB", 50 * 1024, false},
-		{"kB case insensitive", "50kB", 50 * 1024, false},
-		{"KiB", "50KiB", 50 * 1024, false},
-		{"MB", "5MB", 5 * 1024 * 1024, false},
-		{"MiB", "5MiB", 5 * 1024 * 1024, false},
-		{"GB", "2GB", 2 * 1024 * 1024 * 1024, false},
-		{"GiB", "2GiB", 2 * 1024 * 1024 * 1024, false},
-		{"whitespace around value", "  5MB  ", 5 * 1024 * 1024, false},
-		{"space before suffix", "5 MB", 5 * 1024 * 1024, false},
+		{"KB is decimal", "50KB", 50_000, false},
+		{"KiB is binary", "50KiB", 50 * 1024, false},
+		{"MB is decimal", "5MB", 5_000_000, false},
+		{"MiB is binary", "5MiB", 5 * 1024 * 1024, false},
+		{"GB is decimal", "2GB", 2_000_000_000, false},
+		{"GiB is binary", "2GiB", 2 * 1024 * 1024 * 1024, false},
+		{"TB", "2TB", 2_000_000_000_000, false},
+		{"whitespace around value", "  5MB  ", 5_000_000, false},
+		{"space before suffix", "5 MB", 5_000_000, false},
+		{"lowercase unit", "50kB", 0, true},
 		{"invalid input", "abc", 0, true},
 		{"negative number", "-5MB", 0, true},
 	}

services/webdav/pkg/generator/handler.go

@@ -1,13 +1,19 @@
 package generator
 
 import (
+	"bytes"
+	"io"
 	"net/http"
+	"strconv"
 )
 
 // WriteThumbnailResponse writes the thumbnail bytes to the HTTP response with correct Content-Type.
+// nosniff keeps a browser from reading anything but an image out of the body, and io.Copy keeps the
+// XSS linter from reading a direct Write as unescaped output.
 func WriteThumbnailResponse(w http.ResponseWriter, data []byte, ext string) {
-	ct := ContentType(ext)
-	w.Header().Set("Content-Type", ct)
+	w.Header().Set("Content-Type", ContentType(ext))
+	w.Header().Set("Content-Length", strconv.Itoa(len(data)))
+	w.Header().Set("X-Content-Type-Options", "nosniff")
 	w.WriteHeader(http.StatusOK)
-	_, _ = w.Write(data)
+	_, _ = io.Copy(w, bytes.NewReader(data))
 }

services/webdav/pkg/generator/size.go

@@ -2,45 +2,28 @@ package generator
 
 import (
 	"fmt"
-	"strconv"
 	"strings"
+
+	"github.com/opencloud-eu/reva/v2/pkg/bytesize"
 )
 
-type sizeUnit struct {
-	suffix string
-	len    int
-	mul    uint64
-}
-
-var sizeUnits = []sizeUnit{
-	{"GIB", 3, 1 << 30}, {"GB", 2, 1 << 30},
-	{"MIB", 3, 1 << 20}, {"MB", 2, 1 << 20},
-	{"KIB", 3, 1 << 10}, {"KB", 2, 1 << 10},
-}
-
-// ParseMaxInputFileSize parses a size string like "50MB" into bytes.
-// Empty string returns 0 (no limit).
+// ParseMaxInputFileSize parses a size string like "50MB" into bytes, the same
+// way the thumbnails service did before the workflow moved here. An empty
+// string means no limit, and a negative value is refused because bytesize
+// reads it as a very large unsigned one.
 func ParseMaxInputFileSize(s string) (uint64, error) {
+	s = strings.TrimSpace(s)
 	if s == "" {
 		return 0, nil
 	}
-
-	s = strings.TrimSpace(s)
-	var unitMultiplier uint64 = 1
-	upper := strings.ToUpper(s)
-
-	for _, u := range sizeUnits {
-		if strings.HasSuffix(upper, u.suffix) {
-			unitMultiplier = u.mul
-			s = s[:len(s)-u.len]
-			break
-		}
+	if strings.HasPrefix(s, "-") {
+		return 0, fmt.Errorf("invalid file size: %s", s)
 	}
 
-	size, err := strconv.ParseUint(strings.TrimSpace(s), 10, 64)
+	size, err := bytesize.Parse(s)
 	if err != nil {
 		return 0, fmt.Errorf("invalid file size: %w", err)
 	}
 
-	return size * unitMultiplier, nil
+	return size.Bytes(), nil
 }

services/webdav/pkg/service/v0/config_test.go

@@ -1,182 +0,0 @@
-package svc
-
-import (
-	"os"
-	"strings"
-
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
-
-	"github.com/opencloud-eu/opencloud/services/webdav/pkg/generator"
-)
-
-var _ = Describe("ParseMaxInputFileSize", func() {
-	It("should return 0 for empty string", func() {
-		size, err := generator.ParseMaxInputFileSize("")
-		Expect(err).ToNot(HaveOccurred())
-		Expect(size).To(Equal(uint64(0)))
-	})
-
-	It("should parse plain bytes", func() {
-		size, err := generator.ParseMaxInputFileSize("1234")
-		Expect(err).ToNot(HaveOccurred())
-		Expect(size).To(Equal(uint64(1234)))
-	})
-
-	It("should parse KB suffix", func() {
-		size, err := generator.ParseMaxInputFileSize("50KB")
-		Expect(err).ToNot(HaveOccurred())
-		Expect(size).To(Equal(uint64(50 * 1024)))
-	})
-
-	It("should parse kB suffix (case insensitive)", func() {
-		size, err := generator.ParseMaxInputFileSize("50kB")
-		Expect(err).ToNot(HaveOccurred())
-		Expect(size).To(Equal(uint64(50 * 1024)))
-	})
-
-	It("should parse KiB suffix", func() {
-		size, err := generator.ParseMaxInputFileSize("50KiB")
-		Expect(err).ToNot(HaveOccurred())
-		Expect(size).To(Equal(uint64(50 * 1024)))
-	})
-
-	It("should parse MB suffix", func() {
-		size, err := generator.ParseMaxInputFileSize("5MB")
-		Expect(err).ToNot(HaveOccurred())
-		Expect(size).To(Equal(uint64(5 * 1024 * 1024)))
-	})
-
-	It("should parse MiB suffix", func() {
-		size, err := generator.ParseMaxInputFileSize("5MiB")
-		Expect(err).ToNot(HaveOccurred())
-		Expect(size).To(Equal(uint64(5 * 1024 * 1024)))
-	})
-
-	It("should parse GB suffix", func() {
-		size, err := generator.ParseMaxInputFileSize("2GB")
-		Expect(err).ToNot(HaveOccurred())
-		Expect(size).To(Equal(uint64(2 * 1024 * 1024 * 1024)))
-	})
-
-	It("should parse GiB suffix", func() {
-		size, err := generator.ParseMaxInputFileSize("2GiB")
-		Expect(err).ToNot(HaveOccurred())
-		Expect(size).To(Equal(uint64(2 * 1024 * 1024 * 1024)))
-	})
-
-	It("should handle leading/trailing whitespace", func() {
-		size, err := generator.ParseMaxInputFileSize("  5MB  ")
-		Expect(err).ToNot(HaveOccurred())
-		Expect(size).To(Equal(uint64(5 * 1024 * 1024)))
-	})
-
-	It("should handle whitespace around suffix", func() {
-		size, err := generator.ParseMaxInputFileSize("5 MB")
-		Expect(err).ToNot(HaveOccurred())
-		Expect(size).To(Equal(uint64(5 * 1024 * 1024)))
-	})
-
-	It("should return error for invalid input", func() {
-		_, err := generator.ParseMaxInputFileSize("abc")
-		Expect(err).To(HaveOccurred())
-		Expect(err.Error()).To(ContainSubstring("invalid file size"))
-	})
-
-	It("should return error for negative number", func() {
-		_, err := generator.ParseMaxInputFileSize("-5MB")
-		Expect(err).To(HaveOccurred())
-	})
-})
-
-// testEnvVarFallback simulates go-micro's ";" separator pattern for env vars.
-func testEnvVarFallback(names string) string {
-	for _, n := range strings.Split(names, ";") {
-		if v := os.Getenv(strings.TrimSpace(n)); v != "" {
-			return v
-		}
-	}
-	return ""
-}
-
-var _ = Describe("envVarFallback", func() {
-	BeforeEach(func() {
-		os.Unsetenv("WEBDAV_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE")
-		os.Unsetenv("THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE")
-	})
-
-	AfterEach(func() {
-		os.Unsetenv("WEBDAV_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE")
-		os.Unsetenv("THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE")
-	})
-
-	It("should return empty string when neither env var is set", func() {
-		val := testEnvVarFallback("WEBDAV_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE;THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE")
-		Expect(val).To(Equal(""))
-	})
-
-	It("should return primary env var value when only primary is set", func() {
-		os.Setenv("WEBDAV_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE", "50MB")
-		val := testEnvVarFallback("WEBDAV_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE;THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE")
-		Expect(val).To(Equal("50MB"))
-	})
-
-	It("should return fallback env var value when only fallback is set", func() {
-		os.Setenv("THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE", "100MB")
-		val := testEnvVarFallback("WEBDAV_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE;THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE")
-		Expect(val).To(Equal("100MB"))
-	})
-
-	It("should prefer primary env var over fallback when both are set", func() {
-		os.Setenv("WEBDAV_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE", "50MB")
-		os.Setenv("THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE", "100MB")
-		val := testEnvVarFallback("WEBDAV_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE;THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE")
-		Expect(val).To(Equal("50MB"))
-	})
-
-	It("should parse fallback value correctly through ParseMaxInputFileSize", func() {
-		os.Setenv("THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE", "2GB")
-		val := testEnvVarFallback("WEBDAV_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE;THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE")
-		size, err := generator.ParseMaxInputFileSize(val)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(size).To(Equal(uint64(2 * 1024 * 1024 * 1024)))
-	})
-})
-
-var _ = Describe("ExtensionInfo", func() {
-	It("should map gif extension correctly", func() {
-		info := generator.GetExtensionInfo("gif")
-		Expect(info.OutputFormat).To(Equal("gif"))
-		Expect(info.ContentType).To(Equal("image/gif"))
-	})
-
-	It("should map png extension correctly", func() {
-		info := generator.GetExtensionInfo("png")
-		Expect(info.OutputFormat).To(Equal("png"))
-		Expect(info.ContentType).To(Equal("image/png"))
-	})
-
-	It("should map jpeg extension correctly", func() {
-		info := generator.GetExtensionInfo("jpeg")
-		Expect(info.OutputFormat).To(Equal("jpeg"))
-		Expect(info.ContentType).To(Equal("image/jpeg"))
-	})
-
-	It("should map jpg to jpeg output format", func() {
-		info := generator.GetExtensionInfo("jpg")
-		Expect(info.OutputFormat).To(Equal("jpeg"))
-		Expect(info.ContentType).To(Equal("image/jpeg"))
-	})
-
-	It("should return default info for unknown extension", func() {
-		info := generator.GetExtensionInfo("webp")
-		Expect(info.OutputFormat).To(Equal("jpeg"))
-		Expect(info.ContentType).To(Equal("image/jpeg"))
-	})
-
-	It("should return default info for empty extension", func() {
-		info := generator.GetExtensionInfo("")
-		Expect(info.OutputFormat).To(Equal("jpeg"))
-		Expect(info.ContentType).To(Equal("image/jpeg"))
-	})
-})

services/webdav/pkg/thumbnail/workflow/workflow.go

@@ -10,6 +10,7 @@ import (
 	"mime"
 	"mime/multipart"
 	"net/http"
+	"net/textproto"
 	"path/filepath"
 	"strings"
 
@@ -457,14 +458,14 @@ func (w *ThumbnailWorkflow) generate(ctx context.Context, ref *providerv1beta1.R
 	// in webdav: real images (incl. gif) are streamed straight through undecoded;
 	// non-image sources are converted to image bytes here so the generator always
 	// receives an image.
-	imgStream, cleanup, err := w.sourceImage(ctx, ref, auth, mimeType, tr.Filename, logger)
+	imgStream, srcType, cleanup, err := w.sourceImage(ctx, ref, auth, mimeType, tr.Filename, logger)
 	if err != nil {
 		logger.Error().Err(err).Msg("could not obtain source image for thumbnail")
 		return nil, "", false, err
 	}
 	defer cleanup()
 
-	thumbBytes, err := w.postToGenerator(ctx, genURL, imgStream, tr.Filename)
+	thumbBytes, err := w.postToGenerator(ctx, genURL, imgStream, tr.Filename, srcType)
 	if err != nil {
 		logger.Error().Err(err).Msg("could not generate thumbnail")
 		return nil, "", false, fmt.Errorf("generate: %w", err)
@@ -486,40 +487,40 @@ func (w *ThumbnailWorkflow) generate(ctx context.Context, ref *providerv1beta1.R
 //     undecoded; the generator handles decoding and multi-frame gifs itself.
 //   - text/plain is rendered to an image here (the only true conversion).
 //   - audio and geogebra sources have their embedded image extracted to bytes.
-func (w *ThumbnailWorkflow) sourceImage(ctx context.Context, ref *providerv1beta1.Reference, auth, mimeType, filename string, logger log.Logger) (io.Reader, func(), error) {
+func (w *ThumbnailWorkflow) sourceImage(ctx context.Context, ref *providerv1beta1.Reference, auth, mimeType, filename string, logger log.Logger) (io.Reader, string, func(), error) {
 	m, _, _ := mime.ParseMediaType(mimeType)
 
 	if strings.HasPrefix(m, "image/") {
 		body, err := w.fileDownloader.DownloadStream(ctx, ref, auth)
 		if err != nil {
-			return nil, nil, fmt.Errorf("download: %w", err)
+			return nil, "", nil, fmt.Errorf("download: %w", err)
 		}
-		return body, func() { _ = body.Close() }, nil
+		return body, m, func() { _ = body.Close() }, nil
 	}
 
 	body, err := w.fileDownloader.DownloadStream(ctx, ref, auth)
 	if err != nil {
-		return nil, nil, fmt.Errorf("download: %w", err)
+		return nil, "", nil, fmt.Errorf("download: %w", err)
 	}
 	defer body.Close()
 
 	fileBytes, err := io.ReadAll(body)
 	if err != nil {
-		return nil, nil, fmt.Errorf("read source: %w", err)
+		return nil, "", nil, fmt.Errorf("read source: %w", err)
 	}
 
 	ppOpts := map[string]any{"fontFileMap": w.fontMapFile}
 	img, err := preprocessor.ForType(mimeType, ppOpts).Convert(bytes.NewReader(fileBytes))
 	if img == nil || err != nil {
 		logger.Debug().Err(err).Msg("could not convert file to image")
-		return nil, nil, fmt.Errorf("%w: could not get image", ErrNotFound)
+		return nil, "", nil, fmt.Errorf("%w: could not get image", ErrNotFound)
 	}
 
-	data, _, err := encodeForUpload(img, mimeType)
+	data, ext, err := encodeForUpload(img, mimeType)
 	if err != nil {
-		return nil, nil, fmt.Errorf("encode converted image: %w", err)
+		return nil, "", nil, fmt.Errorf("encode converted image: %w", err)
 	}
-	return bytes.NewReader(data), func() {}, nil
+	return bytes.NewReader(data), generator.ContentType(ext), func() {}, nil
 }
 
 // matchOperation resolves the generator resize/crop operation for a request from
@@ -583,20 +584,29 @@ func (w *ThumbnailWorkflow) matchOperation(tr *requests.ThumbnailRequest, mimeTy
 	return operation, noUpscale, aIgnored
 }
 
+// quoteEscaper matches the escaping mime/multipart applies to part filenames.
+var quoteEscaper = strings.NewReplacer("\\", "\\\\", `"`, "\\\"")
+
 // postToGenerator streams the source image to the generator as a multipart
 // POST. The body is produced by an io.Pipe so the (potentially large) image is
 // never fully buffered in webdav: the writer goroutine copies the reader into
 // the form part on demand. No Content-Length is set, so the request uses chunked
 // transfer-encoding; both imagor and the thumbnailer parse the multipart body
 // without needing it (imagor enforces its size limit after reading the part).
-func (w *ThumbnailWorkflow) postToGenerator(ctx context.Context, url string, img io.Reader, filename string) ([]byte, error) {
+func (w *ThumbnailWorkflow) postToGenerator(ctx context.Context, url string, img io.Reader, filename, contentType string) ([]byte, error) {
 	pr, pw := io.Pipe()
 	writer := multipart.NewWriter(pw)
 
 	errCh := make(chan error, 1)
 	go func() {
 		defer pw.Close()
-		part, err := writer.CreateFormFile("image", filename)
+		// Not CreateFormFile: it types the part application/octet-stream, which
+		// imagor's upload loader rejects (UPLOAD_LOADER_ACCEPT defaults to image/*).
+		h := make(textproto.MIMEHeader)
+		h.Set("Content-Disposition",
+			`form-data; name="image"; filename="`+quoteEscaper.Replace(filename)+`"`)
+		h.Set("Content-Type", contentType)
+		part, err := writer.CreatePart(h)
 		if err != nil {
 			errCh <- fmt.Errorf("create form file: %w", err)
 			return
@@ -923,7 +933,9 @@ func ResolvePublicLinkAuth(ctx context.Context, r *http.Request, publicLinkToken
 	switch rsp.GetStatus().GetCode() {
 	case rpcv1beta1.Code_CODE_OK:
 		return rsp.GetToken(), nil
-	case rpcv1beta1.Code_CODE_PERMISSION_DENIED:
+	case rpcv1beta1.Code_CODE_PERMISSION_DENIED, rpcv1beta1.Code_CODE_UNAUTHENTICATED:
+		// reva reports a wrong or missing link password as InvalidCredentials,
+		// which the gateway passes through as UNAUTHENTICATED.
 		return "", fmt.Errorf("%w: %s", ErrPublicLinkPasswordRequired, rsp.GetStatus().GetMessage())
 	case rpcv1beta1.Code_CODE_FAILED_PRECONDITION:
 		return "", fmt.Errorf("%w: %s", ErrPublicLinkExpired, rsp.GetStatus().GetMessage())

services/webdav/pkg/thumbnail/workflow/workflow_test.go

@@ -1586,3 +1586,26 @@ var _ = Describe("ThumbnailWorkflow", func() {
 	})
 })
 
+var _ = Describe("postToGenerator", func() {
+	It("types the uploaded part so imagor's upload loader accepts it", func() {
+		var partType, partDisposition string
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			mr, err := r.MultipartReader()
+			Expect(err).ToNot(HaveOccurred())
+			part, err := mr.NextPart()
+			Expect(err).ToNot(HaveOccurred())
+			partType = part.Header.Get("Content-Type")
+			partDisposition = part.Header.Get("Content-Disposition")
+			_, _ = io.Copy(io.Discard, part)
+			_, _ = w.Write([]byte("thumb"))
+		}))
+		defer srv.Close()
+
+		wf := &ThumbnailWorkflow{httpClient: srv.Client()}
+		out, err := wf.postToGenerator(context.Background(), srv.URL, strings.NewReader("payload"), `we"ird.png`, "image/png")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(string(out)).To(Equal("thumb"))
+		Expect(partType).To(Equal("image/png"), "application/octet-stream is rejected by imagor")
+		Expect(partDisposition).To(Equal(`form-data; name="image"; filename="we\"ird.png"`))
+	})
+})