[FFmpeg-devel] [PATCH] libavformat/flac_picture: Don't return AVERROR_INVALIDDATA for errors with flac picture mimetype

[FFmpeg-devel] [PATCH] libavformat/flac_picture: Don't return AVERROR_INVALIDDATA for errors with flac picture mimetype

[Will Cassella]

In the case where the FLAC picture MIME type is not understood, fail to
parse the picture silently rather than return AVERROR_INVALIDDATA.

This originated from a bug reported in Chromium: https://crbug.com/1052821

Signed-off-by: Will Cassella <cassew@google.com>
---
 libavformat/flac_picture.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/libavformat/flac_picture.c b/libavformat/flac_picture.c
index b33fee75b4..1acad9b251 100644
--- a/libavformat/flac_picture.c
+++ b/libavformat/flac_picture.c
@@ -68,8 +68,6 @@ int ff_flac_parse_picture(AVFormatContext *s,
uint8_t **bufp, int buf_size,
     if (len <= 0 || len >= sizeof(mimetype)) {
         av_log(s, AV_LOG_ERROR, "Could not read mimetype from an attached "
                "picture.\n");
-        if (s->error_recognition & AV_EF_EXPLODE)
-            return AVERROR_INVALIDDATA;
         return 0;
     }
     if (len + 24 > bytestream2_get_bytes_left(&g)) {
@@ -91,8 +89,6 @@ int ff_flac_parse_picture(AVFormatContext *s,
uint8_t **bufp, int buf_size,
     if (id == AV_CODEC_ID_NONE) {
         av_log(s, AV_LOG_ERROR, "Unknown attached picture mimetype: %s.\n",
                mimetype);
-        if (s->error_recognition & AV_EF_EXPLODE)
-            return AVERROR_INVALIDDATA;
         return 0;
     }

-- 
2.37.2.789.g6183377224-goog
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavformat/flac_picture: Don't return AVERROR_INVALIDDATA for errors with flac picture mimetype

[James Almer]

On 9/9/2022 7:44 PM, Will Cassella wrote:
> In the case where the FLAC picture MIME type is not understood, fail to
> parse the picture silently rather than return AVERROR_INVALIDDATA.
> 
> This originated from a bug reported in Chromium: https://crbug.com/1052821
> 
> Signed-off-by: Will Cassella <cassew@google.com>
> ---
>   libavformat/flac_picture.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/libavformat/flac_picture.c b/libavformat/flac_picture.c
> index b33fee75b4..1acad9b251 100644
> --- a/libavformat/flac_picture.c
> +++ b/libavformat/flac_picture.c
> @@ -68,8 +68,6 @@ int ff_flac_parse_picture(AVFormatContext *s,
> uint8_t **bufp, int buf_size,
>       if (len <= 0 || len >= sizeof(mimetype)) {
>           av_log(s, AV_LOG_ERROR, "Could not read mimetype from an attached "
>                  "picture.\n");
> -        if (s->error_recognition & AV_EF_EXPLODE)
> -            return AVERROR_INVALIDDATA;

If you don't want to error out, then don't enable explode mode, which is 
meant to abort on the slightest issue?

>           return 0;
>       }
>       if (len + 24 > bytestream2_get_bytes_left(&g)) {
> @@ -91,8 +89,6 @@ int ff_flac_parse_picture(AVFormatContext *s,
> uint8_t **bufp, int buf_size,
>       if (id == AV_CODEC_ID_NONE) {
>           av_log(s, AV_LOG_ERROR, "Unknown attached picture mimetype: %s.\n",
>                  mimetype);
> -        if (s->error_recognition & AV_EF_EXPLODE)
> -            return AVERROR_INVALIDDATA;
>           return 0;
>       }
> 
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavformat/flac_picture: Don't return AVERROR_INVALIDDATA for errors with flac picture mimetype

[Will Cassella]

Sorry for the delay, didn't see your reply!

The issue is that we do want to error out most of the time on invalid
input, but for our needs the mime-type of the attached picture being
empty or unknown isn't really relevant. Could there be a way to
classify this error differently so that it could be ignored, while
keeping AV_EF_EXPLODE enabled for everything else?

Thanks,
Will

On Fri, Sep 9, 2022 at 6:16 PM James Almer <jamrial@gmail.com> wrote:
>
> On 9/9/2022 7:44 PM, Will Cassella wrote:
> > In the case where the FLAC picture MIME type is not understood, fail to
> > parse the picture silently rather than return AVERROR_INVALIDDATA.
> >
> > This originated from a bug reported in Chromium: https://crbug.com/1052821
> >
> > Signed-off-by: Will Cassella <cassew@google.com>
> > ---
> >   libavformat/flac_picture.c | 4 ----
> >   1 file changed, 4 deletions(-)
> >
> > diff --git a/libavformat/flac_picture.c b/libavformat/flac_picture.c
> > index b33fee75b4..1acad9b251 100644
> > --- a/libavformat/flac_picture.c
> > +++ b/libavformat/flac_picture.c
> > @@ -68,8 +68,6 @@ int ff_flac_parse_picture(AVFormatContext *s,
> > uint8_t **bufp, int buf_size,
> >       if (len <= 0 || len >= sizeof(mimetype)) {
> >           av_log(s, AV_LOG_ERROR, "Could not read mimetype from an attached "
> >                  "picture.\n");
> > -        if (s->error_recognition & AV_EF_EXPLODE)
> > -            return AVERROR_INVALIDDATA;
>
> If you don't want to error out, then don't enable explode mode, which is
> meant to abort on the slightest issue?
>
> >           return 0;
> >       }
> >       if (len + 24 > bytestream2_get_bytes_left(&g)) {
> > @@ -91,8 +89,6 @@ int ff_flac_parse_picture(AVFormatContext *s,
> > uint8_t **bufp, int buf_size,
> >       if (id == AV_CODEC_ID_NONE) {
> >           av_log(s, AV_LOG_ERROR, "Unknown attached picture mimetype: %s.\n",
> >                  mimetype);
> > -        if (s->error_recognition & AV_EF_EXPLODE)
> > -            return AVERROR_INVALIDDATA;
> >           return 0;
> >       }
> >
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavformat/flac_picture: Don't return AVERROR_INVALIDDATA for errors with flac picture mimetype

[Anton Khirnov]

Quoting James Almer (2022-09-10 03:16:32)
> On 9/9/2022 7:44 PM, Will Cassella wrote:
> > In the case where the FLAC picture MIME type is not understood, fail to
> > parse the picture silently rather than return AVERROR_INVALIDDATA.
> > 
> > This originated from a bug reported in Chromium: https://crbug.com/1052821
> > 
> > Signed-off-by: Will Cassella <cassew@google.com>
> > ---
> >   libavformat/flac_picture.c | 4 ----
> >   1 file changed, 4 deletions(-)
> > 
> > diff --git a/libavformat/flac_picture.c b/libavformat/flac_picture.c
> > index b33fee75b4..1acad9b251 100644
> > --- a/libavformat/flac_picture.c
> > +++ b/libavformat/flac_picture.c
> > @@ -68,8 +68,6 @@ int ff_flac_parse_picture(AVFormatContext *s,
> > uint8_t **bufp, int buf_size,
> >       if (len <= 0 || len >= sizeof(mimetype)) {
> >           av_log(s, AV_LOG_ERROR, "Could not read mimetype from an attached "
> >                  "picture.\n");
> > -        if (s->error_recognition & AV_EF_EXPLODE)
> > -            return AVERROR_INVALIDDATA;
> 
> If you don't want to error out, then don't enable explode mode, which is 
> meant to abort on the slightest issue?

IMO failing to recognize the MIME type is a lavf error rather than a
file error and so should not fail, much less with AVERROR_INVALIDDATA
(should be ENOSYS if anything).

THe other error should stay though - then the file actually is broken.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavformat/flac_picture: Don't return AVERROR_INVALIDDATA for errors with flac picture mimetype

[Dale Curtis]

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]

Will left Google, but I've updated the patch to only skip errors when the
type is unrecognized.

On Wed, Sep 28, 2022 at 7:17 AM Anton Khirnov <anton@khirnov.net> wrote:

> IMO failing to recognize the MIME type is a lavf error rather than a
> file error and so should not fail, much less with AVERROR_INVALIDDATA
> (should be ENOSYS if anything).
>

[-- Attachment #2: no-error-on-unrecognized.patch --]
[-- Type: application/octet-stream, Size: 1034 bytes --]

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavformat/flac_picture: Don't return AVERROR_INVALIDDATA for errors with flac picture mimetype

[Dale Curtis]

Bump on this patch -- it still applies cleanly. We're still seeing files in
the wild like this.

- dale

On Mon, Apr 10, 2023 at 10:54 AM Dale Curtis <dalecurtis@chromium.org>
wrote:

> Will left Google, but I've updated the patch to only skip errors when the
> type is unrecognized.
>
> On Wed, Sep 28, 2022 at 7:17 AM Anton Khirnov <anton@khirnov.net> wrote:
>
>> IMO failing to recognize the MIME type is a lavf error rather than a
>> file error and so should not fail, much less with AVERROR_INVALIDDATA
>> (should be ENOSYS if anything).
>>
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavformat/flac_picture: Don't return AVERROR_INVALIDDATA for errors with flac picture mimetype

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 360 bytes --]

On Wed, Mar 27, 2024 at 01:05:52PM -0700, Dale Curtis wrote:
> Bump on this patch -- it still applies cleanly. We're still seeing files in
> the wild like this.

will apply

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".