Skip to content

Commit

Permalink
fix(conversion): SIGSEGV on small images (#345)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bouassaba
bouassaba authored Oct 5, 2024
1 parent 84436d8 commit 088fcda
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 51 deletions.
15 changes: 8 additions & 7 deletions conversion/pipeline/audio_video_pipeline.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -89,33 +90,33 @@ 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) {
if err := os.Remove(path); err != nil {
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{
Expand Down
22 changes: 11 additions & 11 deletions conversion/pipeline/glb_pipeline.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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{
Expand Down
22 changes: 11 additions & 11 deletions conversion/pipeline/image_pipeline.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand Down Expand Up @@ -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{
Expand Down
22 changes: 11 additions & 11 deletions conversion/pipeline/pdf_pipeline.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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."),
Expand All @@ -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{
Expand Down
24 changes: 14 additions & 10 deletions conversion/processor/image_processor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion conversion/processor/video_processor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 088fcda

Please sign in to comment.