On Thu, 21 Nov 2013 15:30:01 +0200 (EET), Martin Storsjö <[email protected]> 
wrote:
> On Thu, 21 Nov 2013, Anton Khirnov wrote:
> 
> >
> > On Thu, 21 Nov 2013 11:57:09 +0200, Martin Storsjö <[email protected]> wrote:
> >> ---
> >>  libavformat/http.c    |   20 +++++++++++++++-----
> >>  libavformat/version.h |    2 +-
> >>  2 files changed, 16 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/libavformat/http.c b/libavformat/http.c
> >> index eb08dfe..96f56f8 100644
> >> --- a/libavformat/http.c
> >> +++ b/libavformat/http.c
> >> @@ -51,7 +51,7 @@ typedef struct {
> >>      int http_code;
> >>      int64_t chunksize;      /**< Used if "Transfer-Encoding: chunked" 
> >> otherwise -1. */
> >>      int64_t off, filesize;
> >> -    char location[MAX_URL_SIZE];
> >> +    char *location;
> >>      HTTPAuthState auth_state;
> >>      HTTPAuthState proxy_auth_state;
> >>      char *headers;
> >> @@ -83,6 +83,7 @@ static const AVOption options[] = {
> >>  {"none", "No auth method set, autodetect", 0, AV_OPT_TYPE_CONST, {.i64 = 
> >> HTTP_AUTH_NONE}, 0, 0, D|E, "auth_type" },
> >>  {"basic", "HTTP basic authentication", 0, AV_OPT_TYPE_CONST, {.i64 = 
> >> HTTP_AUTH_BASIC}, 0, 0, D|E, "auth_type" },
> >>  {"send_expect_100", "Force sending an Expect: 100-continue header for 
> >> POST", OFFSET(send_expect_100), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, E },
> >> +{"location", "The actual location of the data received", 
> >> OFFSET(location), AV_OPT_TYPE_STRING, { 0 }, 0, 0, D|E },
> >>  {NULL}
> >>  };
> >>  #define HTTP_CLASS(flavor)\
> >> @@ -218,7 +219,10 @@ int ff_http_do_new_request(URLContext *h, const char 
> >> *uri)
> >>      int ret;
> >>
> >>      s->off = 0;
> >> -    av_strlcpy(s->location, uri, sizeof(s->location));
> >> +    av_free(s->location);
> >> +    s->location = av_strdup(uri);
> >> +    if (!s->location)
> >> +        return AVERROR(ENOMEM);
> >>
> >>      av_dict_copy(&options, s->chained_options, 0);
> >>      ret = http_open_cnx(h, &options);
> >> @@ -235,7 +239,9 @@ static int http_open(URLContext *h, const char *uri, 
> >> int flags,
> >>      h->is_streamed = 1;
> >>
> >>      s->filesize = -1;
> >> -    av_strlcpy(s->location, uri, sizeof(s->location));
> >> +    s->location = av_strdup(uri);
> >> +    if (!s->location)
> >> +        return AVERROR(ENOMEM);
> >>      if (options)
> >>          av_dict_copy(&s->chained_options, *options, 0);
> >>
> >> @@ -335,10 +341,14 @@ static int process_line(URLContext *h, char *line, 
> >> int line_count,
> >>          while (av_isspace(*p))
> >>              p++;
> >>          if (!av_strcasecmp(tag, "Location")) {
> >> -            char redirected_location[MAX_URL_SIZE];
> >> +            char redirected_location[MAX_URL_SIZE], *new_loc;
> >>              ff_make_absolute_url(redirected_location, 
> >> sizeof(redirected_location),
> >>                                   s->location, p);
> >> -            av_strlcpy(s->location, redirected_location, 
> >> sizeof(s->location));
> >> +            new_loc = av_strdup(redirected_location);
> >> +            if (!new_loc)
> >> +                return AVERROR(ENOMEM);
> >> +            av_free(s->location);
> >> +            s->location = new_loc;
> >
> > Is it really better to leave the old location rather than NULL there?
> 
> I did this as a safety precation - previously s->location was always 
> initialized and non-null, so the code could possibly check it in a number 
> of places. Even if we return an error here, we're pretty deep in a header 
> parsing routine, so there's a number of calling functions whose error 
> paths have to be checked to be sure it doesn't try to use it anywhere if 
> we fail here. I don't think it actually is touched, but I felt it would be 
> safer this way.

Ok then.

-- 
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to