diff --git a/changelog/unreleased/fix-preview-gif.md b/changelog/unreleased/fix-preview-gif.md new file mode 100644 index 0000000000..469bce4027 --- /dev/null +++ b/changelog/unreleased/fix-preview-gif.md @@ -0,0 +1,6 @@ +Enhancement: Fix preview or viewing of shared animated GIFs + +Fix preview or viewing of shared animated GIFs + +https://github.com/owncloud/ocis/pull/6386 +https://github.com/owncloud/ocis/issues/5418 diff --git a/services/thumbnails/pkg/service/grpc/v0/service.go b/services/thumbnails/pkg/service/grpc/v0/service.go index 76f33d7138..541e56bca7 100644 --- a/services/thumbnails/pkg/service/grpc/v0/service.go +++ b/services/thumbnails/pkg/service/grpc/v0/service.go @@ -2,7 +2,6 @@ package svc import ( "context" - "image" "net/http" "net/url" "path" @@ -17,7 +16,6 @@ import ( "github.com/cs3org/reva/v2/pkg/utils" "github.com/golang-jwt/jwt/v4" "github.com/owncloud/ocis/v2/ocis-pkg/log" - thumbnailsmsg "github.com/owncloud/ocis/v2/protogen/gen/ocis/messages/thumbnails/v0" thumbnailssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/thumbnails/v0" "github.com/owncloud/ocis/v2/services/thumbnails/pkg/preprocessor" "github.com/owncloud/ocis/v2/services/thumbnails/pkg/service/grpc/v0/decorators" @@ -77,26 +75,13 @@ type PreprocessorOpts struct { // GetThumbnail retrieves a thumbnail for an image func (g Thumbnail) GetThumbnail(ctx context.Context, req *thumbnailssvc.GetThumbnailRequest, rsp *thumbnailssvc.GetThumbnailResponse) error { - tType, ok := thumbnailsmsg.ThumbnailType_name[int32(req.ThumbnailType)] - if !ok { - g.logger.Debug().Str("thumbnail_type", tType).Msg("unsupported thumbnail type") - return nil - } - generator, err := thumbnail.GeneratorForType(tType) - if err != nil { - return merrors.BadRequest(g.serviceID, "unsupported thumbnail type") - } - encoder, err := thumbnail.EncoderForType(tType) - if err != nil { - return merrors.BadRequest(g.serviceID, "unsupported thumbnail type") - } - + var err error var key string switch { case req.GetWebdavSource() != nil: - key, err = g.handleWebdavSource(ctx, req, generator, encoder) + key, err = g.handleWebdavSource(ctx, req) case req.GetCs3Source() != nil: - key, err = g.handleCS3Source(ctx, req, generator, encoder) + key, err = g.handleCS3Source(ctx, req) default: g.logger.Error().Msg("no image source provided") return merrors.BadRequest(g.serviceID, "image source is missing") @@ -123,25 +108,23 @@ func (g Thumbnail) GetThumbnail(ctx context.Context, req *thumbnailssvc.GetThumb } rsp.DataEndpoint = g.dataEndpoint rsp.TransferToken = transferToken - rsp.Mimetype = encoder.MimeType() return nil } -func (g Thumbnail) handleCS3Source(ctx context.Context, - req *thumbnailssvc.GetThumbnailRequest, - generator thumbnail.Generator, - encoder thumbnail.Encoder) (string, error) { +func (g Thumbnail) handleCS3Source(ctx context.Context, req *thumbnailssvc.GetThumbnailRequest) (string, error) { src := req.GetCs3Source() sRes, err := g.stat(src.Path, src.Authorization) if err != nil { return "", err } - tr := thumbnail.Request{ - Resolution: image.Rect(0, 0, int(req.Width), int(req.Height)), - Generator: generator, - Encoder: encoder, - Checksum: sRes.GetInfo().GetChecksum().GetSum(), + tType := thumbnail.GetExtForMime(sRes.GetInfo().GetMimeType()) + if tType == "" { + tType = req.GetThumbnailType().String() + } + tr, err := thumbnail.PrepareRequest(int(req.Width), int(req.Height), tType, sRes.GetInfo().GetChecksum().GetSum()) + if err != nil { + return "", merrors.BadRequest(g.serviceID, err.Error()) } if key, exists := g.manager.CheckThumbnail(tr); exists { @@ -170,10 +153,7 @@ func (g Thumbnail) handleCS3Source(ctx context.Context, return key, nil } -func (g Thumbnail) handleWebdavSource(ctx context.Context, - req *thumbnailssvc.GetThumbnailRequest, - generator thumbnail.Generator, - encoder thumbnail.Encoder) (string, error) { +func (g Thumbnail) handleWebdavSource(ctx context.Context, req *thumbnailssvc.GetThumbnailRequest) (string, error) { src := req.GetWebdavSource() imgURL, err := url.Parse(src.Url) if err != nil { @@ -217,11 +197,14 @@ func (g Thumbnail) handleWebdavSource(ctx context.Context, if err != nil { return "", err } - tr := thumbnail.Request{ - Resolution: image.Rect(0, 0, int(req.Width), int(req.Height)), - Generator: generator, - Encoder: encoder, - Checksum: sRes.GetInfo().GetChecksum().GetSum(), + + tType := thumbnail.GetExtForMime(sRes.GetInfo().GetMimeType()) + if tType == "" { + tType = req.GetThumbnailType().String() + } + tr, err := thumbnail.PrepareRequest(int(req.Width), int(req.Height), tType, sRes.GetInfo().GetChecksum().GetSum()) + if err != nil { + return "", merrors.BadRequest(g.serviceID, err.Error()) } if key, exists := g.manager.CheckThumbnail(tr); exists { diff --git a/services/thumbnails/pkg/thumbnail/encoding.go b/services/thumbnails/pkg/thumbnail/encoding.go index d9c43d2ece..5126fd6af9 100644 --- a/services/thumbnails/pkg/thumbnail/encoding.go +++ b/services/thumbnails/pkg/thumbnail/encoding.go @@ -80,6 +80,7 @@ func (e JpegEncoder) MimeType() string { type GifEncoder struct{} +// Encode encodes the image to a gif format func (e GifEncoder) Encode(w io.Writer, img interface{}) error { g, ok := img.(*gif.GIF) if !ok { @@ -92,6 +93,7 @@ func (e GifEncoder) Types() []string { return []string{typeGif} } +// MimeType returns the mimetype used by the encoder. func (e GifEncoder) MimeType() string { return "image/gif" } @@ -110,3 +112,14 @@ func EncoderForType(fileType string) (Encoder, error) { return nil, ErrNoEncoderForType } } + +// GetExtForMime return the supported extension by mime +func GetExtForMime(mime string) string { + ext := strings.TrimPrefix(strings.TrimSpace(strings.ToLower(mime)), "image/") + switch ext { + case typeJpg, typeJpeg, typePng, typeGif: + return ext + default: + return "" + } +} diff --git a/services/thumbnails/pkg/thumbnail/thumbnail.go b/services/thumbnails/pkg/thumbnail/thumbnail.go index 31325ff3d7..70fcdc5b20 100644 --- a/services/thumbnails/pkg/thumbnail/thumbnail.go +++ b/services/thumbnails/pkg/thumbnail/thumbnail.go @@ -104,6 +104,7 @@ func mapToStorageRequest(r Request) storage.Request { } } +// IsMimeTypeSupported validate if the mime type is supported func IsMimeTypeSupported(m string) bool { mimeType, _, err := mime.ParseMediaType(m) if err != nil { @@ -112,3 +113,22 @@ func IsMimeTypeSupported(m string) bool { _, supported := SupportedMimeTypes[mimeType] return supported } + +// PrepareRequest prepare the request based on image parameters +func PrepareRequest(width, height int, tType, checksum string) (Request, error) { + generator, err := GeneratorForType(tType) + if err != nil { + return Request{}, err + } + encoder, err := EncoderForType(tType) + if err != nil { + return Request{}, err + } + + return Request{ + Resolution: image.Rect(0, 0, width, height), + Generator: generator, + Encoder: encoder, + Checksum: checksum, + }, nil +} diff --git a/services/thumbnails/pkg/thumbnail/thumbnail_test.go b/services/thumbnails/pkg/thumbnail/thumbnail_test.go index cf301a0ddb..45303dcc87 100644 --- a/services/thumbnails/pkg/thumbnail/thumbnail_test.go +++ b/services/thumbnails/pkg/thumbnail/thumbnail_test.go @@ -4,6 +4,7 @@ import ( "image" "os" "path/filepath" + "reflect" "testing" "github.com/owncloud/ocis/v2/ocis-pkg/log" @@ -45,3 +46,86 @@ func BenchmarkGet(b *testing.B) { _, _ = sut.Generate(req, img) } } + +func TestPrepareRequest(t *testing.T) { + type args struct { + width int + height int + tType string + checksum string + } + tests := []struct { + name string + args args + want Request + wantErr bool + }{ + { + name: "Test successful prepare the request for jpg", + args: args{ + width: 32, + height: 32, + tType: "jpg", + checksum: "1872ade88f3013edeb33decd74a4f947", + }, + want: Request{ + Resolution: image.Rect(0, 0, 32, 32), + Encoder: JpegEncoder{}, + Generator: SimpleGenerator{}, + Checksum: "1872ade88f3013edeb33decd74a4f947", + }, + }, + { + name: "Test successful prepare the request for png", + args: args{ + width: 32, + height: 32, + tType: "png", + checksum: "1872ade88f3013edeb33decd74a4f947", + }, + want: Request{ + Resolution: image.Rect(0, 0, 32, 32), + Encoder: PngEncoder{}, + Generator: SimpleGenerator{}, + Checksum: "1872ade88f3013edeb33decd74a4f947", + }, + }, + { + name: "Test successful prepare the request for gif", + args: args{ + width: 32, + height: 32, + tType: "gif", + checksum: "1872ade88f3013edeb33decd74a4f947", + }, + want: Request{ + Resolution: image.Rect(0, 0, 32, 32), + Encoder: GifEncoder{}, + Generator: GifGenerator{}, + Checksum: "1872ade88f3013edeb33decd74a4f947", + }, + }, + { + name: "Test error when prepare the request for bmp", + args: args{ + width: 32, + height: 32, + tType: "bmp", + checksum: "1872ade88f3013edeb33decd74a4f947", + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := PrepareRequest(tt.args.width, tt.args.height, tt.args.tType, tt.args.checksum) + if (err != nil) != tt.wantErr { + t.Errorf("PrepareRequest() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("PrepareRequest() got = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/services/webdav/pkg/service/v0/service.go b/services/webdav/pkg/service/v0/service.go index 963abbd663..fe63df7cb8 100644 --- a/services/webdav/pkg/service/v0/service.go +++ b/services/webdav/pkg/service/v0/service.go @@ -463,7 +463,7 @@ func (g Webdav) sendThumbnailResponse(rsp *thumbnailssvc.GetThumbnailResponse, w } w.WriteHeader(http.StatusOK) - w.Header().Set("Content-Type", rsp.Mimetype) + w.Header().Set("Content-Type", dlRsp.Header.Get("Content-Type")) _, err = io.Copy(w, dlRsp.Body) if err != nil { logger.Error().Err(err).Msg("failed to write thumbnail to response writer")