[FFmpeg-devel] [PATCH 1/2] avformat/imfdec: do not use filesize when reading XML file

[FFmpeg-devel] [PATCH 1/2] avformat/imfdec: do not use filesize when reading XML file

[Marton Balint]

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)
-- 
2.31.1

_______________________________________________
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] [PATCH 2/2] avformat/dashdec: do not use filesize when reading XML file

[Marton Balint]

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/dashdec.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 797fe74157..0d21989e42 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -30,8 +30,6 @@
 #include "dash.h"
 
 #define INITIAL_BUFFER_SIZE 32768
-#define MAX_BPRINT_READ_SIZE (UINT_MAX - 1)
-#define DEFAULT_MANIFEST_SIZE 8 * 1024
 
 struct fragment {
     int64_t url_offset;
@@ -1195,7 +1193,6 @@ static int parse_manifest(AVFormatContext *s, const char *url, AVIOContext *in)
     DASHContext *c = s->priv_data;
     int ret = 0;
     int close_in = 0;
-    int64_t filesize = 0;
     AVBPrint buf;
     AVDictionary *opts = NULL;
     xmlDoc *doc = NULL;
@@ -1226,26 +1223,17 @@ static int parse_manifest(AVFormatContext *s, const char *url, AVIOContext *in)
     if (av_opt_get(in, "location", AV_OPT_SEARCH_CHILDREN, (uint8_t**)&c->base_url) < 0)
         c->base_url = av_strdup(url);
 
-    filesize = avio_size(in);
-    filesize = filesize > 0 ? filesize : DEFAULT_MANIFEST_SIZE;
+    av_bprint_init(&buf, 0, INT_MAX); // xmlReadMemory uses integer bufsize
 
-    if (filesize > MAX_BPRINT_READ_SIZE) {
-        av_log(s, AV_LOG_ERROR, "Manifest too large: %"PRId64"\n", filesize);
-        return AVERROR_INVALIDDATA;
-    }
-
-    av_bprint_init(&buf, filesize + 1, AV_BPRINT_SIZE_UNLIMITED);
-
-    if ((ret = avio_read_to_bprint(in, &buf, MAX_BPRINT_READ_SIZE)) < 0 ||
-        !avio_feof(in) ||
-        (filesize = buf.len) == 0) {
+    if ((ret = avio_read_to_bprint(in, &buf, SIZE_MAX)) < 0 ||
+        !avio_feof(in)) {
         av_log(s, AV_LOG_ERROR, "Unable to read to manifest '%s'\n", url);
         if (ret == 0)
             ret = AVERROR_INVALIDDATA;
     } else {
         LIBXML_TEST_VERSION
 
-        doc = xmlReadMemory(buf.str, filesize, c->base_url, NULL, 0);
+        doc = xmlReadMemory(buf.str, buf.len, c->base_url, NULL, 0);
         root_element = xmlDocGetRootElement(doc);
         node = root_element;
 
-- 
2.31.1

_______________________________________________
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 1/2] avformat/imfdec: do not use filesize when reading XML file

[Marton Balint]

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
_______________________________________________
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 1/2] avformat/imfdec: do not use filesize when reading XML file

[Pierre-Anthony Lemieux]

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".
_______________________________________________
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 1/2] avformat/imfdec: do not use filesize when reading XML file

[Marton Balint]

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
_______________________________________________
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".