From b04169c3a10ded9886ffb8f150915479aebcbf59 Mon Sep 17 00:00:00 2001 From: ThShepard Date: Wed, 2 Sep 2026 13:59:25 +0200 Subject: [PATCH] fix(thumbnails): public link thumbnails never generated behind compressing reverse proxy --- ...ink-thumbnails-behind-compressing-proxy.md | 18 +++++ .../pkg/thumbnail/imgsource/webdav.go | 17 ++--- .../pkg/thumbnail/imgsource/webdav_test.go | 66 +++++++++++++++++++ 3 files changed, 89 insertions(+), 12 deletions(-) create mode 100644 changelog/unreleased/fix-public-link-thumbnails-behind-compressing-proxy.md create mode 100644 services/thumbnails/pkg/thumbnail/imgsource/webdav_test.go diff --git a/changelog/unreleased/fix-public-link-thumbnails-behind-compressing-proxy.md b/changelog/unreleased/fix-public-link-thumbnails-behind-compressing-proxy.md new file mode 100644 index 0000000000..c6dd4558f7 --- /dev/null +++ b/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. + + diff --git a/services/thumbnails/pkg/thumbnail/imgsource/webdav.go b/services/thumbnails/pkg/thumbnail/imgsource/webdav.go index e7d6cf75c2..0e0769c926 100644 --- a/services/thumbnails/pkg/thumbnail/imgsource/webdav.go +++ b/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 } diff --git a/services/thumbnails/pkg/thumbnail/imgsource/webdav_test.go b/services/thumbnails/pkg/thumbnail/imgsource/webdav_test.go new file mode 100644 index 0000000000..95380c9600 --- /dev/null +++ b/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) + }) +}