[FFmpeg-devel] [PATCH] HTTP cookie support
Stefano Sabatini
stefasab at gmail.com
Sun Jan 13 22:18:00 CET 2013
On date Sunday 2013-01-13 14:09:21 -0500, Micah Galizia encoded:
> On Sun, Jan 13, 2013 at 8:43 AM, Stefano Sabatini <stefasab at gmail.com>wrote:
[...]
> > Alternatively we may want to support a syntax of the kind:
> > ffplay -proto http=cookies="....":... URI
> >
> > to force protocol and options.
> >
>
> I haven't seen that before, but I think I'll keep consistent with all of
> the other http options and stay with what is done (I'm assuming that's an
> optional recommendation as well).
Yes that syntax is currently just an idea in my mind.
> Attached are the same patches with the recommendations implemented.
>
> TIA
> --
> "The mark of an immature man is that he wants to die nobly for a cause,
> while the mark of the mature man is that he wants to live humbly for
> one." --W. Stekel
> From eecd6445401170ab559e7c003d3345ec1661b8b3 Mon Sep 17 00:00:00 2001
> From: Micah Galizia <micahgalizia at gmail.com>
> Date: Sun, 13 Jan 2013 14:00:47 -0500
> Subject: [PATCH 1/2] add HTTP protocol cookie support
>
> ---
> libavformat/http.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 125 insertions(+)
>
> diff --git a/libavformat/http.c b/libavformat/http.c
> index a9d952b..d2db450 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -64,6 +64,7 @@ typedef struct {
> int is_akamai;
> int rw_timeout;
> char *mime_type;
> + char *cookies; ///< holds newline (\n) delimited Set-Cookie header field values (without the "Set-Cookie: " field name)
> } HTTPContext;
>
> #define OFFSET(x) offsetof(HTTPContext, x)
> @@ -80,6 +81,7 @@ static const AVOption options[] = {
> {"post_data", "set custom HTTP post data", OFFSET(post_data), AV_OPT_TYPE_BINARY, .flags = D|E },
> {"timeout", "set timeout of socket I/O operations", OFFSET(rw_timeout), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, D|E },
> {"mime_type", "set MIME type", OFFSET(mime_type), AV_OPT_TYPE_STRING, {0}, 0, 0, 0 },
> +{"cookies", "set cookies to be sent in applicable future requests. Use newline delimited Set-Cookie HTTP field value syntax", OFFSET(cookies), AV_OPT_TYPE_STRING, {0}, 0, 0, 0 },
Nit+: to avoid two sentences:
set cookies to be sent in applicable future requests, use ...
> {NULL}
> };
> #define HTTP_CLASS(flavor)\
> @@ -359,11 +361,126 @@ static int process_line(URLContext *h, char *line, int line_count,
> s->is_akamai = 1;
> } else if (!av_strcasecmp (tag, "Content-Type")) {
> av_free(s->mime_type); s->mime_type = av_strdup(p);
> + } else if (!av_strcasecmp (tag, "Set-Cookie")) {
> + if (!s->cookies) {
> + if (!(s->cookies = av_strdup(p)))
> + return AVERROR(ENOMEM);
> + } else {
> + char *tmp = s->cookies;
> + size_t str_size = strlen(tmp) + strlen(p) + 2;
> + if (!(s->cookies = av_malloc(str_size))) {
> + s->cookies = tmp;
> + return AVERROR(ENOMEM);
> + }
> + snprintf(s->cookies, str_size, "%s\n%s", tmp, p);
> + av_free(tmp);
> + }
> }
> }
> return 1;
> }
>
> +/**
> + * Create a string containing cookie values for use as a HTTP cookie header
> + * field value for a particular path and domain from the cookie values stored in
> + * the HTTP protocol context. The cookie string is stored in *cookies.
> + *
> + * @param s HTTP context containing the cookie values.
> + * @param cookies pointer to the pointer to the string that will be output
> + * @param path the request path to match against the HTTP context cookie values
> + * @param domain the request domain to match against the HTTP context cookie values
> + * @return a negative value if an error condition occurred, 0 otherwise
Nit++: for internal documentation @param docs are usually overkill and
can be ignored
> + */
> +static int get_cookies(HTTPContext *s, char **cookies, const char *path,
> + const char *domain)
> +{
> + // cookie strings will look like Set-Cookie header field values. Multiple
> + // Set-Cookie fields will result in multiple values delimited by a newline
> + int ret = 0;
> + char *next, *cookie, *set_cookies = av_strdup(s->cookies);
> +
> + if (!set_cookies) return AVERROR(EINVAL);
> +
> + *cookies = NULL;
> + cookie = av_strtok(set_cookies, "\n", &next);
> + do {
> + int domain_offset = 0;
> + char *param, *next_param, *cdomain = NULL, *cpath = NULL, *cvalue = NULL;
> +
> +
Nit+++: double empty line
> + param = av_strtok(cookie, "; ", &next_param);
> + do {
> + if (!av_strncasecmp("path=", param, 5)) {
> + cpath = av_strdup(¶m[5]);
> + } else if (!av_strncasecmp("domain=", param, 7)) {
> + cdomain = av_strdup(¶m[7]);
> + } else if (!av_strncasecmp("secure", param, 6) ||
> + !av_strncasecmp("comment", param, 7) ||
> + !av_strncasecmp("max-age", param, 7) ||
> + !av_strncasecmp("version", param, 7)) {
Nit+++: weird indent, you can align the various !av_strncasecmp
> + // ignore Comment, Max-Age, Secure and Version
> + } else {
> + cvalue = av_strdup(param);
> + }
> +
> + param = av_strtok(NULL, "; ", &next_param);
> + } while (param);
alternatively if you want to remove the duplicated call to av_strtok():
while ((param = av_strtok(cookie, ...)) {
...
cookie = NULL;
}
> +
> + // ensure all of the necessary values are valid
> + if (!cdomain || !cpath || !cvalue) {
> + av_log(s, AV_LOG_WARNING, "Invalid cookie in protocol options.\n");
Please provide some more specific info, such as:
"Invalid cookie found, no domain or path or value specified\n"
Printing the cookie itself would be nice, but this would require to
strdup(cookie) since cookie is destroyed by av_strtok().
> + goto done_cookie;
> + }
> +
> + // check if the request path matches the cookie path
> + if (av_strncasecmp(path, cpath, strlen(cpath)))
> + goto done_cookie;
> +
> + // the domain should be at least the size of our cookie domain
> + domain_offset = strlen(domain) - strlen(cdomain);
> + if (domain_offset < 0)
> + goto done_cookie;
> +
> + // match the cookie domain
> + if (av_strcasecmp(&domain[domain_offset], cdomain))
> + goto done_cookie;
> +
> + // cookie parameters match, so copy the value
> + if (!*cookies) {
> + if (!(*cookies = av_strdup(cvalue))) {
> + ret = AVERROR(ENOMEM);
> + goto done_cookie;
> + }
> + } else {
> + char *tmp = *cookies;
> + size_t str_size = strlen(cvalue) + strlen(*cookies) + 3;
> + if (!(*cookies = av_malloc(str_size))) {
> + ret = AVERROR(ENOMEM);
> + goto done_cookie;
> + }
> + snprintf(*cookies, str_size, "%s; %s", tmp, cvalue);
> + av_free(tmp);
> + }
> +
> + done_cookie:
> + av_free(cdomain);
> + av_free(cpath);
> + av_free(cvalue);
> + if (ret) {
> + if (*cookies) av_freep(cookies);
> + av_free(set_cookies);
> + return ret;
> + }
> +
> + // move to the next cookie value
> + cookie = av_strtok(NULL, "\n", &next);
> + } while (cookie);
Same as above.
[...]
Looks good otherwise.
> From 79c539e55bd2ef4fb1f997372d84097968c130aa Mon Sep 17 00:00:00 2001
> From: Micah Galizia <micahgalizia at gmail.com>
> Date: Sun, 13 Jan 2013 14:02:17 -0500
> Subject: [PATCH 2/2] document HTTP protocol cookie support
>
> ---
> doc/protocols.texi | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
LGTM.
--
FFmpeg = Fabulous and Furious Mythic Patchable Embarassing Goblin
More information about the ffmpeg-devel
mailing list