diff mbox series

[FFmpeg-devel,v2,1/3] avformat/apngdec: Return error for incomplete header

Message ID 20200112215651.1258-1-andreas.rheinhardt@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,v2,1/3] avformat/apngdec: Return error for incomplete header | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Andreas Rheinhardt Jan. 12, 2020, 9:56 p.m. UTC 
If avio_read() could read anything, it returns the number of bytes read,
even if it could not read as much as the caller desired.
apng_read_header() only checked the return value of its avio_read() calls
for being negative and this meant that it was possible for an incomplete
header to not be detected. The return value of the last successfull call
has been returned instead. This commit changes this.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
Now returning AVERROR_INVALIDDATA for truncated data.
I wonder whether a function that automatically returns an error when
the desired amount of data couldn't be read (instead of returning the
amount it read) would be worth it.
(Such a function should have a parameter indicating whether the current
position can be a legitimate position for EOF (so that AVERROR_EOF is
returned when nothing could be read due to EOF) or not (where
AVERROR_INVALIDDATA should be returned in such a case).)  

 libavformat/apngdec.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/apngdec.c b/libavformat/apngdec.c
index 0f1d04a365..36657877d2 100644
--- a/libavformat/apngdec.c
+++ b/libavformat/apngdec.c
@@ -138,8 +138,8 @@  static int append_extradata(AVCodecParameters *par, AVIOContext *pb, int len)
     par->extradata = new_extradata;
     par->extradata_size = new_size;
 
-    if ((ret = avio_read(pb, par->extradata + previous_size, len)) < 0)
-        return ret;
+    if ((ret = avio_read(pb, par->extradata + previous_size, len)) != len)
+        return (ret < 0 && ret != AVERROR_EOF) ? ret : AVERROR_INVALIDDATA;
 
     return previous_size;
 }
@@ -185,10 +185,10 @@  static int apng_read_header(AVFormatContext *s)
     AV_WL32(st->codecpar->extradata+4,  tag);
     AV_WB32(st->codecpar->extradata+8,  st->codecpar->width);
     AV_WB32(st->codecpar->extradata+12, st->codecpar->height);
-    if ((ret = avio_read(pb, st->codecpar->extradata+16, 9)) < 0)
-        goto fail;
+    if ((ret = avio_read(pb, st->codecpar->extradata + 16, 9)) != 9)
+        return (ret < 0 && ret != AVERROR_EOF) ? ret : AVERROR_INVALIDDATA;
 
-    while (!avio_feof(pb)) {
+    while (1) {
         if (acTL_found && ctx->num_play != 1) {
             int64_t size   = avio_size(pb);
             int64_t offset = avio_tell(pb);