[FFmpeg-devel] [PATCH 2/2] lavf/http: Add error codes 301 and 503 and make replies more customizable

[Stephan Holljes]

On Thu, Aug 20, 2015 at 11:08 AM, Nicolas George <george at nsup.org> wrote:
> Le tridi 3 fructidor, an CCXXIII, Stephan Holljes a écrit :
>> Signed-off-by: Stephan Holljes <klaxa1337 at googlemail.com>
>> ---
>>  libavformat/http.c | 44 +++++++++++++++++++++++++++++++++++++++-----
>>  1 file changed, 39 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavformat/http.c b/libavformat/http.c
>> index bfe6801..996c130 100644
>> --- a/libavformat/http.c
>> +++ b/libavformat/http.c
>> @@ -331,9 +331,10 @@ int ff_http_averror(int status_code, int default_averror)
>>  static int http_write_reply(URLContext* h, int status_code)
>>  {
>>      int ret, body = 0, reply_code, message_len;
>> -    const char *reply_text, *content_type;
>> +    const char *reply_text, *content_type, *location = NULL, *headers = "\r\n";
>>      HTTPContext *s = h->priv_data;
>>      char message[BUFFER_SIZE];
>> +    char body_content[BUFFER_SIZE];
>>      content_type = "text/plain";
>>
>>      if (status_code < 0)
>> @@ -359,28 +360,61 @@ static int http_write_reply(URLContext* h, int status_code)
>>          reply_text = "OK";
>>          content_type = "application/octet-stream";
>>          break;
>> +    case 301:
>> +        reply_code = 301;
>> +        reply_text = "Moved Permanently";
>
>> +        if (av_strcasecmp(s->method, "HEAD")) {
>
> Why only for HEAD?

It's the opposite. When the request was HEAD, the location header is
not set, otherwise it is.

>
>> +            if (!s->location) {
>> +                av_log(s, AV_LOG_WARNING, "Reply code 301, but no redirection url set\n");
>> +                break;
>> +            }
>> +            location = av_strdup(s->location);
>> +        }
>> +
>> +        break;
>>      case AVERROR_HTTP_SERVER_ERROR:
>>      case 500:
>>          reply_code = 500;
>>          reply_text = "Internal server error";
>>          break;
>> +    case 503:
>> +        reply_code = 503;
>> +        reply_text = "Service Unavailable";
>> +        break;
>>      default:
>>          return AVERROR(EINVAL);
>>      }
>> +    if (s->reply_text)
>> +        reply_text = s->reply_text;
>> +    if (s->content_type)
>> +        content_type = s->content_type;
>> +    if (s->headers)
>> +        headers = s->headers;
>> +    if (s->body) {
>> +        av_strlcpy(body_content, s->body, BUFFER_SIZE);
>> +        body = 1;
>> +    } else {
>> +        snprintf(body_content, BUFFER_SIZE, "%03d %s\r\n", reply_code, reply_text);
>> +    }
>> +
>>      if (body) {
>>          s->chunked_post = 0;
>
>>          message_len = snprintf(message, sizeof(message),
>>                   "HTTP/1.1 %03d %s\r\n"
>>                   "Content-Type: %s\r\n"
>>                   "Content-Length: %zu\r\n"
>> +                 "%s%s"
>> +                 "%s"
>>                   "\r\n"
>> -                 "%03d %s\r\n",
>> +                 "%s",
>>                   reply_code,
>>                   reply_text,
>>                   content_type,
>> -                 strlen(reply_text) + 6, // 3 digit status code + space + \r\n
>> -                 reply_code,
>> -                 reply_text);
>> +                 strlen(body_content),
>> +                 location ? "Location: " : "",
>> +                 location ? location : "",
>> +                 headers,
>> +                 body_content);
>
> Since you are including data from the application in the message, you can no
> longer be certain that the "messsage" buffer is long enough in all cases.
> Actually, the size of the buffer is MAX_URL_SIZE, and location may be that
> to.
>
> At the very least you need to check message_len against sizeof(message), but
> using a dynamically allocated buffer is probably better. I would suggest to
> use AVBPrint, because it makes error checking lightweight (just test
> is_truncated at the end) and because it makes adding conditional parts (the
> location) easier.

AVBPrint is perfect for this, thanks! New patch uses AVBPrint.

>
>>      } else {
>>          s->chunked_post = 1;
>>          message_len = snprintf(message, sizeof(message),
>
> Regards,
>
> --
>   Nicolas George
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Regards,
Stephan