On Wed, Jun 3, 2015 at 10:39 PM, Nicolas George <geo...@nsup.org> wrote:
> http_listen(), you wrote it yourself. Maybe I was not clear enough, I just
> suggested to move the code in a function for better readability (the name of
> the function is self-documentating), nothing more complicated.
I hope I understood what you meant.
>> From 3af5c5867672624d45de447d11c2107b3c77742c Mon Sep 17 00:00:00 2001
>> From: Stephan Holljes <klaxa1...@googlemail.com>
>> Date: Wed, 3 Jun 2015 20:57:53 +0200
>> Subject: [PATCH 5/5] lavf/http: Indent else-clause.
>>
>> Signed-off-by: Stephan Holljes <klaxa1...@googlemail.com>
>> ---
>> libavformat/http.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> This looks ok.
>
> Personally, I have the habit of inserting "/* TODO reindent */" in the code
> where it is needed and only make the reindent patch at the last moment, to
> reduce rebase nuisance.
The order of the patches attached was reversed, maybe that's why it
looked like the indentation was done first.
>
>> From 90f17ce9711304228bc3b7bf8cb4cdfac6b14011 Mon Sep 17 00:00:00 2001
>> From: Stephan Holljes <klaxa1...@googlemail.com>
>> Date: Wed, 3 Jun 2015 20:57:23 +0200
>> Subject: [PATCH 4/5] lavf/http: Properly process HTTP header on listen.
>>
>> Signed-off-by: Stephan Holljes <klaxa1...@googlemail.com>
>> ---
>> libavformat/http.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 43 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/http.c b/libavformat/http.c
>> index 6fad584..dfe2616 100644
>> --- a/libavformat/http.c
>> +++ b/libavformat/http.c
>> @@ -558,7 +558,7 @@ static int process_line(URLContext *h, char *line, int
>> line_count,
>> int *new_location)
>> {
>> HTTPContext *s = h->priv_data;
>> - char *tag, *p, *end;
>> + char *tag, *p, *end, *method, *resource, *version;
>> int ret;
>>
>> /* end of header */
>> @@ -569,6 +569,47 @@ static int process_line(URLContext *h, char *line, int
>> line_count,
>>
>> p = line;
>> if (line_count == 0) {
>> + if (s->listen) {
>> + // HTTP method
>
>> + while (av_isspace(*p))
>> + p++;
>
> I believe spaces are not allowed before the request.
Fixed.
>
>> + method = p;
>> + while (!av_isspace(*p))
>> + p++;
>
>> + *p = '\0';
>
> Plain 0 is acceptable, your choice.
The rest of the function uses '\0' and I wanted to keep it consistent.
>
>> + av_log(h, AV_LOG_TRACE, "Received method: %s\n", method);
>> + if (s->method) {
>> + if (av_strcasecmp(s->method, method)) {
>> + av_log(h, AV_LOG_ERROR, "Received and expected HTTP
>> method do not match. (%s expected, %s received)\n",
>> + s->method, method);
>> + return ff_http_averror(400, AVERROR(EIO));
>> + }
>> + }
>
>> + p++;
>
> Maybe better keep it grouped with "*p = 0", possibly in a single step:
> *(p++) = 0.
Fixed.
>
>> +
>> + // HTTP resource
>> + while (av_isspace(*p))
>> + p++;
>> + resource = p;
>> + while (!av_isspace(*p))
>> + p++;
>> + *p = '\0';
>> + p++;
>> + av_log(h, AV_LOG_TRACE, "Requested resource: %s\n", resource);
>> +
>> + // HTTP version
>> + while (av_isspace(*p))
>> + p++;
>> + version = p;
>> + while (!av_isspace(*p))
>> + p++;
>> + *p = '\0';
>> + if (av_strncasecmp(version, "HTTP/", 5)) {
>> + av_log(h, AV_LOG_ERROR, "Malformed HTTP version string.\n");
>> + return ff_http_averror(400, AVERROR(EIO));
>> + }
>> + av_log(h, AV_LOG_TRACE, "HTTP version string: %s\n", version);
>> + } else {
>> while (!av_isspace(*p) && *p != '\0')
>> p++;
>> while (av_isspace(*p))
>> @@ -579,6 +620,7 @@ static int process_line(URLContext *h, char *line, int
>> line_count,
>>
>> if ((ret = check_http_code(h, s->http_code, end)) < 0)
>> return ret;
>> + }
>> } else {
>> while (*p != '\0' && *p != ':')
>> p++;
>> --
>> 2.1.0
>>
>
>> From bbe015d20c1852e7c0c8885ca49a8ba2d926911d Mon Sep 17 00:00:00 2001
>> From: Stephan Holljes <klaxa1...@googlemail.com>
>> Date: Wed, 3 Jun 2015 20:45:08 +0200
>> Subject: [PATCH 3/5] lavf/http: Rudimentary error handling for HTTP requests
>> received from clients.
>>
>> Signed-off-by: Stephan Holljes <klaxa1...@googlemail.com>
>> ---
>> libavformat/http.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/libavformat/http.c b/libavformat/http.c
>> index e51f524..6fad584 100644
>> --- a/libavformat/http.c
>> +++ b/libavformat/http.c
>> @@ -304,6 +304,9 @@ static int http_listen(URLContext *h, const char *uri,
>> int flags,
>> HTTPContext *s = h->priv_data;
>> int ret;
>> static const char header[] = "HTTP/1.1 200 OK\r\nContent-Type:
>> application/octet-stream\r\nTransfer-Encoding: chunked\r\n\r\n";
>> + static const char bad_request[] = "HTTP/1.1 400 Bad
>> Request\r\nContent-Type: text/plain\r\n\r\n400 Bad Request\r\n";
>> + static const char internal_server_error[] = "HTTP/1.1 500 Internal
>> server error\r\nContent-Type: text/plain\r\n\r\n500 Internal server
>> error\r\n";
>> +
>> char hostname[1024], proto[10];
>> char lower_url[100];
>> const char *lower_proto = "tcp";
>> @@ -325,6 +328,16 @@ static int http_listen(URLContext *h, const char *uri,
>> int flags,
>> return 0;
>>
>> fail:
>> + if (h->is_connected) {
>> + switch(ret) {
>> + case AVERROR_HTTP_BAD_REQUEST:
>> + ffurl_write(s->hd, bad_request, strlen(bad_request));
>> + break;
>> + default:
>
>> + av_log(h, AV_LOG_ERROR, "Internal server error.\n");
>
> "Unhandled HTTP error"? Your choice.
>
>> + ffurl_write(s->hd, internal_server_error,
>> strlen(internal_server_error));
>> + }
>> + }
>> av_dict_free(&s->chained_options);
>> return ret;
>> }
>> --
>> 2.1.0
>>
>
>> From a1a13c83724f3b2ec0ee46efbf6e84e123e5538d Mon Sep 17 00:00:00 2001
>> From: Stephan Holljes <klaxa1...@googlemail.com>
>> Date: Wed, 3 Jun 2015 20:41:50 +0200
>> Subject: [PATCH 2/5] lavf/http: Process HTTP header before sending response.
>>
>> Signed-off-by: Stephan Holljes <klaxa1...@googlemail.com>
>> ---
>> libavformat/http.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> LGTM. Will test when time permits.
>
>> From 40cd84a08ab76fcf5d22e6ca47e3aa3cb4bc0bc1 Mon Sep 17 00:00:00 2001
>> From: Stephan Holljes <klaxa1...@googlemail.com>
>> Date: Wed, 3 Jun 2015 20:26:37 +0200
>> Subject: [PATCH 1/5] lavf/http: Document method option.
>>
>> Signed-off-by: Stephan Holljes <klaxa1...@googlemail.com>
>> ---
>> doc/protocols.texi | 9 +++++++++
>> libavformat/http.c | 2 +-
>> 2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/protocols.texi b/doc/protocols.texi
>> index f822d81..3a4dbb0 100644
>> --- a/doc/protocols.texi
>> +++ b/doc/protocols.texi
>> @@ -278,6 +278,15 @@ Set initial byte offset.
>> @item end_offset
>> Try to limit the request to bytes preceding this offset.
>>
>> +@item method
>> +When used as a client option it sets the HTTP method for the request.
>> +
>> +When used as a server option it sets the HTTP method that is going to be
>> +expected from the client(s).
>> +If the expected and the received HTTP method do not match the client will
>> +be given a Bad Request response.
>
>> +When unset the HTTP method is not checked.
>
> Maybe add something like "not checked for now" or "but this may change", so
> that people will not start relying on it. But this is very minor.
Also fixed.
>
>> +
>> @item listen
>> If set to 1 enables experimental HTTP server. This can be used to send data
>> when
>> used as an output option, or read data from a client with HTTP POST when
>> used as
>> diff --git a/libavformat/http.c b/libavformat/http.c
>> index 4f6716a..546741a 100644
>> --- a/libavformat/http.c
>> +++ b/libavformat/http.c
>> @@ -126,7 +126,7 @@ static const AVOption options[] = {
>> { "location", "The actual location of the data received",
>> OFFSET(location), AV_OPT_TYPE_STRING, { 0 }, 0, 0, D | E },
>> { "offset", "initial byte offset", OFFSET(off), AV_OPT_TYPE_INT64, {
>> .i64 = 0 }, 0, INT64_MAX, D },
>> { "end_offset", "try to limit the request to bytes preceding this
>> offset", OFFSET(end_off), AV_OPT_TYPE_INT64, { .i64 = 0 }, 0, INT64_MAX, D },
>> - { "method", "Override the HTTP method", OFFSET(method),
>> AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
>> + { "method", "Override the HTTP method or set the expected HTTP method
>> from a client", OFFSET(method), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D
>> | E },
>> { "reconnect", "auto reconnect after disconnect before EOF",
>> OFFSET(reconnect), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, D },
>> { "listen", "listen on HTTP", OFFSET(listen), AV_OPT_TYPE_INT, { .i64 =
>> 0 }, 0, 1, D | E },
>> { NULL }
>
> LGTM apart from that.
>
> Regards,
>
> --
> Nicolas George
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
From 5ec28499e32ede262c690c556aef5e54fd3ef5d7 Mon Sep 17 00:00:00 2001
From: Stephan Holljes <klaxa1...@googlemail.com>
Date: Thu, 4 Jun 2015 01:16:59 +0200
Subject: [PATCH 1/5] lavf/http: Document method option.
Signed-off-by: Stephan Holljes <klaxa1...@googlemail.com>
---
doc/protocols.texi | 10 ++++++++++
libavformat/http.c | 2 +-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/doc/protocols.texi b/doc/protocols.texi
index f822d81..453dbcf 100644
--- a/doc/protocols.texi
+++ b/doc/protocols.texi
@@ -278,6 +278,16 @@ Set initial byte offset.
@item end_offset
Try to limit the request to bytes preceding this offset.
+@item method
+When used as a client option it sets the HTTP method for the request.
+
+When used as a server option it sets the HTTP method that is going to be
+expected from the client(s).
+If the expected and the received HTTP method do not match the client will
+be given a Bad Request response.
+When unset the HTTP method is not checked for now. This will be replaced by
+autodetection in the future.
+
@item listen
If set to 1 enables experimental HTTP server. This can be used to send data when
used as an output option, or read data from a client with HTTP POST when used as
diff --git a/libavformat/http.c b/libavformat/http.c
index 4f6716a..546741a 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -126,7 +126,7 @@ static const AVOption options[] = {
{ "location", "The actual location of the data received", OFFSET(location), AV_OPT_TYPE_STRING, { 0 }, 0, 0, D | E },
{ "offset", "initial byte offset", OFFSET(off), AV_OPT_TYPE_INT64, { .i64 = 0 }, 0, INT64_MAX, D },
{ "end_offset", "try to limit the request to bytes preceding this offset", OFFSET(end_off), AV_OPT_TYPE_INT64, { .i64 = 0 }, 0, INT64_MAX, D },
- { "method", "Override the HTTP method", OFFSET(method), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
+ { "method", "Override the HTTP method or set the expected HTTP method from a client", OFFSET(method), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D | E },
{ "reconnect", "auto reconnect after disconnect before EOF", OFFSET(reconnect), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, D },
{ "listen", "listen on HTTP", OFFSET(listen), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, D | E },
{ NULL }
--
2.1.0
From 6e3f560af5f8d9c87154f60c5de0e66eada26b63 Mon Sep 17 00:00:00 2001
From: Stephan Holljes <klaxa1...@googlemail.com>
Date: Thu, 4 Jun 2015 01:17:51 +0200
Subject: [PATCH 2/5] lavf/http: Process HTTP header before sending response.
Signed-off-by: Stephan Holljes <klaxa1...@googlemail.com>
---
libavformat/http.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavformat/http.c b/libavformat/http.c
index 546741a..e51f524 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -318,10 +318,10 @@ static int http_listen(URLContext *h, const char *uri, int flags,
if ((ret = ffurl_open(&s->hd, lower_url, AVIO_FLAG_READ_WRITE,
&h->interrupt_callback, options)) < 0)
goto fail;
- if ((ret = ffurl_write(s->hd, header, strlen(header))) < 0)
- goto fail;
if ((ret = http_read_header(h, &new_location)) < 0)
goto fail;
+ if ((ret = ffurl_write(s->hd, header, strlen(header))) < 0)
+ goto fail;
return 0;
fail:
--
2.1.0
From 500462610f1389a4e8dfa76ca4598500691ea6a7 Mon Sep 17 00:00:00 2001
From: Stephan Holljes <klaxa1...@googlemail.com>
Date: Thu, 4 Jun 2015 01:20:28 +0200
Subject: [PATCH 3/5] lavf/http: Rudimentary error handling for HTTP requests
received from clients.
Signed-off-by: Stephan Holljes <klaxa1...@googlemail.com>
---
libavformat/http.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/libavformat/http.c b/libavformat/http.c
index e51f524..965967d 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -299,11 +299,28 @@ int ff_http_averror(int status_code, int default_averror)
return default_averror;
}
+static void handle_http_errors(URLContext *h, int error) {
+ static const char bad_request[] = "HTTP/1.1 400 Bad Request\r\nContent-Type: text/plain\r\n\r\n400 Bad Request\r\n";
+ static const char internal_server_error[] = "HTTP/1.1 500 Internal server error\r\nContent-Type: text/plain\r\n\r\n500 Internal server error\r\n";
+ HTTPContext *s = h->priv_data;
+ if (h->is_connected) {
+ switch(error) {
+ case AVERROR_HTTP_BAD_REQUEST:
+ ffurl_write(s->hd, bad_request, strlen(bad_request));
+ break;
+ default:
+ av_log(h, AV_LOG_ERROR, "Unhandled HTTP error.\n");
+ ffurl_write(s->hd, internal_server_error, strlen(internal_server_error));
+ }
+ }
+}
+
static int http_listen(URLContext *h, const char *uri, int flags,
AVDictionary **options) {
HTTPContext *s = h->priv_data;
int ret;
static const char header[] = "HTTP/1.1 200 OK\r\nContent-Type: application/octet-stream\r\nTransfer-Encoding: chunked\r\n\r\n";
+
char hostname[1024], proto[10];
char lower_url[100];
const char *lower_proto = "tcp";
@@ -325,6 +342,7 @@ static int http_listen(URLContext *h, const char *uri, int flags,
return 0;
fail:
+ handle_http_errors(h, ret);
av_dict_free(&s->chained_options);
return ret;
}
--
2.1.0
From 986e5fe885ec267357855702c3075db1fea944e5 Mon Sep 17 00:00:00 2001
From: Stephan Holljes <klaxa1...@googlemail.com>
Date: Thu, 4 Jun 2015 01:21:02 +0200
Subject: [PATCH 4/5] lavf/http: Properly process HTTP header on listen.
Signed-off-by: Stephan Holljes <klaxa1...@googlemail.com>
---
libavformat/http.c | 41 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/libavformat/http.c b/libavformat/http.c
index 965967d..b841f3f 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -563,7 +563,7 @@ static int process_line(URLContext *h, char *line, int line_count,
int *new_location)
{
HTTPContext *s = h->priv_data;
- char *tag, *p, *end;
+ char *tag, *p, *end, *method, *resource, *version;
int ret;
/* end of header */
@@ -574,6 +574,44 @@ static int process_line(URLContext *h, char *line, int line_count,
p = line;
if (line_count == 0) {
+ if (s->listen) {
+ // HTTP method
+ method = p;
+ while (!av_isspace(*p))
+ p++;
+ *(p++) = '\0';
+ av_log(h, AV_LOG_TRACE, "Received method: %s\n", method);
+ if (s->method) {
+ if (av_strcasecmp(s->method, method)) {
+ av_log(h, AV_LOG_ERROR, "Received and expected HTTP method do not match. (%s expected, %s received)\n",
+ s->method, method);
+ return ff_http_averror(400, AVERROR(EIO));
+ }
+ }
+
+ // HTTP resource
+ while (av_isspace(*p))
+ p++;
+ resource = p;
+ while (!av_isspace(*p))
+ p++;
+ *(p++) = '\0';
+ av_log(h, AV_LOG_TRACE, "Requested resource: %s\n", resource);
+
+ // HTTP version
+ while (av_isspace(*p))
+ p++;
+ version = p;
+ while (!av_isspace(*p))
+ p++;
+ *p = '\0';
+ if (av_strncasecmp(version, "HTTP/", 5)) {
+ av_log(h, AV_LOG_ERROR, "Malformed HTTP version string.\n");
+ return ff_http_averror(400, AVERROR(EIO));
+ }
+ av_log(h, AV_LOG_TRACE, "HTTP version string: %s\n", version);
+ } else {
+ /* TODO: reindent */
while (!av_isspace(*p) && *p != '\0')
p++;
while (av_isspace(*p))
@@ -584,6 +622,7 @@ static int process_line(URLContext *h, char *line, int line_count,
if ((ret = check_http_code(h, s->http_code, end)) < 0)
return ret;
+ }
} else {
while (*p != '\0' && *p != ':')
p++;
--
2.1.0
From 960333650d17dbb1646dbbd236ec535717b7a1eb Mon Sep 17 00:00:00 2001
From: Stephan Holljes <klaxa1...@googlemail.com>
Date: Thu, 4 Jun 2015 01:21:26 +0200
Subject: [PATCH 5/5] lavf/http: Indent else-clause.
Signed-off-by: Stephan Holljes <klaxa1...@googlemail.com>
---
libavformat/http.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/libavformat/http.c b/libavformat/http.c
index b841f3f..90e88ad 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -611,17 +611,16 @@ static int process_line(URLContext *h, char *line, int line_count,
}
av_log(h, AV_LOG_TRACE, "HTTP version string: %s\n", version);
} else {
- /* TODO: reindent */
- while (!av_isspace(*p) && *p != '\0')
- p++;
- while (av_isspace(*p))
- p++;
- s->http_code = strtol(p, &end, 10);
+ while (!av_isspace(*p) && *p != '\0')
+ p++;
+ while (av_isspace(*p))
+ p++;
+ s->http_code = strtol(p, &end, 10);
- av_log(h, AV_LOG_TRACE, "http_code=%d\n", s->http_code);
+ av_log(h, AV_LOG_TRACE, "http_code=%d\n", s->http_code);
- if ((ret = check_http_code(h, s->http_code, end)) < 0)
- return ret;
+ if ((ret = check_http_code(h, s->http_code, end)) < 0)
+ return ret;
}
} else {
while (*p != '\0' && *p != ':')
--
2.1.0
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
