From 088fcdab0de245db918faf57a4fc22c564254cba Mon Sep 17 00:00:00 2001 From: Anass Bouassaba Date: Sat, 5 Oct 2024 20:09:23 +0200 Subject: [PATCH] fix(conversion): SIGSEGV on small images (#345) --- conversion/pipeline/audio_video_pipeline.go | 15 +++++++------ conversion/pipeline/glb_pipeline.go | 22 +++++++++---------- conversion/pipeline/image_pipeline.go | 22 +++++++++---------- conversion/pipeline/pdf_pipeline.go | 22 +++++++++---------- conversion/processor/image_processor.go | 24 ++++++++++++--------- conversion/processor/video_processor.go | 2 +- 6 files changed, 56 insertions(+), 51 deletions(-) diff --git a/conversion/pipeline/audio_video_pipeline.go b/conversion/pipeline/audio_video_pipeline.go index 0b5acb5eb..3d63a62a1 100644 --- a/conversion/pipeline/audio_video_pipeline.go +++ b/conversion/pipeline/audio_video_pipeline.go @@ -68,6 +68,7 @@ func (p *audioVideoPipeline) RunFromLocalPath(inputPath string, opts api_client. return err } // Here we intentionally ignore the error, as the media file may contain just audio + // Additionally, we don't consider failing to create the thumbnail an error _ = p.createThumbnail(inputPath, opts) if err := p.taskClient.Patch(opts.TaskID, api_client.TaskPatchOptions{ Fields: []string{api_client.TaskFieldName}, @@ -89,7 +90,7 @@ func (p *audioVideoPipeline) RunFromLocalPath(inputPath string, opts api_client. } func (p *audioVideoPipeline) createThumbnail(inputPath string, opts api_client.PipelineRunOptions) error { - tmpPath := filepath.FromSlash(os.TempDir() + "/" + helper.NewID() + ".png") + outputPath := filepath.FromSlash(os.TempDir() + "/" + helper.NewID() + ".png") defer func(path string) { _, err := os.Stat(path) if os.IsExist(err) { @@ -97,25 +98,25 @@ func (p *audioVideoPipeline) createThumbnail(inputPath string, opts api_client.P infra.GetLogger().Error(err) } } - }(tmpPath) - if err := p.videoProc.Thumbnail(inputPath, 0, p.config.Limits.ImagePreviewMaxHeight, tmpPath); err != nil { + }(outputPath) + if err := p.videoProc.Thumbnail(inputPath, p.config.Limits.ImagePreviewMaxWidth, p.config.Limits.ImagePreviewMaxHeight, outputPath); err != nil { return err } - props, err := p.imageProc.MeasureImage(tmpPath) + props, err := p.imageProc.MeasureImage(outputPath) if err != nil { return err } - stat, err := os.Stat(tmpPath) + stat, err := os.Stat(outputPath) if err != nil { return err } s3Object := &api_client.S3Object{ Bucket: opts.Bucket, - Key: opts.SnapshotID + "/thumbnail" + filepath.Ext(tmpPath), + Key: opts.SnapshotID + "/thumbnail" + filepath.Ext(outputPath), Image: props, Size: helper.ToPtr(stat.Size()), } - if err := p.s3.PutFile(s3Object.Key, tmpPath, helper.DetectMimeFromFile(tmpPath), s3Object.Bucket, minio.PutObjectOptions{}); err != nil { + if err := p.s3.PutFile(s3Object.Key, outputPath, helper.DetectMimeFromFile(outputPath), s3Object.Bucket, minio.PutObjectOptions{}); err != nil { return err } if err := p.snapshotClient.Patch(api_client.SnapshotPatchOptions{ diff --git a/conversion/pipeline/glb_pipeline.go b/conversion/pipeline/glb_pipeline.go index 378fb2c04..0c8539d0e 100644 --- a/conversion/pipeline/glb_pipeline.go +++ b/conversion/pipeline/glb_pipeline.go @@ -67,9 +67,8 @@ func (p *glbPipeline) RunFromLocalPath(inputPath string, opts api_client.Pipelin }); err != nil { return err } - if err := p.createThumbnail(inputPath, opts); err != nil { - return err - } + // We don't consider failing to create the thumbnail an error + _ = p.createThumbnail(inputPath, opts) if err := p.patchSnapshotPreviewField(inputPath, opts); err != nil { return err } @@ -112,29 +111,30 @@ func (p *glbPipeline) patchSnapshotPreviewField(inputPath string, opts api_clien } func (p *glbPipeline) createThumbnail(inputPath string, opts api_client.PipelineRunOptions) error { - tmpPath := filepath.FromSlash(os.TempDir() + "/" + helper.NewID() + ".png") + outputPath := filepath.FromSlash(os.TempDir() + "/" + helper.NewID() + ".png") defer func(path string) { if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { return } else if err != nil { infra.GetLogger().Error(err) } - }(tmpPath) - // We don't consider failing to create the thumbnail as an error - _ = p.glbProc.Thumbnail(inputPath, p.config.Limits.ImagePreviewMaxWidth, p.config.Limits.ImagePreviewMaxHeight, tmpPath) - stat, err := os.Stat(tmpPath) + }(outputPath) + if err := p.glbProc.Thumbnail(inputPath, p.config.Limits.ImagePreviewMaxWidth, p.config.Limits.ImagePreviewMaxHeight, outputPath); err != nil { + return err + } + stat, err := os.Stat(outputPath) if err == nil { - props, err := p.imageProc.MeasureImage(tmpPath) + props, err := p.imageProc.MeasureImage(outputPath) if err != nil { return err } s3Object := &api_client.S3Object{ Bucket: opts.Bucket, - Key: opts.SnapshotID + "/thumbnail" + filepath.Ext(tmpPath), + Key: opts.SnapshotID + "/thumbnail" + filepath.Ext(outputPath), Image: props, Size: helper.ToPtr(stat.Size()), } - if err := p.s3.PutFile(s3Object.Key, tmpPath, helper.DetectMimeFromFile(tmpPath), s3Object.Bucket, minio.PutObjectOptions{}); err != nil { + if err := p.s3.PutFile(s3Object.Key, outputPath, helper.DetectMimeFromFile(outputPath), s3Object.Bucket, minio.PutObjectOptions{}); err != nil { return err } if err := p.snapshotClient.Patch(api_client.SnapshotPatchOptions{ diff --git a/conversion/pipeline/image_pipeline.go b/conversion/pipeline/image_pipeline.go index ddb823146..92e0b0e5d 100644 --- a/conversion/pipeline/image_pipeline.go +++ b/conversion/pipeline/image_pipeline.go @@ -108,7 +108,7 @@ func (p *imagePipeline) RunFromLocalPath(inputPath string, opts api_client.Pipel }); err != nil { return err } - // We don't consider failing the creation of the thumbnail as an error + // We don't consider failing the creation of the thumbnail an error _ = p.createThumbnail(imagePath, opts) // Automatically trigger mosaic pipeline if the image exceeds the pixels threshold if imageProps.Width >= p.config.Limits.ImageMosaicTriggerThresholdPixels || @@ -153,36 +153,36 @@ func (p *imagePipeline) measureImageDimensions(inputPath string, opts api_client } func (p *imagePipeline) createThumbnail(inputPath string, opts api_client.PipelineRunOptions) error { - tmpPath := filepath.FromSlash(os.TempDir() + "/" + helper.NewID() + filepath.Ext(inputPath)) - thumbnailResult, err := p.imageProc.Thumbnail(inputPath, tmpPath) + outputPath := filepath.FromSlash(os.TempDir() + "/" + helper.NewID() + filepath.Ext(inputPath)) + res, err := p.imageProc.Thumbnail(inputPath, p.config.Limits.ImagePreviewMaxWidth, p.config.Limits.ImagePreviewMaxHeight, outputPath) if err != nil { return err } - if thumbnailResult.Success { + if res.IsCreated { defer func(path string) { if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { return } else if err != nil { infra.GetLogger().Error(err) } - }(tmpPath) + }(outputPath) } else { - tmpPath = inputPath + outputPath = inputPath } - stat, err := os.Stat(tmpPath) + stat, err := os.Stat(outputPath) if err != nil { return err } s3Object := &api_client.S3Object{ Bucket: opts.Bucket, - Key: opts.SnapshotID + "/thumbnail" + filepath.Ext(tmpPath), + Key: opts.SnapshotID + "/thumbnail" + filepath.Ext(outputPath), Image: &api_client.ImageProps{ - Width: *thumbnailResult.Width, - Height: *thumbnailResult.Height, + Width: res.Width, + Height: res.Height, }, Size: helper.ToPtr(stat.Size()), } - if err := p.s3.PutFile(s3Object.Key, tmpPath, helper.DetectMimeFromFile(tmpPath), s3Object.Bucket, minio.PutObjectOptions{}); err != nil { + if err := p.s3.PutFile(s3Object.Key, outputPath, helper.DetectMimeFromFile(outputPath), s3Object.Bucket, minio.PutObjectOptions{}); err != nil { return err } if err := p.snapshotClient.Patch(api_client.SnapshotPatchOptions{ diff --git a/conversion/pipeline/pdf_pipeline.go b/conversion/pipeline/pdf_pipeline.go index b4a34c048..4c20d9432 100644 --- a/conversion/pipeline/pdf_pipeline.go +++ b/conversion/pipeline/pdf_pipeline.go @@ -83,9 +83,8 @@ func (p *pdfPipeline) RunFromLocalPath(inputPath string, opts api_client.Pipelin }); err != nil { return err } - if err := p.createThumbnail(inputPath, opts); err != nil { - return err - } + // We don't consider failing the creation of the thumbnail an error + _ = p.createThumbnail(inputPath, opts) if err := p.taskClient.Patch(opts.TaskID, api_client.TaskPatchOptions{ Fields: []string{api_client.TaskFieldName}, Name: helper.ToPtr("Saving preview."), @@ -112,31 +111,32 @@ func (p *pdfPipeline) RunFromLocalPath(inputPath string, opts api_client.Pipelin } func (p *pdfPipeline) createThumbnail(inputPath string, opts api_client.PipelineRunOptions) error { - tmpPath := filepath.FromSlash(os.TempDir() + "/" + helper.NewID() + ".png") - // We don't consider failing the creation of the thumbnail as an error - _ = p.pdfProc.Thumbnail(inputPath, 0, p.config.Limits.ImagePreviewMaxHeight, tmpPath) + outputPath := filepath.FromSlash(os.TempDir() + "/" + helper.NewID() + ".png") + if err := p.pdfProc.Thumbnail(inputPath, 0, p.config.Limits.ImagePreviewMaxHeight, outputPath); err != nil { + return err + } defer func(path string) { if err := os.Remove(path); errors.Is(err, os.ErrNotExist) { return } else if err != nil { infra.GetLogger().Error(err) } - }(tmpPath) - props, err := p.imageProc.MeasureImage(tmpPath) + }(outputPath) + props, err := p.imageProc.MeasureImage(outputPath) if err != nil { return err } - stat, err := os.Stat(tmpPath) + stat, err := os.Stat(outputPath) if err != nil { return err } s3Object := &api_client.S3Object{ Bucket: opts.Bucket, - Key: opts.SnapshotID + "/thumbnail" + filepath.Ext(tmpPath), + Key: opts.SnapshotID + "/thumbnail" + filepath.Ext(outputPath), Image: props, Size: helper.ToPtr(stat.Size()), } - if err := p.s3.PutFile(s3Object.Key, tmpPath, helper.DetectMimeFromFile(tmpPath), s3Object.Bucket, minio.PutObjectOptions{}); err != nil { + if err := p.s3.PutFile(s3Object.Key, outputPath, helper.DetectMimeFromFile(outputPath), s3Object.Bucket, minio.PutObjectOptions{}); err != nil { return err } if err := p.snapshotClient.Patch(api_client.SnapshotPatchOptions{ diff --git a/conversion/processor/image_processor.go b/conversion/processor/image_processor.go index ed205d11c..b526b630d 100644 --- a/conversion/processor/image_processor.go +++ b/conversion/processor/image_processor.go @@ -39,12 +39,12 @@ func NewImageProcessor() *ImageProcessor { } type ThumbnailResult struct { - Success bool - Width *int - Height *int + Width int + Height int + IsCreated bool } -func (p *ImageProcessor) Thumbnail(inputPath string, outputPath string) (*ThumbnailResult, error) { +func (p *ImageProcessor) Thumbnail(inputPath string, width int, height int, outputPath string) (*ThumbnailResult, error) { props, err := p.MeasureImage(inputPath) if err != nil { return nil, err @@ -53,23 +53,27 @@ func (p *ImageProcessor) Thumbnail(inputPath string, outputPath string) (*Thumbn var newHeight int if props.Width > p.config.Limits.ImagePreviewMaxWidth || props.Height > p.config.Limits.ImagePreviewMaxHeight { if props.Width > props.Height { - newWidth, newHeight = helper.AspectRatio(p.config.Limits.ImagePreviewMaxWidth, 0, props.Width, props.Height) + newWidth, newHeight = helper.AspectRatio(width, 0, props.Width, props.Height) if err := p.ResizeImage(inputPath, newWidth, newHeight, outputPath); err != nil { return nil, err } } else { - newWidth, newHeight = helper.AspectRatio(0, p.config.Limits.ImagePreviewMaxHeight, props.Width, props.Height) + newWidth, newHeight = helper.AspectRatio(0, height, props.Width, props.Height) if err := p.ResizeImage(inputPath, newWidth, newHeight, outputPath); err != nil { return nil, err } } return &ThumbnailResult{ - Success: true, - Width: &newWidth, - Height: &newHeight, + Width: newWidth, + Height: newHeight, + IsCreated: true, + }, nil + } else { + return &ThumbnailResult{ + Width: props.Width, + Height: props.Height, }, nil } - return &ThumbnailResult{Success: false}, nil } func (p *ImageProcessor) MeasureImage(inputPath string) (*api_client.ImageProps, error) { diff --git a/conversion/processor/video_processor.go b/conversion/processor/video_processor.go index 1e7d417c4..5b2b14b3b 100644 --- a/conversion/processor/video_processor.go +++ b/conversion/processor/video_processor.go @@ -56,7 +56,7 @@ func (p *VideoProcessor) Thumbnail(inputPath string, width int, height int, outp return err } } else { - newWidth, newHeight := helper.AspectRatio(0, p.config.Limits.ImagePreviewMaxHeight, size.Width, size.Height) + newWidth, newHeight := helper.AspectRatio(0, height, size.Width, size.Height) if err := p.imageProc.ResizeImage(tmpPath, newWidth, newHeight, outputPath); err != nil { return err }