Message ID | 20220101192001.13688-1-cus@passwd.hu |
---|---|
State | Accepted |
Commit | ae690d5cf51d8b1b09f8ed6799041311cfa57a6b |
Headers | show |
Series | [FFmpeg-devel,1/2] avformat/imfdec: do not use filesize when reading XML file | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
andriy/make_ppc | success | Make finished |
andriy/make_fate_ppc | fail | Make fate failed |
On Sat, 1 Jan 2022, Marton Balint wrote: > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > libavformat/imfdec.c | 15 ++++----------- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c > index f17064cfcd..ef1e7cf206 100644 > --- a/libavformat/imfdec.c > +++ b/libavformat/imfdec.c > @@ -73,8 +73,6 @@ > #include <inttypes.h> > #include <libxml/parser.h> > > -#define MAX_BPRINT_READ_SIZE (UINT_MAX - 1) > -#define DEFAULT_ASSETMAP_SIZE 8 * 1024 > #define AVRATIONAL_FORMAT "%d/%d" > #define AVRATIONAL_ARG(rational) rational.num, rational.den > > @@ -279,7 +277,6 @@ static int parse_assetmap(AVFormatContext *s, const char *url) > const char *base_url; > char *tmp_str = NULL; > int ret; > - int64_t filesize; > > av_log(s, AV_LOG_DEBUG, "Asset Map URL: %s\n", url); > > @@ -289,13 +286,10 @@ static int parse_assetmap(AVFormatContext *s, const char *url) > if (ret < 0) > return ret; > > - filesize = avio_size(in); > - filesize = filesize > 0 ? filesize : DEFAULT_ASSETMAP_SIZE; > + av_bprint_init(&buf, 0, INT_MAX); // xmlReadMemory uses integer length > > - av_bprint_init(&buf, filesize + 1, AV_BPRINT_SIZE_UNLIMITED); > - > - ret = avio_read_to_bprint(in, &buf, MAX_BPRINT_READ_SIZE); > - if (ret < 0 || !avio_feof(in) || buf.len == 0) { > + ret = avio_read_to_bprint(in, &buf, SIZE_MAX); > + if (ret < 0 || !avio_feof(in)) { > av_log(s, AV_LOG_ERROR, "Unable to read to asset map '%s'\n", url); > if (ret == 0) > ret = AVERROR_INVALIDDATA; > @@ -311,8 +305,7 @@ static int parse_assetmap(AVFormatContext *s, const char *url) > } > base_url = av_dirname(tmp_str); > > - filesize = buf.len; > - doc = xmlReadMemory(buf.str, filesize, url, NULL, 0); > + doc = xmlReadMemory(buf.str, buf.len, url, NULL, 0); > > ret = parse_imf_asset_map_from_xml_dom(s, doc, &c->asset_locator_map, base_url); > if (!ret) Will apply the series tomorrow. Regards, Marton
On Sat, Jan 8, 2022 at 10:49 AM Marton Balint <cus@passwd.hu> wrote: > > > > On Sat, 1 Jan 2022, Marton Balint wrote: > > > Signed-off-by: Marton Balint <cus@passwd.hu> > > --- > > libavformat/imfdec.c | 15 ++++----------- > > 1 file changed, 4 insertions(+), 11 deletions(-) > > > > diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c > > index f17064cfcd..ef1e7cf206 100644 > > --- a/libavformat/imfdec.c > > +++ b/libavformat/imfdec.c > > @@ -73,8 +73,6 @@ > > #include <inttypes.h> > > #include <libxml/parser.h> > > > > -#define MAX_BPRINT_READ_SIZE (UINT_MAX - 1) > > -#define DEFAULT_ASSETMAP_SIZE 8 * 1024 > > #define AVRATIONAL_FORMAT "%d/%d" > > #define AVRATIONAL_ARG(rational) rational.num, rational.den > > > > @@ -279,7 +277,6 @@ static int parse_assetmap(AVFormatContext *s, const char *url) > > const char *base_url; > > char *tmp_str = NULL; > > int ret; > > - int64_t filesize; > > > > av_log(s, AV_LOG_DEBUG, "Asset Map URL: %s\n", url); > > > > @@ -289,13 +286,10 @@ static int parse_assetmap(AVFormatContext *s, const char *url) > > if (ret < 0) > > return ret; > > > > - filesize = avio_size(in); > > - filesize = filesize > 0 ? filesize : DEFAULT_ASSETMAP_SIZE; > > + av_bprint_init(&buf, 0, INT_MAX); // xmlReadMemory uses integer length > > > > - av_bprint_init(&buf, filesize + 1, AV_BPRINT_SIZE_UNLIMITED); > > - > > - ret = avio_read_to_bprint(in, &buf, MAX_BPRINT_READ_SIZE); > > - if (ret < 0 || !avio_feof(in) || buf.len == 0) { > > + ret = avio_read_to_bprint(in, &buf, SIZE_MAX); > > + if (ret < 0 || !avio_feof(in)) { > > av_log(s, AV_LOG_ERROR, "Unable to read to asset map '%s'\n", url); > > if (ret == 0) > > ret = AVERROR_INVALIDDATA; > > @@ -311,8 +305,7 @@ static int parse_assetmap(AVFormatContext *s, const char *url) > > } > > base_url = av_dirname(tmp_str); > > > > - filesize = buf.len; > > - doc = xmlReadMemory(buf.str, filesize, url, NULL, 0); > > + doc = xmlReadMemory(buf.str, buf.len, url, NULL, 0); > > > > ret = parse_imf_asset_map_from_xml_dom(s, doc, &c->asset_locator_map, base_url); > > if (!ret) > > Will apply the series tomorrow. It looks like the same code exists at libavformat/imf_cpl.c [1]. [1] https://github.com/FFmpeg/FFmpeg/blob/ce4d459db186a7d8ac842685cd6256c9ac1b7f25/libavformat/imf_cpl.c#L795 Does it need to be changed there as well? > > Regards, > Marton > _______________________________________________ > 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".
On Sun, 9 Jan 2022, Pierre-Anthony Lemieux wrote: > On Sat, Jan 8, 2022 at 10:49 AM Marton Balint <cus@passwd.hu> wrote: >> >> >> >> On Sat, 1 Jan 2022, Marton Balint wrote: >> >>> Signed-off-by: Marton Balint <cus@passwd.hu> >>> --- >>> libavformat/imfdec.c | 15 ++++----------- >>> 1 file changed, 4 insertions(+), 11 deletions(-) >>> >>> diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c >>> index f17064cfcd..ef1e7cf206 100644 >>> --- a/libavformat/imfdec.c >>> +++ b/libavformat/imfdec.c >>> @@ -73,8 +73,6 @@ >>> #include <inttypes.h> >>> #include <libxml/parser.h> >>> >>> -#define MAX_BPRINT_READ_SIZE (UINT_MAX - 1) >>> -#define DEFAULT_ASSETMAP_SIZE 8 * 1024 >>> #define AVRATIONAL_FORMAT "%d/%d" >>> #define AVRATIONAL_ARG(rational) rational.num, rational.den >>> >>> @@ -279,7 +277,6 @@ static int parse_assetmap(AVFormatContext *s, const char *url) >>> const char *base_url; >>> char *tmp_str = NULL; >>> int ret; >>> - int64_t filesize; >>> >>> av_log(s, AV_LOG_DEBUG, "Asset Map URL: %s\n", url); >>> >>> @@ -289,13 +286,10 @@ static int parse_assetmap(AVFormatContext *s, const char *url) >>> if (ret < 0) >>> return ret; >>> >>> - filesize = avio_size(in); >>> - filesize = filesize > 0 ? filesize : DEFAULT_ASSETMAP_SIZE; >>> + av_bprint_init(&buf, 0, INT_MAX); // xmlReadMemory uses integer length >>> >>> - av_bprint_init(&buf, filesize + 1, AV_BPRINT_SIZE_UNLIMITED); >>> - >>> - ret = avio_read_to_bprint(in, &buf, MAX_BPRINT_READ_SIZE); >>> - if (ret < 0 || !avio_feof(in) || buf.len == 0) { >>> + ret = avio_read_to_bprint(in, &buf, SIZE_MAX); >>> + if (ret < 0 || !avio_feof(in)) { >>> av_log(s, AV_LOG_ERROR, "Unable to read to asset map '%s'\n", url); >>> if (ret == 0) >>> ret = AVERROR_INVALIDDATA; >>> @@ -311,8 +305,7 @@ static int parse_assetmap(AVFormatContext *s, const char *url) >>> } >>> base_url = av_dirname(tmp_str); >>> >>> - filesize = buf.len; >>> - doc = xmlReadMemory(buf.str, filesize, url, NULL, 0); >>> + doc = xmlReadMemory(buf.str, buf.len, url, NULL, 0); >>> >>> ret = parse_imf_asset_map_from_xml_dom(s, doc, &c->asset_locator_map, base_url); >>> if (!ret) >> >> Will apply the series tomorrow. > > It looks like the same code exists at libavformat/imf_cpl.c [1]. > > [1] https://github.com/FFmpeg/FFmpeg/blob/ce4d459db186a7d8ac842685cd6256c9ac1b7f25/libavformat/imf_cpl.c#L795 > > Does it need to be changed there as well? Yes, preferably that should be changed as well. No that it matters too much, it just better/simpler not to depend on filesize. Thanks, Marton
diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c index f17064cfcd..ef1e7cf206 100644 --- a/libavformat/imfdec.c +++ b/libavformat/imfdec.c @@ -73,8 +73,6 @@ #include <inttypes.h> #include <libxml/parser.h> -#define MAX_BPRINT_READ_SIZE (UINT_MAX - 1) -#define DEFAULT_ASSETMAP_SIZE 8 * 1024 #define AVRATIONAL_FORMAT "%d/%d" #define AVRATIONAL_ARG(rational) rational.num, rational.den @@ -279,7 +277,6 @@ static int parse_assetmap(AVFormatContext *s, const char *url) const char *base_url; char *tmp_str = NULL; int ret; - int64_t filesize; av_log(s, AV_LOG_DEBUG, "Asset Map URL: %s\n", url); @@ -289,13 +286,10 @@ static int parse_assetmap(AVFormatContext *s, const char *url) if (ret < 0) return ret; - filesize = avio_size(in); - filesize = filesize > 0 ? filesize : DEFAULT_ASSETMAP_SIZE; + av_bprint_init(&buf, 0, INT_MAX); // xmlReadMemory uses integer length - av_bprint_init(&buf, filesize + 1, AV_BPRINT_SIZE_UNLIMITED); - - ret = avio_read_to_bprint(in, &buf, MAX_BPRINT_READ_SIZE); - if (ret < 0 || !avio_feof(in) || buf.len == 0) { + ret = avio_read_to_bprint(in, &buf, SIZE_MAX); + if (ret < 0 || !avio_feof(in)) { av_log(s, AV_LOG_ERROR, "Unable to read to asset map '%s'\n", url); if (ret == 0) ret = AVERROR_INVALIDDATA; @@ -311,8 +305,7 @@ static int parse_assetmap(AVFormatContext *s, const char *url) } base_url = av_dirname(tmp_str); - filesize = buf.len; - doc = xmlReadMemory(buf.str, filesize, url, NULL, 0); + doc = xmlReadMemory(buf.str, buf.len, url, NULL, 0); ret = parse_imf_asset_map_from_xml_dom(s, doc, &c->asset_locator_map, base_url); if (!ret)
Signed-off-by: Marton Balint <cus@passwd.hu> --- libavformat/imfdec.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-)