From 51b92da2e57572906bfd413bcb4f7c3ed02dd529 Mon Sep 17 00:00:00 2001 From: Pascal Bleser Date: Fri, 13 Nov 2026 15:20:26 +0100 Subject: [PATCH] fix(graph): refuse space images too large for a thumbnail A space image is only ever shown as a thumbnail. When a file larger than the thumbnail generator accepts was set as the image of a space, the PATCH of the drive succeeded, but no thumbnail could be made of the file, so the space showed no image and nobody got an error. Before setting the image, graph now looks up the size of the file, and refuses a file above the limit with 413 Request Entity Too Large, without changing the space. The limit is the one of the thumbnail generator: graph reads it under the same names as the webdav and thumbnails services (OC_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE and the older THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE, 50MB by default, empty for unlimited), so one setting keeps the three in agreement. GRAPH_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE sets a lower limit for space images alone. The default of 50MB is now one constant, which the graph, webdav and thumbnails services all use. Only the file size is checked: the dimension limits of the generator would need the whole image to be read. The space readme has no limit, and an empty image id is passed on unchecked, as before. Assisted-by: Claude Code:claude-opus-5-5 --- pkg/config/defaults/thumbnails.go | 7 + services/graph/README.md | 69 ++++++ services/graph/pkg/config/config.go | 8 +- .../pkg/config/defaults/defaultconfig.go | 4 + .../graph/pkg/config/parser/parse_test.go | 56 +++++ services/graph/pkg/service/v0/drives.go | 50 +++- .../pkg/service/v0/drives_space_image_test.go | 214 ++++++++++++++++++ services/graph/pkg/service/v0/graph.go | 2 + services/graph/pkg/service/v0/service.go | 10 + services/thumbnails/README.md | 2 +- .../pkg/config/defaults/defaultconfig.go | 3 +- services/webdav/MIGRATION.md | 2 +- .../pkg/config/defaults/defaultconfig.go | 3 +- .../features/apiSpaces/changeSpaces.feature | 8 + 14 files changed, 430 insertions(+), 8 deletions(-) create mode 100644 pkg/config/defaults/thumbnails.go create mode 100644 services/graph/pkg/service/v0/drives_space_image_test.go diff --git a/pkg/config/defaults/thumbnails.go b/pkg/config/defaults/thumbnails.go new file mode 100644 index 0000000000..ca48f8d195 --- /dev/null +++ b/pkg/config/defaults/thumbnails.go @@ -0,0 +1,7 @@ +package defaults + +// ThumbnailsMaxInputImageFileSize is the default of OC_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE, the +// maximum size of an image a thumbnail is made of. The thumbnails, webdav and graph services read +// that setting each, and they must agree on the default too: webdav would otherwise send images the +// thumbnails service refuses, and graph would accept space images no thumbnail is made of. +const ThumbnailsMaxInputImageFileSize = "50MB" diff --git a/services/graph/README.md b/services/graph/README.md index f310aec7f3..ea1876404b 100644 --- a/services/graph/README.md +++ b/services/graph/README.md @@ -55,6 +55,75 @@ is based on the [OData Specification](https://docs.oasis-open.org/odata/odata/v4 See the [Libre Graph API](https://docs.opencloud.eu/swagger/libre-graph-api/#/users/ListUsers) for examples on the filters supported when querying users. +## Space Images + +The image of a space is a file in the space, usually in its `.space` folder. A space manager +uploads the file, and then assigns it with `PATCH /graph/v1.0/drives/{drive-id}`, naming the id of +the file in the `image` entry of `special`: + +```bash +# upload the image into the .space folder of the space; the response header +# OC-FileId holds the id of the file +curl -i -X PUT -H "Authorization: Bearer $TOKEN" --data-binary @image.png \ + "https://cloud.example.com/dav/spaces/$DRIVE_ID/.space/image.png" + +# assign the file as the image of the space +curl -X PATCH -H "Authorization: Bearer $TOKEN" -H "Content-Type: application/json" \ + -d '{"special": [{"specialFolder": {"name": "image"}, "id": "'"$FILE_ID"'"}]}' \ + "https://cloud.example.com/graph/v1.0/drives/$DRIVE_ID" +``` + +Clients only ever show a space image as a thumbnail. A file that is too large for the thumbnail +generator is therefore never shown at all: the space would keep a broken image, and nobody would +get an error. + +So the graph service looks at the size of the file when it is assigned, and refuses a file above +the limit with `413 Request Entity Too Large`, without changing the space: + +```json +{ + "error": { + "code": "invalidRequest", + "message": "the space image is too large: 73400320 bytes, the maximum is 50000000 bytes", + "innererror": { + "date": "2026-11-13T15:04:05Z", + "request-id": "..." + } + } +} +``` + +The space keeps the image it had, as long as that is a different file. A client that overwrites the +file of the current image with the new content has replaced that content already, so the space +then shows no image. + +The limit is the one of the thumbnail generator, which the graph service reads under the same names +as the webdav and thumbnails services: `OC_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE`, `50MB` by default. +An empty value means unlimited. + +Setting the limit there sets it for all three services: + +```bash +# space images, and thumbnails in general, up to 100 MB +OC_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE=100MB +``` + +`GRAPH_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE` sets a limit for space images alone. It only makes sense +to set it lower than the limit of the thumbnail generator: a space image between the two would be +accepted, but not shown. + +```bash +# thumbnails up to 100 MB, but space images only up to 10 MB +OC_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE=100MB +GRAPH_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE=10MB +``` + +Only the file size is checked. The thumbnail generator also has limits for the width and the height +of an image (`THUMBNAILS_MAX_INPUT_WIDTH` and `THUMBNAILS_MAX_INPUT_HEIGHT`), and for the number of +frames of an animation. The graph service can't check those without reading the whole image, so a +small file with too many pixels is still accepted, and still not shown. The space readme has no +size limit. + ## Caching The `graph` service can use a configured store via `GRAPH_CACHE_STORE`. Possible stores are: diff --git a/services/graph/pkg/config/config.go b/services/graph/pkg/config/config.go index 3a9bcfbe68..3de3b92bf0 100644 --- a/services/graph/pkg/config/config.go +++ b/services/graph/pkg/config/config.go @@ -210,8 +210,12 @@ type Store struct { TLSRootCACertificate string `yaml:"tls_root_ca_certificate" env:"OC_PERSISTENT_STORE_TLS_ROOT_CA_CERTIFICATE;GRAPH_STORE_TLS_ROOT_CA_CERTIFICATE" desc:"The root CA certificate used to validate the server's TLS certificate. If provided GRAPH_STORE_TLS_INSECURE will be seen as false." introductionVersion:"7.3.0"` } -// Thumbnails is what this service announces a preview for. The names match the -// ones the webdav service uses for the same list. +// Thumbnails is what this service announces a preview for, and which space +// images it accepts, since a space image is only ever shown as a thumbnail. The +// names match the ones the webdav service uses for the same settings. type Thumbnails struct { AdditionalSourceTypes []string `yaml:"additional_source_types" env:"OC_THUMBNAILS_ADDITIONAL_SOURCE_TYPES;GRAPH_THUMBNAILS_ADDITIONAL_SOURCE_TYPES" desc:"Additional mime types a preview is announced for, the same list the webdav service attempts previews for. The types the build ships are included; this only adds to them." introductionVersion:"9.0.0"` + // read under the same names as by webdav and the thumbnails service, so that + // graph doesn't accept a space image no thumbnail can be made of + MaxInputImageFileSize string `yaml:"max_input_image_file_size" env:"OC_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE;THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE;GRAPH_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE" desc:"The maximum file size of an image that can be set as the image of a space. A larger one is refused with HTTP 413, because no thumbnail can be made of it, and space images are only shown as thumbnails. Usable common abbreviations: [KB, KiB, MB, MiB, GB, GiB], example: 50MB. An empty value means unlimited." introductionVersion:"%%NEXT%%"` } diff --git a/services/graph/pkg/config/defaults/defaultconfig.go b/services/graph/pkg/config/defaults/defaultconfig.go index b7ec0af936..b98a6c9df9 100644 --- a/services/graph/pkg/config/defaults/defaultconfig.go +++ b/services/graph/pkg/config/defaults/defaultconfig.go @@ -4,6 +4,7 @@ import ( "strings" "time" + ocdefaults "github.com/opencloud-eu/opencloud/pkg/config/defaults" "github.com/opencloud-eu/opencloud/pkg/shared" "github.com/opencloud-eu/opencloud/pkg/structs" "github.com/opencloud-eu/opencloud/services/graph/pkg/config" @@ -88,6 +89,9 @@ func DefaultConfig() *config.Config { // 1 minute UsersCacheTTL: 60, }, + Thumbnails: config.Thumbnails{ + MaxInputImageFileSize: ocdefaults.ThumbnailsMaxInputImageFileSize, + }, Identity: config.Identity{ Backend: "ldap", Metrics: config.IdentityMetrics{ diff --git a/services/graph/pkg/config/parser/parse_test.go b/services/graph/pkg/config/parser/parse_test.go index fdb421fec6..222df4be15 100644 --- a/services/graph/pkg/config/parser/parse_test.go +++ b/services/graph/pkg/config/parser/parse_test.go @@ -1,15 +1,71 @@ package parser_test import ( + "errors" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/opencloud-eu/opencloud/pkg/config/envdecode" "github.com/opencloud-eu/opencloud/pkg/shared" "github.com/opencloud-eu/opencloud/services/graph/pkg/config" "github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults" "github.com/opencloud-eu/opencloud/services/graph/pkg/config/parser" + thumbnailsdefaults "github.com/opencloud-eu/opencloud/services/thumbnails/pkg/config/defaults" + thumbnailsparser "github.com/opencloud-eu/opencloud/services/thumbnails/pkg/config/parser" ) +var _ = Describe("the maximum size of a space image", func() { + BeforeEach(func() { + GinkgoT().Setenv("OC_CONFIG_DIR", GinkgoT().TempDir()) + }) + + // only the environment, the way ParseConfig reads it: ParseConfig itself would + // also ask for secrets + decodeEnv := func() *config.Config { + cfg := defaults.DefaultConfig() + if err := envdecode.Decode(cfg); !errors.Is(err, envdecode.ErrNoTargetFieldsAreSet) { + Expect(err).ToNot(HaveOccurred()) + } + return cfg + } + + // graph would otherwise accept space images no thumbnail is made of, or + // refuse ones that would be shown + DescribeTable("is read the way the thumbnails service reads its maximum input file size", + func(env map[string]string, want string) { + for k, v := range env { + GinkgoT().Setenv(k, v) + } + + cfg := decodeEnv() + + generator := thumbnailsdefaults.DefaultConfig() + Expect(thumbnailsparser.ParseConfig(generator)).To(Succeed()) + + Expect(cfg.Thumbnails.MaxInputImageFileSize).To(Equal(want)) + Expect(generator.Thumbnail.MaxInputImageFileSize).To(Equal(want)) + }, + Entry("from the shared name", map[string]string{ + "OC_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE": "100MB", + }, "100MB"), + Entry("from the old name", map[string]string{ + "THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE": "20MB", + }, "20MB"), + Entry("from the old name when both are set", map[string]string{ + "THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE": "20MB", + "OC_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE": "100MB", + }, "20MB"), + ) + + It("can be set lower for graph alone", func() { + GinkgoT().Setenv("OC_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE", "100MB") + GinkgoT().Setenv("GRAPH_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE", "10MB") + + Expect(decodeEnv().Thumbnails.MaxInputImageFileSize).To(Equal("10MB")) + }) +}) + var _ = Describe("Validate", func() { var cfg *config.Config diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go index 8a47487b1d..cf82a562d7 100644 --- a/services/graph/pkg/service/v0/drives.go +++ b/services/graph/pkg/service/v0/drives.go @@ -62,8 +62,37 @@ var ( // ErrForbiddenCharacter is thrown when the spacename contains an invalid character ErrForbiddenCharacter = fmt.Errorf("spacenames must not contain %v", _invalidSpaceNameCharacters) + + // ErrSpaceImageTooLarge is returned when the file set as the image of a space is larger than + // the maximum size of a space image + ErrSpaceImageTooLarge = errors.New("the space image is too large") ) +// checkSpaceImageSize returns ErrSpaceImageTooLarge if the file with the given +// id is larger than the maximum size of a space image. A space image is only +// ever shown as a thumbnail, and no thumbnail is made of a file above that +// size, so such an image would be stored but never shown. +func (g Graph) checkSpaceImageSize(ctx context.Context, gatewayClient gateway.GatewayAPIClient, fileID string) error { + if g.maxSpaceImageSize == 0 { + return nil + } + + rid, err := storagespace.ParseID(fileID) + if err != nil { + return errorcode.New(errorcode.InvalidRequest, fmt.Sprintf("invalid space image id %q: %v", fileID, err)) + } + + res, err := gatewayClient.Stat(ctx, &storageprovider.StatRequest{Ref: &storageprovider.Reference{ResourceId: &rid}}) + if err := errorcode.FromStat(res, err); err != nil { + return err + } + + if size := res.GetInfo().GetSize(); size > g.maxSpaceImageSize { + return fmt.Errorf("%w: %d bytes, the maximum is %d bytes", ErrSpaceImageTooLarge, size, g.maxSpaceImageSize) + } + return nil +} + // GetDrives serves as a factory method that returns the appropriate // http.Handler function based on the specified API version. func (g Graph) GetDrives(version APIVersion) http.HandlerFunc { @@ -576,9 +605,26 @@ func (g Graph) UpdateDrive(w http.ResponseWriter, r *http.Request) { } for _, special := range drive.Special { - if special.Id != nil { - updateSpaceRequest.StorageSpace.Opaque = utils.AppendPlainToOpaque(updateSpaceRequest.StorageSpace.Opaque, *special.SpecialFolder.Name, *special.Id) + // an entry without an id names no file to set, so it changes nothing, as it always did; + // the web UI, for one, sends such an entry after it has deleted the file of the space image + if special.Id == nil { + continue } + + // an empty id names no file, so there is nothing to look at + if special.SpecialFolder.GetName() == SpaceImageSpecialFolderName && *special.Id != "" { + if err := g.checkSpaceImageSize(r.Context(), gatewayClient, *special.Id); err != nil { + log.Warn().Err(err).Str("image", *special.Id).Msg("could not update drive: space image refused") + if errors.Is(err, ErrSpaceImageTooLarge) { + errorcode.InvalidRequest.Render(w, r, http.StatusRequestEntityTooLarge, err.Error()) + return + } + errorcode.RenderError(w, r, err) + return + } + } + + updateSpaceRequest.StorageSpace.Opaque = utils.AppendPlainToOpaque(updateSpaceRequest.StorageSpace.Opaque, *special.SpecialFolder.Name, *special.Id) } if _, ok := drive.GetNameOk(); ok { diff --git a/services/graph/pkg/service/v0/drives_space_image_test.go b/services/graph/pkg/service/v0/drives_space_image_test.go new file mode 100644 index 0000000000..efa66a181c --- /dev/null +++ b/services/graph/pkg/service/v0/drives_space_image_test.go @@ -0,0 +1,214 @@ +package svc_test + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + userprovider "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + cs3rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/go-chi/chi/v5" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + libregraph "github.com/opencloud-eu/libre-graph-api-go" + revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" + "github.com/opencloud-eu/reva/v2/pkg/rgrpc/status" + "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" + "github.com/opencloud-eu/reva/v2/pkg/utils" + cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks" + "github.com/stretchr/testify/mock" + "google.golang.org/grpc" + + "github.com/opencloud-eu/opencloud/pkg/shared" + "github.com/opencloud-eu/opencloud/services/graph/mocks" + "github.com/opencloud-eu/opencloud/services/graph/pkg/config" + "github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults" + service "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" +) + +var _ = Describe("setting the image of a space", func() { + const ( + imageID = "storageid$spaceid!imageid" + readmeID = "storageid$spaceid!readmeid" + ) + + var ( + svc service.Service + gatewayClient *cs3mocks.GatewayAPIClient + cfg *config.Config + ctx context.Context + rr *httptest.ResponseRecorder + ) + + newService := func() (service.Service, error) { + pool.RemoveSelector("GatewaySelector" + "eu.opencloud.api.gateway") + gatewaySelector := pool.GetSelector[gateway.GatewayAPIClient]( + "GatewaySelector", + "eu.opencloud.api.gateway", + func(cc grpc.ClientConnInterface) gateway.GatewayAPIClient { + return gatewayClient + }, + ) + return service.NewService( + service.Config(cfg), + service.WithGatewaySelector(gatewaySelector), + service.EventsPublisher(&mocks.Publisher{}), + service.PermissionService(&mocks.Permissions{}), + ) + } + + // sets the maximum size of a space image, as the configuration does + setMaxSize := func(size string) { + cfg.Thumbnails.MaxInputImageFileSize = size + var err error + svc, err = newService() + Expect(err).ToNot(HaveOccurred()) + } + + // sends a PATCH of the space with the given file as its special folder + patchSpecial := func(folder, fileID string) { + drive := libregraph.NewDriveUpdate() + drive.SetSpecial([]libregraph.DriveItem{{ + Id: libregraph.PtrString(fileID), + SpecialFolder: &libregraph.SpecialFolder{Name: libregraph.PtrString(folder)}, + }}) + body, err := json.Marshal(drive) + Expect(err).ToNot(HaveOccurred()) + + r := httptest.NewRequest(http.MethodPatch, "/graph/v1.0/drives/{driveID}/", bytes.NewBuffer(body)) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("driveID", "storageid$spaceid") + r = r.WithContext(context.WithValue(ctx, chi.RouteCtxKey, rctx)) + svc.UpdateDrive(rr, r) + } + + // the stat of the image file answers with the given size + statReturnsSize := func(size uint64) { + gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{ + Status: status.NewOK(ctx), + Info: &provider.ResourceInfo{Size: size}, + }, nil) + } + + updateSucceeds := func() { + gatewayClient.On("UpdateStorageSpace", mock.Anything, mock.Anything).Return(func(_ context.Context, req *provider.UpdateStorageSpaceRequest, _ ...grpc.CallOption) *provider.UpdateStorageSpaceResponse { + return &provider.UpdateStorageSpaceResponse{ + Status: status.NewOK(ctx), + StorageSpace: req.StorageSpace, + } + }, nil) + // the response describes the updated space, with its quota and special folders + gatewayClient.On("GetQuota", mock.Anything, mock.Anything).Return(&provider.GetQuotaResponse{ + Status: status.NewOK(ctx), + TotalBytes: 500, + }, nil) + gatewayClient.On("GetPath", mock.Anything, mock.Anything).Return(&provider.GetPathResponse{ + Status: status.NewOK(ctx), + Path: "/.space/file", + }, nil) + } + + expectStored := func(folder, fileID string) { + gatewayClient.AssertCalled(GinkgoT(), "UpdateStorageSpace", mock.Anything, mock.MatchedBy(func(req *provider.UpdateStorageSpaceRequest) bool { + return utils.ReadPlainFromOpaque(req.GetStorageSpace().GetOpaque(), folder) == fileID + })) + } + + expectNothingStored := func() { + gatewayClient.AssertNotCalled(GinkgoT(), "UpdateStorageSpace", mock.Anything, mock.Anything) + } + + BeforeEach(func() { + rr = httptest.NewRecorder() + ctx = revactx.ContextSetUser(context.Background(), &userprovider.User{ + Id: &userprovider.UserId{Type: userprovider.UserType_USER_TYPE_PRIMARY, OpaqueId: "testuser"}, + Username: "testuser", + }) + gatewayClient = &cs3mocks.GatewayAPIClient{} + + cfg = defaults.FullDefaultConfig() + cfg.Identity.LDAP.CACert = "" // no LDAP in these tests + cfg.TokenManager.JWTSecret = "loremipsum" + cfg.Commons = &shared.Commons{} + cfg.GRPCClientTLS = &shared.GRPCClientTLS{} + + setMaxSize("1KB") + }) + + It("refuses an image above the maximum size with 413, and keeps the previous one", func() { + updateSucceeds() + statReturnsSize(1001) + + patchSpecial("image", imageID) + + Expect(rr.Code).To(Equal(http.StatusRequestEntityTooLarge)) + expectNothingStored() + }) + + It("accepts an image of exactly the maximum size", func() { + updateSucceeds() + statReturnsSize(1000) // KB is 1000 bytes, KiB would be 1024 + + patchSpecial("image", imageID) + + Expect(rr.Code).To(Equal(http.StatusOK)) + expectStored("image", imageID) + }) + + It("accepts an image of any size when the maximum is empty", func() { + setMaxSize("") + updateSucceeds() + statReturnsSize(1 << 40) + + patchSpecial("image", imageID) + + Expect(rr.Code).To(Equal(http.StatusOK)) + expectStored("image", imageID) + }) + + It("doesn't limit the size of the space readme", func() { + updateSucceeds() + statReturnsSize(1 << 40) + + patchSpecial("readme", readmeID) + + Expect(rr.Code).To(Equal(http.StatusOK)) + expectStored("readme", readmeID) + }) + + It("passes an empty image id on as before, without looking it up", func() { + updateSucceeds() + + patchSpecial("image", "") + + Expect(rr.Code).To(Equal(http.StatusOK)) + gatewayClient.AssertNotCalled(GinkgoT(), "Stat", mock.Anything, mock.Anything) + gatewayClient.AssertCalled(GinkgoT(), "UpdateStorageSpace", mock.Anything, mock.Anything) + }) + + DescribeTable("keeps the previous image when the image file can't be looked up", + func(code cs3rpc.Code, want int) { + updateSucceeds() + gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{ + Status: &cs3rpc.Status{Code: code, Message: "stat failed"}, + }, nil) + + patchSpecial("image", imageID) + + Expect(rr.Code).To(Equal(want)) + expectNothingStored() + }, + Entry("not found", cs3rpc.Code_CODE_NOT_FOUND, http.StatusNotFound), + Entry("internal error", cs3rpc.Code_CODE_INTERNAL, http.StatusInternalServerError), + ) + + It("doesn't start with a maximum size that isn't a size", func() { + cfg.Thumbnails.MaxInputImageFileSize = "fifty megabytes" + _, err := newService() + Expect(err).To(MatchError(ContainSubstring("thumbnails.max_input_image_file_size"))) + }) +}) diff --git a/services/graph/pkg/service/v0/graph.go b/services/graph/pkg/service/v0/graph.go index 097715a472..49ebba5112 100644 --- a/services/graph/pkg/service/v0/graph.go +++ b/services/graph/pkg/service/v0/graph.go @@ -72,6 +72,8 @@ type Graph struct { metrics *metrics.Metrics traceProvider trace.TracerProvider natskv jetstream.KeyValue + // the maximum file size of a space image in bytes, 0 for unlimited + maxSpaceImageSize uint64 } var _ Service = Graph{} // ensure that the Graph struct implements all of the Service interface diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index 9dd651f346..bc54b87d18 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -5,6 +5,7 @@ import ( "net/http" "net/url" "strconv" + "strings" "time" "github.com/go-chi/chi/v5" @@ -14,6 +15,7 @@ import ( "github.com/riandyrn/otelchi" microstore "go-micro.dev/v4/store" + "github.com/opencloud-eu/reva/v2/pkg/bytesize" "github.com/opencloud-eu/reva/v2/pkg/store" "github.com/opencloud-eu/opencloud/pkg/roles" @@ -199,6 +201,14 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx natskv: options.NatsKeyValue, } + if raw := strings.TrimSpace(options.Config.Thumbnails.MaxInputImageFileSize); raw != "" { + size, err := bytesize.Parse(raw) + if err != nil { + return svc, fmt.Errorf("invalid thumbnails.max_input_image_file_size %q: %w", raw, err) + } + svc.maxSpaceImageSize = size.Bytes() + } + if options.PermissionService == nil { grpcClient, err := grpc.NewClient(append(grpc.GetClientOptions(options.Config.GRPCClientTLS), grpc.WithTraceProvider(options.TraceProvider))...) if err != nil { diff --git a/services/thumbnails/README.md b/services/thumbnails/README.md index f0e12e0690..f92929cf1f 100644 --- a/services/thumbnails/README.md +++ b/services/thumbnails/README.md @@ -100,7 +100,7 @@ webdav decides which size is asked for, keeps the thumbnail cache and holds the | `THUMBNAILS_MAX_INPUT_HEIGHT` | Maximum height of a source image, 7680 by default | | `THUMBNAILS_MAX_OUTPUT_WIDTH` | Maximum width of a thumbnail that may be asked for, 2048 by default. A larger box is refused with HTTP 422 | | `THUMBNAILS_MAX_OUTPUT_HEIGHT` | Maximum height of a thumbnail that may be asked for, 2048 by default | -| `THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE` | Maximum size of a posted image, `50MB` by default, empty for unlimited. A larger body is refused with HTTP 413. `OC_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE` sets this and webdav's own limit together, which is what keeps webdav from sending a body this service then refuses | +| `THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE` | Maximum size of a posted image, `50MB` by default, empty for unlimited. A larger body is refused with HTTP 413. `OC_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE` sets this and webdav's own limit together, which is what keeps webdav from sending a body this service then refuses. The graph service reads it too, and refuses a larger file as the image of a space | | `THUMBNAILS_MAX_ANIMATION_FRAMES` | Maximum number of frames an animated image may have, 100 by default, 0 for unlimited. A file with more frames is refused with HTTP 422 before any frame is decoded. The memory a request can ask for is this limit times the maximum dimensions, so lower it when raising those | The source and the thumbnail have ceilings of their own, for different reasons. How large a preview may be is webdav's decision, and its largest configured resolution is what it ever asks for; `THUMBNAILS_MAX_OUTPUT_*` is the net under that for anything else reaching this endpoint, because `fill` and `stretch` enlarge a small source to fill the box. How large a source may be is this service's decision, since it is the one that sees the pixels: webdav knows a file's type, size and checksum, not its dimensions. diff --git a/services/thumbnails/pkg/config/defaults/defaultconfig.go b/services/thumbnails/pkg/config/defaults/defaultconfig.go index 090a3c86dc..20a2452855 100644 --- a/services/thumbnails/pkg/config/defaults/defaultconfig.go +++ b/services/thumbnails/pkg/config/defaults/defaultconfig.go @@ -1,6 +1,7 @@ package defaults import ( + ocdefaults "github.com/opencloud-eu/opencloud/pkg/config/defaults" "github.com/opencloud-eu/opencloud/services/thumbnails/pkg/config" ) @@ -39,7 +40,7 @@ func DefaultConfig() *config.Config { MaxInputHeight: 7680, MaxOutputWidth: 2048, MaxOutputHeight: 2048, - MaxInputImageFileSize: "50MB", + MaxInputImageFileSize: ocdefaults.ThumbnailsMaxInputImageFileSize, MaxAnimationFrames: 100, }, } diff --git a/services/webdav/MIGRATION.md b/services/webdav/MIGRATION.md index 70101aac52..bf18b7690a 100644 --- a/services/webdav/MIGRATION.md +++ b/services/webdav/MIGRATION.md @@ -30,7 +30,7 @@ Nothing has to be set in the single binary. The URL may also point at a real | Old name | Now | | | --- | --- | --- | | `THUMBNAILS_RESOLUTIONS` | `WEBDAV_THUMBNAILS_RESOLUTIONS` | a ladder of edge lengths, not a list of boxes | -| `THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE` | `OC_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE` | read by webdav and the generator, which have to agree; the old name still wins over the new one in both, so drop it when setting the new one | +| `THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE` | `OC_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE` | read by webdav, the generator and graph (for space images), which have to agree; the old name still wins over the new one in all three, so drop it when setting the new one | | `THUMBNAILS_TXT_FONTMAP_FILE` | `WEBDAV_THUMBNAILS_TXT_FONTMAP_FILE` | webdav renders text before it posts | | `THUMBNAILS_FILESYSTEMSTORAGE_ROOT` | `WEBDAV_THUMBNAILS_CACHE_DIR` | the old name is the directory that holds `files/`, and the cache goes into `cache/` next to it; the new one is the directory itself | diff --git a/services/webdav/pkg/config/defaults/defaultconfig.go b/services/webdav/pkg/config/defaults/defaultconfig.go index f5884b6db5..f62e22a0a1 100644 --- a/services/webdav/pkg/config/defaults/defaultconfig.go +++ b/services/webdav/pkg/config/defaults/defaultconfig.go @@ -3,6 +3,7 @@ package defaults import ( "strings" + ocdefaults "github.com/opencloud-eu/opencloud/pkg/config/defaults" "github.com/opencloud-eu/opencloud/pkg/shared" "github.com/opencloud-eu/opencloud/pkg/structs" "github.com/opencloud-eu/opencloud/services/webdav/pkg/config" @@ -46,7 +47,7 @@ func DefaultConfig() *config.Config { Thumbnails: config.Thumbnails{ GeneratorURL: "http://127.0.0.1:9186", GeneratorTimeout: "30s", - MaxInputFileSize: "50MB", + MaxInputFileSize: ocdefaults.ThumbnailsMaxInputImageFileSize, CacheBackend: "file", Resolutions: []string{ "16", "32", "64", "128", "192", "256", "320", "384", "448", diff --git a/tests/acceptance/features/apiSpaces/changeSpaces.feature b/tests/acceptance/features/apiSpaces/changeSpaces.feature index 438c27c1ff..2778890772 100644 --- a/tests/acceptance/features/apiSpaces/changeSpaces.feature +++ b/tests/acceptance/features/apiSpaces/changeSpaces.feature @@ -372,6 +372,14 @@ Feature: Change data of space When user "Bob" sets the file ".space/someImageFile.jpg" as a space image in a special section of the "Project Jupiter" space Then the HTTP status code should be "404" + @env-config + Scenario: user cannot set an image file larger than the maximum size as space image of the space via the Graph API + Given the config "GRAPH_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE" has been set to "10" + And user "Alice" has created a folder ".space" in space "Project Jupiter" + And user "Alice" has uploaded a file inside space "Project Jupiter" with content "nineteen bytes long" to ".space/spaceImage.jpg" + When user "Alice" sets the file ".space/spaceImage.jpg" as a space image in a special section of the "Project Jupiter" space + Then the HTTP status code should be "413" + Scenario Outline: user set new readme file as description of the space via the graph API Given user "Alice" has created a folder ".space" in space "Project Jupiter"