Merge pull request #8 from owncloud/predefined-resolutions

use predefined resolutions for thumbnail generation

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

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":

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{}

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"},
 		},
 	}
 }

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)

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
 }

pkg/thumbnail/encoding.go

@@ -1,4 +1,4 @@
-package thumbnails
+package thumbnail
 
 import (
 	"image"

pkg/thumbnail/resolution/resolution.go

@@ -0,0 +1,37 @@
+package resolution
+
+import (
+	"fmt"
+	"strconv"
+	"strings"
+)
+
+// Parse parses a resolution string in the form <width>x<height> 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 <width>x<height>", 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:
+//
+// <width>x<height>
+func (r Resolution) String() string {
+	return strconv.Itoa(r.Width) + "x" + strconv.Itoa(r.Height)
+}

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())
+	}
+}

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
+}

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())
+		}
+	}
+}

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])

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, "+")
 }

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
+}

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
 }

pkg/thumbnails/storage/storage.go

@@ -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
-}