fix(thumbnails): public link thumbnails never generated behind compressed proxy

changelog/unreleased/fix-public-link-thumbnails-behind-compressing-proxy.md

@@ -0,0 +1,18 @@
+Bugfix: Generate public link thumbnails behind a compressing reverse proxy
+
+Public link thumbnails were never generated when a reverse proxy in front of
+OpenCloud compressed responses. The thumbnails service reported "thumbnails:
+image is too large" and the webdav service turned that into a 500, no matter how
+small the image actually was.
+
+A missing Content-Length is not evidence of size: a proxy drops the header
+whenever it compresses or re-chunks a response.
+
+The download is now capped while it is read instead of being rejected up front,
+so images of unknown length are processed and oversized ones are still refused.
+
+Already generated thumbnails were served from the cache and were never affected,
+which is why the problem only showed up for images opened for the first time
+while logged out.
+
+<https://github.com/opencloud-eu/opencloud/issues/3450>

services/thumbnails/pkg/thumbnail/imgsource/webdav.go

@@ -9,7 +9,6 @@ import (
 	_ "image/png"  // Import the png package so that image.Decode can understand pngs
 	"io"
 	"net/http"
-	"strconv"
 
 	"github.com/opencloud-eu/opencloud/services/thumbnails/pkg/config"
 	thumbnailerErrors "github.com/opencloud-eu/opencloud/services/thumbnails/pkg/errors"
@@ -55,21 +54,15 @@ func (s WebDav) Get(ctx context.Context, url string) (io.ReadCloser, error) {
 	}
 
 	if resp.StatusCode != http.StatusOK {
+		resp.Body.Close()
 		return nil, fmt.Errorf("could not get the image \"%s\". Request returned with statuscode %d ", url, resp.StatusCode)
 	}
 
-	contentLength := resp.Header.Get("Content-Length")
-	if contentLength == "" {
-		// no size information - let's assume it is too big
-		return nil, thumbnailerErrors.ErrImageTooLarge
-	}
-	c, err := strconv.ParseUint(contentLength, 10, 64)
-	if err != nil {
-		return nil, errors.Wrapf(err, `could not parse content length of webdav response "%s"`, url)
-	}
-	if c > s.maxImageFileSize {
+	// no size information - stop at maxImageFileSize
+	if resp.ContentLength >= 0 && uint64(resp.ContentLength) > s.maxImageFileSize {
+		resp.Body.Close()
 		return nil, thumbnailerErrors.ErrImageTooLarge
 	}
 
-	return resp.Body, nil
+	return http.MaxBytesReader(nil, resp.Body, int64(s.maxImageFileSize)), nil
 }

services/thumbnails/pkg/thumbnail/imgsource/webdav_test.go

@@ -0,0 +1,66 @@
+package imgsource
+
+import (
+	"compress/gzip"
+	"context"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/opencloud-eu/opencloud/services/thumbnails/pkg/config"
+	thumbnailerErrors "github.com/opencloud-eu/opencloud/services/thumbnails/pkg/errors"
+	"github.com/opencloud-eu/reva/v2/pkg/bytesize"
+)
+
+func gzipHandler(body string) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Encoding", "gzip")
+		gz := gzip.NewWriter(w)
+		defer gz.Close()
+		_, _ = io.WriteString(gz, body)
+	}
+}
+
+func TestWebDavGet(t *testing.T) {
+	const body = "not really an image, but bytes all the same"
+
+	t.Run("gzipped response without Content-Length is downloaded", func(t *testing.T) {
+		srv := httptest.NewServer(gzipHandler(body))
+		defer srv.Close()
+
+		r, err := NewWebDavSource(config.Thumbnail{}, bytesize.MB).Get(context.Background(), srv.URL)
+		require.NoError(t, err)
+		defer r.Close()
+
+		got, err := io.ReadAll(r)
+		require.NoError(t, err)
+		assert.Equal(t, body, string(got))
+	})
+
+	t.Run("Content-Length above the limit is rejected", func(t *testing.T) {
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			_, _ = io.WriteString(w, strings.Repeat("x", 32))
+		}))
+		defer srv.Close()
+
+		_, err := NewWebDavSource(config.Thumbnail{}, bytesize.ByteSize(16)).Get(context.Background(), srv.URL)
+		assert.ErrorIs(t, err, thumbnailerErrors.ErrImageTooLarge)
+	})
+
+	t.Run("length-less response above the limit fails instead of truncating", func(t *testing.T) {
+		srv := httptest.NewServer(gzipHandler(strings.Repeat("x", 32)))
+		defer srv.Close()
+
+		r, err := NewWebDavSource(config.Thumbnail{}, bytesize.ByteSize(16)).Get(context.Background(), srv.URL)
+		require.NoError(t, err)
+		defer r.Close()
+
+		_, err = io.ReadAll(r)
+		assert.Error(t, err)
+	})
+}