diff --git a/changelog/unreleased/predefined-resolutions.md b/changelog/unreleased/predefined-resolutions.md new file mode 100644 index 0000000000..ce8d3e1989 --- /dev/null +++ b/changelog/unreleased/predefined-resolutions.md @@ -0,0 +1,6 @@ +Change: Use predefined resolutions for thumbnail generation + +We implemented predefined resolutions to prevent attacker from flooding the service with a large number of thumbnails. +The requested resolution gets mapped to the closest matching predefined resolution. + +https://github.com/owncloud/ocis-thumbnails/issues/7 diff --git a/pkg/command/server.go b/pkg/command/server.go index af6cafecf8..9967701efa 100644 --- a/pkg/command/server.go +++ b/pkg/command/server.go @@ -29,11 +29,11 @@ func Server(cfg *config.Config) *cli.Command { Usage: "Start integrated server", Flags: flagset.ServerWithConfig(cfg), Before: func(c *cli.Context) error { + cfg.Thumbnail.Resolutions = c.StringSlice("thumbnail-resolution") return nil }, Action: func(c *cli.Context) error { logger := NewLogger(cfg) - if cfg.Tracing.Enabled { switch t := cfg.Tracing.Type; t { case "agent": diff --git a/pkg/config/config.go b/pkg/config/config.go index e84b514b9e..37286518ab 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -33,13 +33,12 @@ type Tracing struct { // Config combines all available configuration parts. type Config struct { - File string - Log Log - Debug Debug - Server Server - Tracing Tracing - FileSystemStorage FileSystemStorage - WebDavSource WebDavSource + File string + Log Log + Debug Debug + Server Server + Tracing Tracing + Thumbnail Thumbnail } // FileSystemStorage defines the available filesystem storage configuration. @@ -52,6 +51,13 @@ type WebDavSource struct { BaseURL string } +// Thumbnail defines the available thumbnail related configuration. +type Thumbnail struct { + Resolutions []string + FileSystemStorage FileSystemStorage + WebDavSource WebDavSource +} + // New initializes a new configuration with or without defaults. func New() *Config { return &Config{} diff --git a/pkg/flagset/flagset.go b/pkg/flagset/flagset.go index 2f34817b1a..60126809c3 100644 --- a/pkg/flagset/flagset.go +++ b/pkg/flagset/flagset.go @@ -142,14 +142,20 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { Value: filepath.Join(os.TempDir(), "ocis-thumbnails/"), Usage: "Root path of the filesystem storage directory", EnvVars: []string{"THUMBNAILS_FILESYSTEMSTORAGE_ROOT"}, - Destination: &cfg.FileSystemStorage.RootDirectory, + Destination: &cfg.Thumbnail.FileSystemStorage.RootDirectory, }, &cli.StringFlag{ Name: "webdavsource-baseurl", Value: "http://localhost:9140/remote.php/webdav/", Usage: "Base url for a webdav api", EnvVars: []string{"THUMBNAILS_WEBDAVSOURCE_BASEURL"}, - Destination: &cfg.WebDavSource.BaseURL, + Destination: &cfg.Thumbnail.WebDavSource.BaseURL, + }, + &cli.StringSliceFlag{ + Name: "thumbnail-resolution", + Value: cli.NewStringSlice("16x16", "32x32", "64x64", "128x128"), + Usage: "--thumbnail-resolution 16x16 [--thumbnail-resolution 32x32]", + EnvVars: []string{"THUMBNAILS_RESOLUTIONS"}, }, } } diff --git a/pkg/server/grpc/server.go b/pkg/server/grpc/server.go index 96c64e1d0c..37a79b755a 100644 --- a/pkg/server/grpc/server.go +++ b/pkg/server/grpc/server.go @@ -24,6 +24,7 @@ func NewService(opts ...Option) grpc.Service { { thumbnail = svc.NewService( svc.Config(options.Config), + svc.Logger(options.Logger), ) thumbnail = svc.NewInstrument(thumbnail, options.Metrics) thumbnail = svc.NewLogging(thumbnail, options.Logger) diff --git a/pkg/service/v0/service.go b/pkg/service/v0/service.go index 3a2f973ace..914968c8dd 100644 --- a/pkg/service/v0/service.go +++ b/pkg/service/v0/service.go @@ -6,25 +6,31 @@ import ( "github.com/owncloud/ocis-pkg/v2/log" v0proto "github.com/owncloud/ocis-thumbnails/pkg/proto/v0" - "github.com/owncloud/ocis-thumbnails/pkg/thumbnails" - "github.com/owncloud/ocis-thumbnails/pkg/thumbnails/imgsource" - "github.com/owncloud/ocis-thumbnails/pkg/thumbnails/storage" + "github.com/owncloud/ocis-thumbnails/pkg/thumbnail" + "github.com/owncloud/ocis-thumbnails/pkg/thumbnail/imgsource" + "github.com/owncloud/ocis-thumbnails/pkg/thumbnail/resolution" + "github.com/owncloud/ocis-thumbnails/pkg/thumbnail/storage" ) // NewService returns a service implementation for Service. func NewService(opts ...Option) v0proto.ThumbnailServiceHandler { options := newOptions(opts...) - + logger := options.Logger + resolutions, err := resolution.New(options.Config.Thumbnail.Resolutions) + if err != nil { + logger.Fatal().Err(err).Msg("resolutions not configured correctly") + } svc := Thumbnail{ - manager: thumbnails.NewSimpleManager( + manager: thumbnail.NewSimpleManager( storage.NewFileSystemStorage( - options.Config.FileSystemStorage, - options.Logger, + options.Config.Thumbnail.FileSystemStorage, + logger, ), - options.Logger, + logger, ), - source: imgsource.NewWebDavSource(options.Config.WebDavSource), - logger: options.Logger, + resolutions: resolutions, + source: imgsource.NewWebDavSource(options.Config.Thumbnail.WebDavSource), + logger: logger, } return svc @@ -32,49 +38,50 @@ func NewService(opts ...Option) v0proto.ThumbnailServiceHandler { // Thumbnail implements the GRPC handler. type Thumbnail struct { - manager thumbnails.Manager - source imgsource.Source - logger log.Logger + manager thumbnail.Manager + resolutions resolution.Resolutions + source imgsource.Source + logger log.Logger } // GetThumbnail retrieves a thumbnail for an image func (g Thumbnail) GetThumbnail(ctx context.Context, req *v0proto.GetRequest, rsp *v0proto.GetResponse) error { - encoder := thumbnails.EncoderForType(req.Filetype.String()) + encoder := thumbnail.EncoderForType(req.Filetype.String()) if encoder == nil { // TODO: better error responses return fmt.Errorf("can't be encoded. filetype %s not supported", req.Filetype.String()) } - tCtx := thumbnails.Context{ - Width: int(req.Width), - Height: int(req.Height), - ImagePath: req.Filepath, - Encoder: encoder, - ETag: req.Etag, + r := g.resolutions.ClosestMatch(int(req.Width), int(req.Height)) + tr := thumbnail.Request{ + Resolution: r, + ImagePath: req.Filepath, + Encoder: encoder, + ETag: req.Etag, } - thumbnail := g.manager.GetStored(tCtx) + thumbnail := g.manager.GetStored(tr) if thumbnail != nil { rsp.Thumbnail = thumbnail - rsp.Mimetype = tCtx.Encoder.MimeType() + rsp.Mimetype = tr.Encoder.MimeType() return nil } auth := req.Authorization sCtx := context.WithValue(ctx, imgsource.WebDavAuth, auth) // TODO: clean up error handling - img, err := g.source.Get(sCtx, tCtx.ImagePath) + img, err := g.source.Get(sCtx, tr.ImagePath) if err != nil { return err } if img == nil { return fmt.Errorf("could not retrieve image") } - thumbnail, err = g.manager.Get(tCtx, img) + thumbnail, err = g.manager.Get(tr, img) if err != nil { return err } rsp.Thumbnail = thumbnail - rsp.Mimetype = tCtx.Encoder.MimeType() + rsp.Mimetype = tr.Encoder.MimeType() return nil } diff --git a/pkg/thumbnails/encoding.go b/pkg/thumbnail/encoding.go similarity index 98% rename from pkg/thumbnails/encoding.go rename to pkg/thumbnail/encoding.go index a4d4647bb9..d8d2d358bc 100644 --- a/pkg/thumbnails/encoding.go +++ b/pkg/thumbnail/encoding.go @@ -1,4 +1,4 @@ -package thumbnails +package thumbnail import ( "image" diff --git a/pkg/thumbnails/imgsource/imgsource.go b/pkg/thumbnail/imgsource/imgsource.go similarity index 100% rename from pkg/thumbnails/imgsource/imgsource.go rename to pkg/thumbnail/imgsource/imgsource.go diff --git a/pkg/thumbnails/imgsource/webdav.go b/pkg/thumbnail/imgsource/webdav.go similarity index 100% rename from pkg/thumbnails/imgsource/webdav.go rename to pkg/thumbnail/imgsource/webdav.go diff --git a/pkg/thumbnail/resolution/resolution.go b/pkg/thumbnail/resolution/resolution.go new file mode 100644 index 0000000000..514fe85744 --- /dev/null +++ b/pkg/thumbnail/resolution/resolution.go @@ -0,0 +1,37 @@ +package resolution + +import ( + "fmt" + "strconv" + "strings" +) + +// Parse parses a resolution string in the form x and returns a resolution instance. +func Parse(s string) (Resolution, error) { + parts := strings.Split(s, "x") + if len(parts) != 2 { + return Resolution{}, fmt.Errorf("failed to parse resolution: %s. Expected format x", s) + } + width, err := strconv.Atoi(parts[0]) + if err != nil { + return Resolution{}, fmt.Errorf("width: %s has an invalid value. Expected an integer", parts[0]) + } + height, err := strconv.Atoi(parts[1]) + if err != nil { + return Resolution{}, fmt.Errorf("height: %s has an invalid value. Expected an integer", parts[1]) + } + return Resolution{Width: width, Height: height}, nil +} + +// Resolution defines represents the width and height of a thumbnail. +type Resolution struct { + Width int + Height int +} + +// String returns the resolution in the format: +// +// x +func (r Resolution) String() string { + return strconv.Itoa(r.Width) + "x" + strconv.Itoa(r.Height) +} diff --git a/pkg/thumbnail/resolution/resolution_test.go b/pkg/thumbnail/resolution/resolution_test.go new file mode 100644 index 0000000000..b8c3e6c883 --- /dev/null +++ b/pkg/thumbnail/resolution/resolution_test.go @@ -0,0 +1,40 @@ +package resolution + +import "testing" + +func TestParseWithEmptyString(t *testing.T) { + _, err := Parse("") + if err == nil { + t.Error("Parse with empty string should return an error.") + } +} + +func TestParseWithInvalidWidth(t *testing.T) { + _, err := Parse("invalidx42") + if err == nil { + t.Error("Parse with invalid width should return an error.") + } +} + +func TestParseWithInvalidHeight(t *testing.T) { + _, err := Parse("42xinvalid") + if err == nil { + t.Error("Parse with invalid height should return an error.") + } +} + +func TestParse(t *testing.T) { + rStr := "42x23" + r, _ := Parse(rStr) + if r.Width != 42 || r.Height != 23 { + t.Errorf("Expected resolution %s got %s", rStr, r.String()) + } +} + +func TestString(t *testing.T) { + r := Resolution{Width: 42, Height: 23} + expected := "42x23" + if r.String() != expected { + t.Errorf("Expected string %s got %s", expected, r.String()) + } +} diff --git a/pkg/thumbnail/resolution/resolutions.go b/pkg/thumbnail/resolution/resolutions.go new file mode 100644 index 0000000000..2a83d45e7d --- /dev/null +++ b/pkg/thumbnail/resolution/resolutions.go @@ -0,0 +1,74 @@ +package resolution + +import ( + "fmt" + "math" + "sort" +) + +// New creates an instance of Resolutions from resolution strings. +func New(rStrs []string) (Resolutions, error) { + var rs Resolutions + for _, rStr := range rStrs { + r, err := Parse(rStr) + if err != nil { + return nil, fmt.Errorf("failed to initialize resolutions: %s", err.Error()) + } + rs = append(rs, r) + } + sort.Slice(rs, func(i, j int) bool { + left := rs[i] + right := rs[j] + + leftSize := left.Width * left.Height + rightSize := right.Width * right.Height + + return leftSize < rightSize + }) + + return rs, nil +} + +// Resolutions represents the available thumbnail resolutions. +type Resolutions []Resolution + +// ClosestMatch returns the resolution which is closest to the provided resolution. +// If there is no exact match the resolution will be the next higher one. +// If the given resolution is bigger than all available resolutions the biggest available one is used. +func (r Resolutions) ClosestMatch(width, height int) Resolution { + if len(r) == 0 { + return Resolution{Width: width, Height: height} + } + + isLandscape := width > height + givenLen := int(math.Max(float64(width), float64(height))) + + // Initialize with the first resolution + var match Resolution + minDiff := math.MaxInt32 + + for _, current := range r { + len := dimensionLength(current, isLandscape) + diff := givenLen - len + if diff > 0 { + continue + } + absDiff := int(math.Abs(float64(diff))) + if absDiff < minDiff { + minDiff = absDiff + match = current + } + } + + if match == (Resolution{}) { + match = r[len(r)-1] + } + return match +} + +func dimensionLength(r Resolution, landscape bool) int { + if landscape { + return r.Width + } + return r.Height +} diff --git a/pkg/thumbnail/resolution/resolutions_test.go b/pkg/thumbnail/resolution/resolutions_test.go new file mode 100644 index 0000000000..1190cc942a --- /dev/null +++ b/pkg/thumbnail/resolution/resolutions_test.go @@ -0,0 +1,111 @@ +package resolution + +import ( + "testing" +) + +func TestInitWithEmptyArray(t *testing.T) { + rs, err := New([]string{}) + if err != nil { + t.Errorf("Init with an empty array should not fail. Error: %s.\n", err.Error()) + } + if len(rs) != 0 { + t.Error("Init with an empty array should return an empty Resolutions instance.\n") + } +} + +func TestInitWithNil(t *testing.T) { + rs, err := New(nil) + if err != nil { + t.Errorf("Init with nil parameter should not fail. Error: %s.\n", err.Error()) + } + if len(rs) != 0 { + t.Error("Init with nil parameter should return an empty Resolutions instance.\n") + } +} + +func TestInitWithInvalidValuesInArray(t *testing.T) { + _, err := New([]string{"invalid"}) + if err == nil { + t.Error("Init with invalid parameter should fail.\n") + } +} + +func TestInit(t *testing.T) { + rs, err := New([]string{"16x16"}) + if err != nil { + t.Errorf("Init with valid parameter should not fail. Error: %s.\n", err.Error()) + } + if len(rs) != 1 { + t.Errorf("resolutions has size %d, expected size %d.\n", len(rs), 1) + } +} + +func TestInitWithMultipleResolutions(t *testing.T) { + rStrs := []string{"16x16", "32x32", "64x64", "128x128"} + rs, err := New(rStrs) + if err != nil { + t.Errorf("Init with valid parameter should not fail. Error: %s.\n", err.Error()) + } + if len(rs) != len(rStrs) { + t.Errorf("resolutions has size %d, expected size %d.\n", len(rs), len(rStrs)) + } +} + +func TestInitWithMultipleResolutionsShouldBeSorted(t *testing.T) { + rStrs := []string{"32x32", "64x64", "16x16", "128x128"} + rs, err := New(rStrs) + if err != nil { + t.Errorf("Init with valid parameter should not fail. Error: %s.\n", err.Error()) + } + + for i := 0; i < len(rs)-1; i++ { + current := rs[i] + currentSize := current.Width * current.Height + next := rs[i] + nextSize := next.Width * next.Height + + if currentSize > nextSize { + t.Error("Resolutions are not sorted.") + } + + } +} +func TestClosestMatchWithEmptyResolutions(t *testing.T) { + rs, _ := New(nil) + width := 24 + height := 24 + + r := rs.ClosestMatch(width, height) + if r.Width != width || r.Height != height { + t.Errorf("ClosestMatch from empty resolutions should return the given resolution") + } +} + +func TestClosestMatch(t *testing.T) { + rs, _ := New([]string{"16x16", "24x24", "32x32", "64x64", "128x128"}) + table := [][]int{ + // width, height, expectedWidth, expectedHeight + []int{17, 17, 24, 24}, + []int{12, 17, 24, 24}, + []int{24, 24, 24, 24}, + []int{20, 20, 24, 24}, + []int{20, 80, 128, 128}, + []int{80, 20, 128, 128}, + []int{48, 48, 64, 64}, + []int{1024, 1024, 128, 128}, + } + + for _, row := range table { + width := row[0] + height := row[1] + expectedWidth := row[2] + expectedHeight := row[3] + + match := rs.ClosestMatch(width, height) + + if match.Width != expectedWidth || match.Height != expectedHeight { + t.Errorf("Expected resolution %dx%d got %s", expectedWidth, expectedHeight, match.String()) + } + } +} diff --git a/pkg/thumbnails/storage/filesystem.go b/pkg/thumbnail/storage/filesystem.go similarity index 90% rename from pkg/thumbnails/storage/filesystem.go rename to pkg/thumbnail/storage/filesystem.go index e59b77170e..dc45d9de03 100644 --- a/pkg/thumbnails/storage/filesystem.go +++ b/pkg/thumbnail/storage/filesystem.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "os" "path/filepath" - "strconv" "github.com/owncloud/ocis-pkg/v2/log" "github.com/owncloud/ocis-thumbnails/pkg/config" @@ -65,10 +64,10 @@ func (s FileSystem) Set(key string, img []byte) error { // e.g. 97/9f/4c8db98f7b82e768ef478d3c8612/500x300.png // // The key also represents the path to the thumbnail in the filesystem under the configured root directory. -func (s FileSystem) BuildKey(ctx Context) string { - etag := ctx.ETag - filetype := ctx.Types[0] - filename := strconv.Itoa(ctx.Width) + "x" + strconv.Itoa(ctx.Height) + "." + filetype +func (s FileSystem) BuildKey(r Request) string { + etag := r.ETag + filetype := r.Types[0] + filename := r.Resolution.String() + "." + filetype key := new(bytes.Buffer) key.WriteString(etag[:2]) diff --git a/pkg/thumbnails/storage/inmemory.go b/pkg/thumbnail/storage/inmemory.go similarity index 83% rename from pkg/thumbnails/storage/inmemory.go rename to pkg/thumbnail/storage/inmemory.go index c8f02a9ae2..12017eed0c 100644 --- a/pkg/thumbnails/storage/inmemory.go +++ b/pkg/thumbnail/storage/inmemory.go @@ -29,11 +29,11 @@ func (s InMemory) Set(key string, thumbnail []byte) error { } // BuildKey generates a unique key to store and retrieve the thumbnail. -func (s InMemory) BuildKey(ctx Context) string { +func (s InMemory) BuildKey(r Request) string { parts := []string{ - ctx.ETag, - string(ctx.Width) + "x" + string(ctx.Height), - strings.Join(ctx.Types, ","), + r.ETag, + r.Resolution.String(), + strings.Join(r.Types, ","), } return strings.Join(parts, "+") } diff --git a/pkg/thumbnail/storage/storage.go b/pkg/thumbnail/storage/storage.go new file mode 100644 index 0000000000..a23a757d3c --- /dev/null +++ b/pkg/thumbnail/storage/storage.go @@ -0,0 +1,17 @@ +package storage + +import "github.com/owncloud/ocis-thumbnails/pkg/thumbnail/resolution" + +// Request combines different attributes needed for storage operations. +type Request struct { + ETag string + Types []string + Resolution resolution.Resolution +} + +// Storage defines the interface for a thumbnail store. +type Storage interface { + Get(string) []byte + Set(string, []byte) error + BuildKey(Request) string +} diff --git a/pkg/thumbnails/thumbnails.go b/pkg/thumbnail/thumbnails.go similarity index 52% rename from pkg/thumbnails/thumbnails.go rename to pkg/thumbnail/thumbnails.go index 76223228dc..846cd60138 100644 --- a/pkg/thumbnails/thumbnails.go +++ b/pkg/thumbnail/thumbnails.go @@ -1,4 +1,4 @@ -package thumbnails +package thumbnail import ( "bytes" @@ -6,25 +6,25 @@ import ( "github.com/nfnt/resize" "github.com/owncloud/ocis-pkg/v2/log" - "github.com/owncloud/ocis-thumbnails/pkg/thumbnails/storage" + "github.com/owncloud/ocis-thumbnails/pkg/thumbnail/resolution" + "github.com/owncloud/ocis-thumbnails/pkg/thumbnail/storage" ) -// Context bundles information needed to generate a thumbnail for afile -type Context struct { - Width int - Height int - ImagePath string - Encoder Encoder - ETag string +// Request bundles information needed to generate a thumbnail for afile +type Request struct { + Resolution resolution.Resolution + ImagePath string + Encoder Encoder + ETag string } // Manager is responsible for generating thumbnails type Manager interface { // Get will return a thumbnail for a file - Get(Context, image.Image) ([]byte, error) + Get(Request, image.Image) ([]byte, error) // GetStored loads the thumbnail from the storage. // It will return nil if no image is stored for the given context. - GetStored(Context) []byte + GetStored(Request) []byte } // NewSimpleManager creates a new instance of SimpleManager @@ -42,13 +42,13 @@ type SimpleManager struct { } // Get implements the Get Method of Manager -func (s SimpleManager) Get(ctx Context, img image.Image) ([]byte, error) { - thumbnail := s.generate(ctx, img) +func (s SimpleManager) Get(r Request, img image.Image) ([]byte, error) { + thumbnail := s.generate(r, img) - key := s.storage.BuildKey(mapToStorageContext(ctx)) + key := s.storage.BuildKey(mapToStorageRequest(r)) buf := new(bytes.Buffer) - err := ctx.Encoder.Encode(buf, thumbnail) + err := r.Encoder.Encode(buf, thumbnail) if err != nil { return nil, err } @@ -62,23 +62,22 @@ func (s SimpleManager) Get(ctx Context, img image.Image) ([]byte, error) { // GetStored tries to get the stored thumbnail and return it. // If there is no cached thumbnail it will return nil -func (s SimpleManager) GetStored(ctx Context) []byte { - key := s.storage.BuildKey(mapToStorageContext(ctx)) +func (s SimpleManager) GetStored(r Request) []byte { + key := s.storage.BuildKey(mapToStorageRequest(r)) stored := s.storage.Get(key) return stored } -func (s SimpleManager) generate(ctx Context, img image.Image) image.Image { - thumbnail := resize.Thumbnail(uint(ctx.Width), uint(ctx.Height), img, resize.Lanczos2) +func (s SimpleManager) generate(r Request, img image.Image) image.Image { + thumbnail := resize.Thumbnail(uint(r.Resolution.Width), uint(r.Resolution.Height), img, resize.Lanczos2) return thumbnail } -func mapToStorageContext(ctx Context) storage.Context { - sCtx := storage.Context{ - ETag: ctx.ETag, - Width: ctx.Width, - Height: ctx.Height, - Types: ctx.Encoder.Types(), +func mapToStorageRequest(r Request) storage.Request { + sR := storage.Request{ + ETag: r.ETag, + Resolution: r.Resolution, + Types: r.Encoder.Types(), } - return sCtx + return sR } diff --git a/pkg/thumbnails/storage/storage.go b/pkg/thumbnails/storage/storage.go deleted file mode 100644 index 8f5f3d9de0..0000000000 --- a/pkg/thumbnails/storage/storage.go +++ /dev/null @@ -1,16 +0,0 @@ -package storage - -// Context combines different attributes needed for storage operations. -type Context struct { - ETag string - Types []string - Width int - Height int -} - -// Storage defines the interface for a thumbnail store. -type Storage interface { - Get(string) []byte - Set(string, []byte) error - BuildKey(Context) string -}