Byte range implementation for httpd(8)

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Byte range implementation for httpd(8)

Sunil Nimmagadda-3
Range requests as defined in RFC7233 is required for resuming
interrupted http(s) downloads for example:
ftp -C http://foo.bar/install57.iso

With this diff, httpd parses "Range" header in the requests and
provide either 206(Partial Content) or 416(Range not Satisfiable)
responses with "Content-Range" header set appropriately. Further,
it understands multi range request and generate satisfiable payloads
with "multipart/byteranges" media type.

Suggestions/comments to improve the diff are welcome.

Note, "If-Range" isn't implemented yet.

Index: server_file.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
retrieving revision 1.51
diff -u -p -r1.51 server_file.c
--- server_file.c 12 Feb 2015 10:05:29 -0000 1.51
+++ server_file.c 17 Apr 2015 02:22:12 -0000
@@ -36,12 +36,23 @@
 
 #define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
 #define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b))
+#define MAX_RANGES 4
+
+struct range {
+ off_t start;
+ off_t end;
+};
 
 int server_file_access(struct httpd *, struct client *, char *, size_t);
 int server_file_request(struct httpd *, struct client *, char *,
     struct stat *);
+int server_partial_file_request(struct httpd *, struct client *, char *,
+    struct stat *, char *);
 int server_file_index(struct httpd *, struct client *, struct stat *);
 int server_file_method(struct client *);
+int parse_range_spec(char *, size_t, struct range *);
+struct range *parse_range(char *, size_t, int *);
+int buffer_add_range(int, struct evbuffer *, struct range *);
 
 int
 server_file_access(struct httpd *env, struct client *clt,
@@ -50,6 +61,7 @@ server_file_access(struct httpd *env, st
  struct http_descriptor *desc = clt->clt_descreq;
  struct server_config *srv_conf = clt->clt_srv_conf;
  struct stat st;
+ struct kv *r, key;
  char *newpath;
  int ret;
 
@@ -123,7 +135,13 @@ server_file_access(struct httpd *env, st
  goto fail;
  }
 
- return (server_file_request(env, clt, path, &st));
+ key.kv_key = "Range";
+ r = kv_find(&desc->http_headers, &key);
+ if (r != NULL)
+ return (server_partial_file_request(env, clt, path, &st,
+    r->kv_value));
+ else
+ return (server_file_request(env, clt, path, &st));
 
  fail:
  switch (errno) {
@@ -262,6 +280,138 @@ server_file_request(struct httpd *env, s
 }
 
 int
+server_partial_file_request(struct httpd *env, struct client *clt, char *path,
+    struct stat *st, char *range_str)
+{
+ struct http_descriptor *resp = clt->clt_descresp;
+ struct media_type *media, multipart_media;
+ struct range *range;
+ struct evbuffer *evb = NULL;
+ size_t content_length;
+ int code = 500, fd = -1, i, nranges, ret;
+ char content_range[64];
+ const char *errstr = NULL;
+ uint32_t boundary;
+
+ if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
+ code = 416;
+ (void)snprintf(content_range, sizeof(content_range),
+    "bytes */%lld", st->st_size);
+ errstr = content_range;
+ goto abort;
+ }
+
+ /* Now open the file, should be readable or we have another problem */
+ if ((fd = open(path, O_RDONLY)) == -1)
+ goto abort;
+
+ media = media_find(env->sc_mediatypes, path);
+ if ((evb = evbuffer_new()) == NULL) {
+ errstr = "failed to allocate file buffer";
+ goto abort;
+ }
+
+ if (nranges == 1) {
+ (void)snprintf(content_range, sizeof(content_range),
+        "bytes %lld-%lld/%lld", range->start, range->end,
+    st->st_size);
+ if (kv_add(&resp->http_headers, "Content-Range",
+    content_range) == NULL)
+ goto abort;
+
+ content_length = range->end - range->start + 1;
+ if (buffer_add_range(fd, evb, range) == 0)
+ goto abort;
+
+ } else {
+ content_length = 0;
+ boundary = arc4random();
+ /* Generate a multipart payload of byteranges */
+ while (nranges--) {
+ if ((i = evbuffer_add_printf(evb, "\r\n--%ud\r\n",
+    boundary)) == -1)
+ goto abort;
+
+ content_length += i;
+ if ((i = evbuffer_add_printf(evb,
+    "Content-Type: %s/%s\r\n",
+        media == NULL ? "application" : media->media_type,
+        media == NULL ?
+    "octet-stream" : media->media_subtype)) == -1)
+ goto abort;
+
+ content_length += i;
+ if ((i = evbuffer_add_printf(evb,
+    "Content-Range: bytes %lld-%lld/%lld\r\n\r\n",
+    range->start, range->end, st->st_size)) == -1)
+ goto abort;
+
+ content_length += i;
+ if (buffer_add_range(fd, evb, range) == 0)
+ goto abort;
+
+ content_length += range->end - range->start + 1;
+ range++;
+ }
+
+ if ((i = evbuffer_add_printf(evb, "\r\n--%ud--\r\n",
+    boundary)) == -1)
+ goto abort;
+
+ content_length += i;
+
+ /* prepare multipart/byteranges media type */
+ (void)strlcpy(multipart_media.media_type, "multipart",
+        sizeof(multipart_media.media_type));
+ (void)snprintf(multipart_media.media_subtype,
+        sizeof(multipart_media.media_subtype),
+        "byteranges; boundary=%ud", boundary);
+ media = &multipart_media;
+ }
+
+ ret = server_response_http(clt, 206, media, content_length,
+    MINIMUM(time(NULL), st->st_mtim.tv_sec));
+ switch (ret) {
+ case -1:
+ goto fail;
+ case 0:
+ /* Connection is already finished */
+ close(fd);
+ evbuffer_free(evb);
+ evb = NULL;
+ goto done;
+ default:
+ break;
+ }
+
+ if (server_bufferevent_write_buffer(clt, evb) == -1)
+ goto fail;
+
+ evbuffer_free(evb);
+ evb = NULL;
+
+ bufferevent_enable(clt->clt_bev, EV_READ|EV_WRITE);
+ if (clt->clt_persist)
+ clt->clt_toread = TOREAD_HTTP_HEADER;
+ else
+ clt->clt_toread = TOREAD_HTTP_NONE;
+ clt->clt_done = 0;
+
+ done:
+ server_reset_http(clt);
+ return (0);
+ fail:
+ bufferevent_disable(clt->clt_bev, EV_READ|EV_WRITE);
+ bufferevent_free(clt->clt_bev);
+ clt->clt_bev = NULL;
+ abort:
+ if (errstr == NULL)
+ errstr = strerror(errno);
+ server_abort_http(clt, code, errstr);
+ return (-1);
+}
+
+int
 server_file_index(struct httpd *env, struct client *clt, struct stat *st)
 {
  char  path[PATH_MAX];
@@ -467,3 +617,112 @@ server_file_error(struct bufferevent *be
  server_close(clt, "unknown event error");
  return;
 }
+
+struct range *
+parse_range(char *str, size_t file_sz, int *nranges)
+{
+ static struct range ranges[MAX_RANGES];
+ char *p, *q;
+ int i = 0;
+
+ /* Extract range unit */
+ if ((p = strchr(str, '=')) == NULL)
+ return (NULL);
+
+ *p++ = '\0';
+ /* Check if it's a bytes range spec */
+ if (strcmp(str, "bytes") != 0)
+ return (NULL);
+
+ while ((q = strchr(p, ',')) != NULL) {
+ *q++ = '\0';
+
+ /* Extract start and end positions */
+ if (parse_range_spec(p, file_sz, &ranges[i]) == 0)
+ continue;
+
+ i += 1;
+ if (i == MAX_RANGES)
+ return (NULL);
+
+ p = q;
+ }
+
+ if (p != NULL && parse_range_spec(p, file_sz, &ranges[i]) != 0)
+ i += 1;
+
+ *nranges = i;
+ return (i ? ranges : NULL);
+}
+
+int
+parse_range_spec(char *str, size_t size, struct range *r)
+{
+ size_t start_str_len, end_str_len;
+ char *p, *start_str, *end_str;
+ const char *errstr;
+
+ if ((p = strchr(str, '-')) == NULL)
+ return (0);
+
+ *p++ = '\0';
+ start_str = str;
+ end_str = p;
+ start_str_len = strlen(start_str);
+ end_str_len = strlen(end_str);
+
+ /* Either 'start' or 'end' is optional but not both */
+ if ((start_str_len == 0) && (end_str_len == 0))
+ return (0);
+
+ if (end_str_len) {
+ r->end = strtonum(end_str, 0, LLONG_MAX, &errstr);
+ if (errstr)
+ return (0);
+
+ if ((size_t)r->end >= size)
+ r->end = size - 1;
+ } else
+ r->end = size - 1;
+
+ if (start_str_len) {
+ r->start = strtonum(start_str, 0, LLONG_MAX, &errstr);
+ if (errstr)
+ return (0);
+
+ if ((size_t)r->start >= size)
+ return (0);
+ } else {
+ r->start = size - r->end;
+ r->end = size - 1;
+ }
+
+ if (r->end < r->start)
+ return (0);
+
+ return (1);
+}
+
+int
+buffer_add_range(int fd, struct evbuffer *evb, struct range *range)
+{
+ char buf[BUFSIZ];
+ size_t n, range_sz;
+ ssize_t nread;
+
+ if (lseek(fd, range->start, SEEK_SET) == -1)
+ return (0);
+
+ range_sz = range->end - range->start + 1;
+ while (range_sz) {
+ n = MINIMUM(range_sz, sizeof(buf));
+ if ((nread = read(fd, buf, n)) == -1)
+ return (0);
+
+ evbuffer_add(evb, buf, nread);
+ range_sz -= nread;
+ }
+
+ return (1);
+}
+
Index: server_http.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.77
diff -u -p -r1.77 server_http.c
--- server_http.c 9 Apr 2015 16:48:29 -0000 1.77
+++ server_http.c 17 Apr 2015 02:22:12 -0000
@@ -787,6 +787,13 @@ server_abort_http(struct client *clt, u_
  extraheader = NULL;
  }
  break;
+ case 416:
+ if (asprintf(&extraheader,
+    "Content-Range: %s\r\n", msg) == -1) {
+ code = 500;
+ extraheader = NULL;
+ }
+ break;
  default:
  /*
  * Do not send details of the error.  Traditionally,
@@ -1177,6 +1184,11 @@ server_response_http(struct client *clt,
  /* Date header is mandatory and should be added as late as possible */
  if (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
     kv_add(&resp->http_headers, "Date", tmbuf) == NULL)
+ return (-1);
+
+ /* Accept byte ranges */
+ if (code == 200 &&
+    kv_add(&resp->http_headers, "Accept-Ranges", "bytes") == NULL)
  return (-1);
 
  /* Write completed header */

Reply | Threaded
Open this post in threaded view
|

Re: Byte range implementation for httpd(8)

Sunil Nimmagadda-3
Any interest/comments/suggestions for this diff...

On Fri, Apr 17, 2015 at 05:04:01AM +0200, Sunil Nimmagadda wrote:

> Range requests as defined in RFC7233 is required for resuming
> interrupted http(s) downloads for example:
> ftp -C http://foo.bar/install57.iso
>
> With this diff, httpd parses "Range" header in the requests and
> provide either 206(Partial Content) or 416(Range not Satisfiable)
> responses with "Content-Range" header set appropriately. Further,
> it understands multi range request and generate satisfiable payloads
> with "multipart/byteranges" media type.
>
> Suggestions/comments to improve the diff are welcome.
>
> Note, "If-Range" isn't implemented yet.
>
> Index: server_file.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 server_file.c
> --- server_file.c 12 Feb 2015 10:05:29 -0000 1.51
> +++ server_file.c 17 Apr 2015 02:22:12 -0000
> @@ -36,12 +36,23 @@
>  
>  #define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
>  #define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b))
> +#define MAX_RANGES 4
> +
> +struct range {
> + off_t start;
> + off_t end;
> +};
>  
>  int server_file_access(struct httpd *, struct client *, char *, size_t);
>  int server_file_request(struct httpd *, struct client *, char *,
>      struct stat *);
> +int server_partial_file_request(struct httpd *, struct client *, char *,
> +    struct stat *, char *);
>  int server_file_index(struct httpd *, struct client *, struct stat *);
>  int server_file_method(struct client *);
> +int parse_range_spec(char *, size_t, struct range *);
> +struct range *parse_range(char *, size_t, int *);
> +int buffer_add_range(int, struct evbuffer *, struct range *);
>  
>  int
>  server_file_access(struct httpd *env, struct client *clt,
> @@ -50,6 +61,7 @@ server_file_access(struct httpd *env, st
>   struct http_descriptor *desc = clt->clt_descreq;
>   struct server_config *srv_conf = clt->clt_srv_conf;
>   struct stat st;
> + struct kv *r, key;
>   char *newpath;
>   int ret;
>  
> @@ -123,7 +135,13 @@ server_file_access(struct httpd *env, st
>   goto fail;
>   }
>  
> - return (server_file_request(env, clt, path, &st));
> + key.kv_key = "Range";
> + r = kv_find(&desc->http_headers, &key);
> + if (r != NULL)
> + return (server_partial_file_request(env, clt, path, &st,
> +    r->kv_value));
> + else
> + return (server_file_request(env, clt, path, &st));
>  
>   fail:
>   switch (errno) {
> @@ -262,6 +280,138 @@ server_file_request(struct httpd *env, s
>  }
>  
>  int
> +server_partial_file_request(struct httpd *env, struct client *clt, char *path,
> +    struct stat *st, char *range_str)
> +{
> + struct http_descriptor *resp = clt->clt_descresp;
> + struct media_type *media, multipart_media;
> + struct range *range;
> + struct evbuffer *evb = NULL;
> + size_t content_length;
> + int code = 500, fd = -1, i, nranges, ret;
> + char content_range[64];
> + const char *errstr = NULL;
> + uint32_t boundary;
> +
> + if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
> + code = 416;
> + (void)snprintf(content_range, sizeof(content_range),
> +    "bytes */%lld", st->st_size);
> + errstr = content_range;
> + goto abort;
> + }
> +
> + /* Now open the file, should be readable or we have another problem */
> + if ((fd = open(path, O_RDONLY)) == -1)
> + goto abort;
> +
> + media = media_find(env->sc_mediatypes, path);
> + if ((evb = evbuffer_new()) == NULL) {
> + errstr = "failed to allocate file buffer";
> + goto abort;
> + }
> +
> + if (nranges == 1) {
> + (void)snprintf(content_range, sizeof(content_range),
> +        "bytes %lld-%lld/%lld", range->start, range->end,
> +    st->st_size);
> + if (kv_add(&resp->http_headers, "Content-Range",
> +    content_range) == NULL)
> + goto abort;
> +
> + content_length = range->end - range->start + 1;
> + if (buffer_add_range(fd, evb, range) == 0)
> + goto abort;
> +
> + } else {
> + content_length = 0;
> + boundary = arc4random();
> + /* Generate a multipart payload of byteranges */
> + while (nranges--) {
> + if ((i = evbuffer_add_printf(evb, "\r\n--%ud\r\n",
> +    boundary)) == -1)
> + goto abort;
> +
> + content_length += i;
> + if ((i = evbuffer_add_printf(evb,
> +    "Content-Type: %s/%s\r\n",
> +        media == NULL ? "application" : media->media_type,
> +        media == NULL ?
> +    "octet-stream" : media->media_subtype)) == -1)
> + goto abort;
> +
> + content_length += i;
> + if ((i = evbuffer_add_printf(evb,
> +    "Content-Range: bytes %lld-%lld/%lld\r\n\r\n",
> +    range->start, range->end, st->st_size)) == -1)
> + goto abort;
> +
> + content_length += i;
> + if (buffer_add_range(fd, evb, range) == 0)
> + goto abort;
> +
> + content_length += range->end - range->start + 1;
> + range++;
> + }
> +
> + if ((i = evbuffer_add_printf(evb, "\r\n--%ud--\r\n",
> +    boundary)) == -1)
> + goto abort;
> +
> + content_length += i;
> +
> + /* prepare multipart/byteranges media type */
> + (void)strlcpy(multipart_media.media_type, "multipart",
> +        sizeof(multipart_media.media_type));
> + (void)snprintf(multipart_media.media_subtype,
> +        sizeof(multipart_media.media_subtype),
> +        "byteranges; boundary=%ud", boundary);
> + media = &multipart_media;
> + }
> +
> + ret = server_response_http(clt, 206, media, content_length,
> +    MINIMUM(time(NULL), st->st_mtim.tv_sec));
> + switch (ret) {
> + case -1:
> + goto fail;
> + case 0:
> + /* Connection is already finished */
> + close(fd);
> + evbuffer_free(evb);
> + evb = NULL;
> + goto done;
> + default:
> + break;
> + }
> +
> + if (server_bufferevent_write_buffer(clt, evb) == -1)
> + goto fail;
> +
> + evbuffer_free(evb);
> + evb = NULL;
> +
> + bufferevent_enable(clt->clt_bev, EV_READ|EV_WRITE);
> + if (clt->clt_persist)
> + clt->clt_toread = TOREAD_HTTP_HEADER;
> + else
> + clt->clt_toread = TOREAD_HTTP_NONE;
> + clt->clt_done = 0;
> +
> + done:
> + server_reset_http(clt);
> + return (0);
> + fail:
> + bufferevent_disable(clt->clt_bev, EV_READ|EV_WRITE);
> + bufferevent_free(clt->clt_bev);
> + clt->clt_bev = NULL;
> + abort:
> + if (errstr == NULL)
> + errstr = strerror(errno);
> + server_abort_http(clt, code, errstr);
> + return (-1);
> +}
> +
> +int
>  server_file_index(struct httpd *env, struct client *clt, struct stat *st)
>  {
>   char  path[PATH_MAX];
> @@ -467,3 +617,112 @@ server_file_error(struct bufferevent *be
>   server_close(clt, "unknown event error");
>   return;
>  }
> +
> +struct range *
> +parse_range(char *str, size_t file_sz, int *nranges)
> +{
> + static struct range ranges[MAX_RANGES];
> + char *p, *q;
> + int i = 0;
> +
> + /* Extract range unit */
> + if ((p = strchr(str, '=')) == NULL)
> + return (NULL);
> +
> + *p++ = '\0';
> + /* Check if it's a bytes range spec */
> + if (strcmp(str, "bytes") != 0)
> + return (NULL);
> +
> + while ((q = strchr(p, ',')) != NULL) {
> + *q++ = '\0';
> +
> + /* Extract start and end positions */
> + if (parse_range_spec(p, file_sz, &ranges[i]) == 0)
> + continue;
> +
> + i += 1;
> + if (i == MAX_RANGES)
> + return (NULL);
> +
> + p = q;
> + }
> +
> + if (p != NULL && parse_range_spec(p, file_sz, &ranges[i]) != 0)
> + i += 1;
> +
> + *nranges = i;
> + return (i ? ranges : NULL);
> +}
> +
> +int
> +parse_range_spec(char *str, size_t size, struct range *r)
> +{
> + size_t start_str_len, end_str_len;
> + char *p, *start_str, *end_str;
> + const char *errstr;
> +
> + if ((p = strchr(str, '-')) == NULL)
> + return (0);
> +
> + *p++ = '\0';
> + start_str = str;
> + end_str = p;
> + start_str_len = strlen(start_str);
> + end_str_len = strlen(end_str);
> +
> + /* Either 'start' or 'end' is optional but not both */
> + if ((start_str_len == 0) && (end_str_len == 0))
> + return (0);
> +
> + if (end_str_len) {
> + r->end = strtonum(end_str, 0, LLONG_MAX, &errstr);
> + if (errstr)
> + return (0);
> +
> + if ((size_t)r->end >= size)
> + r->end = size - 1;
> + } else
> + r->end = size - 1;
> +
> + if (start_str_len) {
> + r->start = strtonum(start_str, 0, LLONG_MAX, &errstr);
> + if (errstr)
> + return (0);
> +
> + if ((size_t)r->start >= size)
> + return (0);
> + } else {
> + r->start = size - r->end;
> + r->end = size - 1;
> + }
> +
> + if (r->end < r->start)
> + return (0);
> +
> + return (1);
> +}
> +
> +int
> +buffer_add_range(int fd, struct evbuffer *evb, struct range *range)
> +{
> + char buf[BUFSIZ];
> + size_t n, range_sz;
> + ssize_t nread;
> +
> + if (lseek(fd, range->start, SEEK_SET) == -1)
> + return (0);
> +
> + range_sz = range->end - range->start + 1;
> + while (range_sz) {
> + n = MINIMUM(range_sz, sizeof(buf));
> + if ((nread = read(fd, buf, n)) == -1)
> + return (0);
> +
> + evbuffer_add(evb, buf, nread);
> + range_sz -= nread;
> + }
> +
> + return (1);
> +}
> +
> Index: server_http.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 server_http.c
> --- server_http.c 9 Apr 2015 16:48:29 -0000 1.77
> +++ server_http.c 17 Apr 2015 02:22:12 -0000
> @@ -787,6 +787,13 @@ server_abort_http(struct client *clt, u_
>   extraheader = NULL;
>   }
>   break;
> + case 416:
> + if (asprintf(&extraheader,
> +    "Content-Range: %s\r\n", msg) == -1) {
> + code = 500;
> + extraheader = NULL;
> + }
> + break;
>   default:
>   /*
>   * Do not send details of the error.  Traditionally,
> @@ -1177,6 +1184,11 @@ server_response_http(struct client *clt,
>   /* Date header is mandatory and should be added as late as possible */
>   if (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
>      kv_add(&resp->http_headers, "Date", tmbuf) == NULL)
> + return (-1);
> +
> + /* Accept byte ranges */
> + if (code == 200 &&
> +    kv_add(&resp->http_headers, "Accept-Ranges", "bytes") == NULL)
>   return (-1);
>  
>   /* Write completed header */

Reply | Threaded
Open this post in threaded view
|

Re: Byte range implementation for httpd(8)

Florian Obser-2
In reply to this post by Sunil Nimmagadda-3
Sorry for the very late reply, I'm currently very busy :/

On Fri, Apr 17, 2015 at 05:04:01AM +0200, Sunil Nimmagadda wrote:

> Range requests as defined in RFC7233 is required for resuming
> interrupted http(s) downloads for example:
> ftp -C http://foo.bar/install57.iso
>
> With this diff, httpd parses "Range" header in the requests and
> provide either 206(Partial Content) or 416(Range not Satisfiable)
> responses with "Content-Range" header set appropriately. Further,
> it understands multi range request and generate satisfiable payloads
> with "multipart/byteranges" media type.
>
> Suggestions/comments to improve the diff are welcome.
>
> Note, "If-Range" isn't implemented yet.
>
> Index: server_file.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 server_file.c
> --- server_file.c 12 Feb 2015 10:05:29 -0000 1.51
> +++ server_file.c 17 Apr 2015 02:22:12 -0000
> @@ -36,12 +36,23 @@
>  
>  #define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
>  #define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b))
> +#define MAX_RANGES 4
> +
> +struct range {
> + off_t start;
> + off_t end;
> +};
>  
>  int server_file_access(struct httpd *, struct client *, char *, size_t);
>  int server_file_request(struct httpd *, struct client *, char *,
>      struct stat *);
> +int server_partial_file_request(struct httpd *, struct client *, char *,
> +    struct stat *, char *);
>  int server_file_index(struct httpd *, struct client *, struct stat *);
>  int server_file_method(struct client *);
> +int parse_range_spec(char *, size_t, struct range *);
> +struct range *parse_range(char *, size_t, int *);
> +int buffer_add_range(int, struct evbuffer *, struct range *);

This whole block now needs another tab indentation after the type
(except for parse_range of course)

>  
>  int
>  server_file_access(struct httpd *env, struct client *clt,
> @@ -50,6 +61,7 @@ server_file_access(struct httpd *env, st
>   struct http_descriptor *desc = clt->clt_descreq;
>   struct server_config *srv_conf = clt->clt_srv_conf;
>   struct stat st;
> + struct kv *r, key;
>   char *newpath;
>   int ret;
>  
> @@ -123,7 +135,13 @@ server_file_access(struct httpd *env, st
>   goto fail;
>   }
>  
> - return (server_file_request(env, clt, path, &st));
> + key.kv_key = "Range";
> + r = kv_find(&desc->http_headers, &key);
> + if (r != NULL)
> + return (server_partial_file_request(env, clt, path, &st,
> +    r->kv_value));
> + else
> + return (server_file_request(env, clt, path, &st));
>  
>   fail:
>   switch (errno) {
> @@ -262,6 +280,138 @@ server_file_request(struct httpd *env, s
>  }
>  
>  int
> +server_partial_file_request(struct httpd *env, struct client *clt, char *path,
> +    struct stat *st, char *range_str)
> +{
> + struct http_descriptor *resp = clt->clt_descresp;
> + struct media_type *media, multipart_media;
> + struct range *range;
> + struct evbuffer *evb = NULL;
> + size_t content_length;
> + int code = 500, fd = -1, i, nranges, ret;
> + char content_range[64];
> + const char *errstr = NULL;
> + uint32_t boundary;

Nit: uint32_t should be below int

> +

this is missing the server_file_method() song and dance from
server_file_request()

> + if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
> + code = 416;
> + (void)snprintf(content_range, sizeof(content_range),
> +    "bytes */%lld", st->st_size);
> + errstr = content_range;
> + goto abort;
> + }

hm, apache answers with the full file and 200 if the range header is
syntactically incorrect or if end < start. As far as I can tell it
only answers 416 if the range actually lies outside of the file.

> +
> + /* Now open the file, should be readable or we have another problem */
> + if ((fd = open(path, O_RDONLY)) == -1)
> + goto abort;
> +
> + media = media_find(env->sc_mediatypes, path);
> + if ((evb = evbuffer_new()) == NULL) {
> + errstr = "failed to allocate file buffer";
> + goto abort;
> + }
> +
> + if (nranges == 1) {
> + (void)snprintf(content_range, sizeof(content_range),
> +        "bytes %lld-%lld/%lld", range->start, range->end,
> +    st->st_size);
> + if (kv_add(&resp->http_headers, "Content-Range",
> +    content_range) == NULL)
> + goto abort;
> +
> + content_length = range->end - range->start + 1;
> + if (buffer_add_range(fd, evb, range) == 0)
> + goto abort;
> +
> + } else {
> + content_length = 0;
> + boundary = arc4random();
> + /* Generate a multipart payload of byteranges */
> + while (nranges--) {
> + if ((i = evbuffer_add_printf(evb, "\r\n--%ud\r\n",
> +    boundary)) == -1)
> + goto abort;
> +
> + content_length += i;
> + if ((i = evbuffer_add_printf(evb,
> +    "Content-Type: %s/%s\r\n",
> +        media == NULL ? "application" : media->media_type,
> +        media == NULL ?
> +    "octet-stream" : media->media_subtype)) == -1)
> + goto abort;
> +
> + content_length += i;
> + if ((i = evbuffer_add_printf(evb,
> +    "Content-Range: bytes %lld-%lld/%lld\r\n\r\n",
> +    range->start, range->end, st->st_size)) == -1)
> + goto abort;
> +
> + content_length += i;
> + if (buffer_add_range(fd, evb, range) == 0)
> + goto abort;
> +
> + content_length += range->end - range->start + 1;
> + range++;
> + }
> +
> + if ((i = evbuffer_add_printf(evb, "\r\n--%ud--\r\n",
> +    boundary)) == -1)
> + goto abort;
> +
> + content_length += i;
> +
> + /* prepare multipart/byteranges media type */
> + (void)strlcpy(multipart_media.media_type, "multipart",
> +        sizeof(multipart_media.media_type));
> + (void)snprintf(multipart_media.media_subtype,
> +        sizeof(multipart_media.media_subtype),
> +        "byteranges; boundary=%ud", boundary);
> + media = &multipart_media;
> + }
> +
> + ret = server_response_http(clt, 206, media, content_length,
> +    MINIMUM(time(NULL), st->st_mtim.tv_sec));
> + switch (ret) {
> + case -1:
> + goto fail;
> + case 0:
> + /* Connection is already finished */
> + close(fd);
> + evbuffer_free(evb);
> + evb = NULL;
> + goto done;
> + default:
> + break;
> + }
> +
> + if (server_bufferevent_write_buffer(clt, evb) == -1)
> + goto fail;
> +
> + evbuffer_free(evb);
> + evb = NULL;
> +
> + bufferevent_enable(clt->clt_bev, EV_READ|EV_WRITE);
> + if (clt->clt_persist)
> + clt->clt_toread = TOREAD_HTTP_HEADER;
> + else
> + clt->clt_toread = TOREAD_HTTP_NONE;
> + clt->clt_done = 0;
> +
> + done:
> + server_reset_http(clt);
> + return (0);
> + fail:
> + bufferevent_disable(clt->clt_bev, EV_READ|EV_WRITE);
> + bufferevent_free(clt->clt_bev);
> + clt->clt_bev = NULL;
> + abort:
> + if (errstr == NULL)
> + errstr = strerror(errno);
> + server_abort_http(clt, code, errstr);
> + return (-1);
> +}
> +
> +int
>  server_file_index(struct httpd *env, struct client *clt, struct stat *st)
>  {
>   char  path[PATH_MAX];
> @@ -467,3 +617,112 @@ server_file_error(struct bufferevent *be
>   server_close(clt, "unknown event error");
>   return;
>  }
> +
> +struct range *
> +parse_range(char *str, size_t file_sz, int *nranges)
> +{
> + static struct range ranges[MAX_RANGES];
> + char *p, *q;
> + int i = 0;

Nit: flip those two lines, int before char.

> +
> + /* Extract range unit */
> + if ((p = strchr(str, '=')) == NULL)
> + return (NULL);
> +
> + *p++ = '\0';
> + /* Check if it's a bytes range spec */
> + if (strcmp(str, "bytes") != 0)
> + return (NULL);
> +
> + while ((q = strchr(p, ',')) != NULL) {
> + *q++ = '\0';
> +
> + /* Extract start and end positions */
> + if (parse_range_spec(p, file_sz, &ranges[i]) == 0)
> + continue;
> +
> + i += 1;

i++;

> + if (i == MAX_RANGES)
> + return (NULL);
> +
> + p = q;
> + }
> +
> + if (p != NULL && parse_range_spec(p, file_sz, &ranges[i]) != 0)

why p != NULL? that can never be false here?

> + i += 1;

i++;

> +
> + *nranges = i;
> + return (i ? ranges : NULL);
> +}
> +
> +int
> +parse_range_spec(char *str, size_t size, struct range *r)
> +{
> + size_t start_str_len, end_str_len;
> + char *p, *start_str, *end_str;
> + const char *errstr;
> +
> + if ((p = strchr(str, '-')) == NULL)
> + return (0);
> +
> + *p++ = '\0';
> + start_str = str;
> + end_str = p;
> + start_str_len = strlen(start_str);
> + end_str_len = strlen(end_str);
> +
> + /* Either 'start' or 'end' is optional but not both */
> + if ((start_str_len == 0) && (end_str_len == 0))
> + return (0);
> +
> + if (end_str_len) {
> + r->end = strtonum(end_str, 0, LLONG_MAX, &errstr);
> + if (errstr)
> + return (0);
> +
> + if ((size_t)r->end >= size)
> + r->end = size - 1;
> + } else
> + r->end = size - 1;
> +
> + if (start_str_len) {
> + r->start = strtonum(start_str, 0, LLONG_MAX, &errstr);
> + if (errstr)
> + return (0);
> +
> + if ((size_t)r->start >= size)
> + return (0);
> + } else {
> + r->start = size - r->end;
> + r->end = size - 1;
> + }
> +
> + if (r->end < r->start)
> + return (0);
> +
> + return (1);
> +}
> +
> +int
> +buffer_add_range(int fd, struct evbuffer *evb, struct range *range)
> +{
> + char buf[BUFSIZ];
> + size_t n, range_sz;
> + ssize_t nread;
> +
> + if (lseek(fd, range->start, SEEK_SET) == -1)
> + return (0);
> +
> + range_sz = range->end - range->start + 1;
> + while (range_sz) {
> + n = MINIMUM(range_sz, sizeof(buf));
> + if ((nread = read(fd, buf, n)) == -1)
> + return (0);
> +
> + evbuffer_add(evb, buf, nread);
> + range_sz -= nread;
> + }
> +
> + return (1);
> +}
> +
> Index: server_http.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 server_http.c
> --- server_http.c 9 Apr 2015 16:48:29 -0000 1.77
> +++ server_http.c 17 Apr 2015 02:22:12 -0000
> @@ -787,6 +787,13 @@ server_abort_http(struct client *clt, u_
>   extraheader = NULL;
>   }
>   break;
> + case 416:
> + if (asprintf(&extraheader,
> +    "Content-Range: %s\r\n", msg) == -1) {
> + code = 500;
> + extraheader = NULL;
> + }
> + break;
>   default:
>   /*
>   * Do not send details of the error.  Traditionally,
> @@ -1177,6 +1184,11 @@ server_response_http(struct client *clt,
>   /* Date header is mandatory and should be added as late as possible */
>   if (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
>      kv_add(&resp->http_headers, "Date", tmbuf) == NULL)
> + return (-1);
> +
> + /* Accept byte ranges */
> + if (code == 200 &&
> +    kv_add(&resp->http_headers, "Accept-Ranges", "bytes") == NULL)
>   return (-1);

I don't think we should advertise ranges for all 200 results, for
example we don't support it for directory indexes.

>  
>   /* Write completed header */
>


--
I'm not entirely sure you are real.

Reply | Threaded
Open this post in threaded view
|

Re: Byte range implementation for httpd(8)

Sunil Nimmagadda-3
On Sat, May 02, 2015 at 02:49:30PM +0000, Florian Obser wrote:
> Sorry for the very late reply, I'm currently very busy :/

Thank you for taking time to review it. A new patch with style nits
fixed and a gratuitous NULL check removed.

[trimming some text]
 
> this is missing the server_file_method() song and dance from
> server_file_request()

Fixed in a slightly different way than using server_file_method().
Since range request should be ignored for methods other than GET,
fallback to server_file_request() for further processing/validation.

> > + if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
> > + code = 416;
> > + (void)snprintf(content_range, sizeof(content_range),
> > +    "bytes */%lld", st->st_size);
> > + errstr = content_range;
> > + goto abort;
> > + }
>
> hm, apache answers with the full file and 200 if the range header is
> syntactically incorrect or if end < start. As far as I can tell it
> only answers 416 if the range actually lies outside of the file.

I am confused about the RFC here, section 4.4 states that 416 is
returned if "...the set of ranges requested has been rejected due
to invalid ranges..." and a note follows immediately, "Because
servers are free to ignore Range, many implementations will simply
respond with the entire selected representation in a 200 response"

As you noted apache returns 200 while nginx returns 416 for an
incorrect range. What should httpd do ideally?

> > +
> > + /* Accept byte ranges */
> > + if (code == 200 &&
> > +    kv_add(&resp->http_headers, "Accept-Ranges", "bytes") == NULL)
> >   return (-1);
>
> I don't think we should advertise ranges for all 200 results, for
> example we don't support it for directory indexes.

Agree, since RFC says "server MAY send Accept-Range header" and
"clients MAY generate range requests without having received this
header field", dropping this header shouldn't make a difference.

Comments?

Index: server_file.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
retrieving revision 1.52
diff -u -p -r1.52 server_file.c
--- server_file.c 25 Apr 2015 14:40:35 -0000 1.52
+++ server_file.c 3 May 2015 11:18:07 -0000
@@ -36,12 +36,25 @@
 
 #define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
 #define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b))
+#define MAX_RANGES 4
 
-int server_file_access(struct httpd *, struct client *, char *, size_t);
-int server_file_request(struct httpd *, struct client *, char *,
-    struct stat *);
-int server_file_index(struct httpd *, struct client *, struct stat *);
-int server_file_method(struct client *);
+struct range {
+ off_t start;
+ off_t end;
+};
+
+int server_file_access(struct httpd *, struct client *,
+    char *, size_t);
+int server_file_request(struct httpd *, struct client *,
+    char *, struct stat *);
+int server_partial_file_request(struct httpd *, struct client *,
+    char *, struct stat *, char *);
+int server_file_index(struct httpd *, struct client *,
+    struct stat *);
+int server_file_method(struct client *);
+int parse_range_spec(char *, size_t, struct range *);
+struct range *parse_range(char *, size_t, int *);
+int buffer_add_range(int, struct evbuffer *, struct range *);
 
 int
 server_file_access(struct httpd *env, struct client *clt,
@@ -50,6 +63,7 @@ server_file_access(struct httpd *env, st
  struct http_descriptor *desc = clt->clt_descreq;
  struct server_config *srv_conf = clt->clt_srv_conf;
  struct stat st;
+ struct kv *r, key;
  char *newpath;
  int ret;
 
@@ -123,7 +137,13 @@ server_file_access(struct httpd *env, st
  goto fail;
  }
 
- return (server_file_request(env, clt, path, &st));
+ key.kv_key = "Range";
+ r = kv_find(&desc->http_headers, &key);
+ if (r != NULL)
+ return (server_partial_file_request(env, clt, path, &st,
+    r->kv_value));
+ else
+ return (server_file_request(env, clt, path, &st));
 
  fail:
  switch (errno) {
@@ -262,6 +282,143 @@ server_file_request(struct httpd *env, s
 }
 
 int
+server_partial_file_request(struct httpd *env, struct client *clt, char *path,
+    struct stat *st, char *range_str)
+{
+ struct http_descriptor *resp = clt->clt_descresp;
+ struct http_descriptor *desc = clt->clt_descreq;
+ struct media_type *media, multipart_media;
+ struct range *range;
+ struct evbuffer *evb = NULL;
+ size_t content_length;
+ int code = 500, fd = -1, i, nranges, ret;
+ uint32_t boundary;
+ char content_range[64];
+ const char *errstr = NULL;
+
+ /* Ignore range request for methods other than GET */
+ if (desc->http_method != HTTP_METHOD_GET)
+ return server_file_request(env, clt, path, st);
+
+ if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
+ code = 416;
+ (void)snprintf(content_range, sizeof(content_range),
+    "bytes */%lld", st->st_size);
+ errstr = content_range;
+ goto abort;
+ }
+
+ /* Now open the file, should be readable or we have another problem */
+ if ((fd = open(path, O_RDONLY)) == -1)
+ goto abort;
+
+ media = media_find(env->sc_mediatypes, path);
+ if ((evb = evbuffer_new()) == NULL) {
+ errstr = "failed to allocate file buffer";
+ goto abort;
+ }
+
+ if (nranges == 1) {
+ (void)snprintf(content_range, sizeof(content_range),
+        "bytes %lld-%lld/%lld", range->start, range->end,
+    st->st_size);
+ if (kv_add(&resp->http_headers, "Content-Range",
+    content_range) == NULL)
+ goto abort;
+
+ content_length = range->end - range->start + 1;
+ if (buffer_add_range(fd, evb, range) == 0)
+ goto abort;
+
+ } else {
+ content_length = 0;
+ boundary = arc4random();
+ /* Generate a multipart payload of byteranges */
+ while (nranges--) {
+ if ((i = evbuffer_add_printf(evb, "\r\n--%ud\r\n",
+    boundary)) == -1)
+ goto abort;
+
+ content_length += i;
+ if ((i = evbuffer_add_printf(evb,
+    "Content-Type: %s/%s\r\n",
+        media == NULL ? "application" : media->media_type,
+        media == NULL ?
+    "octet-stream" : media->media_subtype)) == -1)
+ goto abort;
+
+ content_length += i;
+ if ((i = evbuffer_add_printf(evb,
+    "Content-Range: bytes %lld-%lld/%lld\r\n\r\n",
+    range->start, range->end, st->st_size)) == -1)
+ goto abort;
+
+ content_length += i;
+ if (buffer_add_range(fd, evb, range) == 0)
+ goto abort;
+
+ content_length += range->end - range->start + 1;
+ range++;
+ }
+
+ if ((i = evbuffer_add_printf(evb, "\r\n--%ud--\r\n",
+    boundary)) == -1)
+ goto abort;
+
+ content_length += i;
+
+ /* prepare multipart/byteranges media type */
+ (void)strlcpy(multipart_media.media_type, "multipart",
+        sizeof(multipart_media.media_type));
+ (void)snprintf(multipart_media.media_subtype,
+        sizeof(multipart_media.media_subtype),
+        "byteranges; boundary=%ud", boundary);
+ media = &multipart_media;
+ }
+
+ ret = server_response_http(clt, 206, media, content_length,
+    MINIMUM(time(NULL), st->st_mtim.tv_sec));
+ switch (ret) {
+ case -1:
+ goto fail;
+ case 0:
+ /* Connection is already finished */
+ close(fd);
+ evbuffer_free(evb);
+ evb = NULL;
+ goto done;
+ default:
+ break;
+ }
+
+ if (server_bufferevent_write_buffer(clt, evb) == -1)
+ goto fail;
+
+ evbuffer_free(evb);
+ evb = NULL;
+
+ bufferevent_enable(clt->clt_bev, EV_READ|EV_WRITE);
+ if (clt->clt_persist)
+ clt->clt_toread = TOREAD_HTTP_HEADER;
+ else
+ clt->clt_toread = TOREAD_HTTP_NONE;
+ clt->clt_done = 0;
+
+ done:
+ server_reset_http(clt);
+ return (0);
+ fail:
+ bufferevent_disable(clt->clt_bev, EV_READ|EV_WRITE);
+ bufferevent_free(clt->clt_bev);
+ clt->clt_bev = NULL;
+ abort:
+ if (errstr == NULL)
+ errstr = strerror(errno);
+ server_abort_http(clt, code, errstr);
+ return (-1);
+}
+
+int
 server_file_index(struct httpd *env, struct client *clt, struct stat *st)
 {
  char  path[PATH_MAX];
@@ -469,3 +626,112 @@ server_file_error(struct bufferevent *be
  server_close(clt, "unknown event error");
  return;
 }
+
+struct range *
+parse_range(char *str, size_t file_sz, int *nranges)
+{
+ static struct range ranges[MAX_RANGES];
+ int i = 0;
+ char *p, *q;
+
+ /* Extract range unit */
+ if ((p = strchr(str, '=')) == NULL)
+ return (NULL);
+
+ *p++ = '\0';
+ /* Check if it's a bytes range spec */
+ if (strcmp(str, "bytes") != 0)
+ return (NULL);
+
+ while ((q = strchr(p, ',')) != NULL) {
+ *q++ = '\0';
+
+ /* Extract start and end positions */
+ if (parse_range_spec(p, file_sz, &ranges[i]) == 0)
+ continue;
+
+ i++;
+ if (i == MAX_RANGES)
+ return (NULL);
+
+ p = q;
+ }
+
+ if (parse_range_spec(p, file_sz, &ranges[i]) != 0)
+ i++;
+
+ *nranges = i;
+ return (i ? ranges : NULL);
+}
+
+int
+parse_range_spec(char *str, size_t size, struct range *r)
+{
+ size_t start_str_len, end_str_len;
+ char *p, *start_str, *end_str;
+ const char *errstr;
+
+ if ((p = strchr(str, '-')) == NULL)
+ return (0);
+
+ *p++ = '\0';
+ start_str = str;
+ end_str = p;
+ start_str_len = strlen(start_str);
+ end_str_len = strlen(end_str);
+
+ /* Either 'start' or 'end' is optional but not both */
+ if ((start_str_len == 0) && (end_str_len == 0))
+ return (0);
+
+ if (end_str_len) {
+ r->end = strtonum(end_str, 0, LLONG_MAX, &errstr);
+ if (errstr)
+ return (0);
+
+ if ((size_t)r->end >= size)
+ r->end = size - 1;
+ } else
+ r->end = size - 1;
+
+ if (start_str_len) {
+ r->start = strtonum(start_str, 0, LLONG_MAX, &errstr);
+ if (errstr)
+ return (0);
+
+ if ((size_t)r->start >= size)
+ return (0);
+ } else {
+ r->start = size - r->end;
+ r->end = size - 1;
+ }
+
+ if (r->end < r->start)
+ return (0);
+
+ return (1);
+}
+
+int
+buffer_add_range(int fd, struct evbuffer *evb, struct range *range)
+{
+ char buf[BUFSIZ];
+ size_t n, range_sz;
+ ssize_t nread;
+
+ if (lseek(fd, range->start, SEEK_SET) == -1)
+ return (0);
+
+ range_sz = range->end - range->start + 1;
+ while (range_sz) {
+ n = MINIMUM(range_sz, sizeof(buf));
+ if ((nread = read(fd, buf, n)) == -1)
+ return (0);
+
+ evbuffer_add(evb, buf, nread);
+ range_sz -= nread;
+ }
+
+ return (1);
+}
+
Index: server_http.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.78
diff -u -p -r1.78 server_http.c
--- server_http.c 18 Apr 2015 09:27:54 -0000 1.78
+++ server_http.c 3 May 2015 11:18:07 -0000
@@ -788,6 +788,13 @@ server_abort_http(struct client *clt, u_
  extraheader = NULL;
  }
  break;
+ case 416:
+ if (asprintf(&extraheader,
+    "Content-Range: %s\r\n", msg) == -1) {
+ code = 500;
+ extraheader = NULL;
+ }
+ break;
  default:
  /*
  * Do not send details of the error.  Traditionally,

Reply | Threaded
Open this post in threaded view
|

Re: Byte range implementation for httpd(8)

Florian Obser-2
On Sun, May 03, 2015 at 01:46:56PM +0200, Sunil Nimmagadda wrote:

> On Sat, May 02, 2015 at 02:49:30PM +0000, Florian Obser wrote:
> > Sorry for the very late reply, I'm currently very busy :/
>
> Thank you for taking time to review it. A new patch with style nits
> fixed and a gratuitous NULL check removed.
>
> [trimming some text]
>  
> > this is missing the server_file_method() song and dance from
> > server_file_request()
>
> Fixed in a slightly different way than using server_file_method().
> Since range request should be ignored for methods other than GET,
> fallback to server_file_request() for further processing/validation.

yep, this is good.

>
> > > + if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
> > > + code = 416;
> > > + (void)snprintf(content_range, sizeof(content_range),
> > > +    "bytes */%lld", st->st_size);
> > > + errstr = content_range;
> > > + goto abort;
> > > + }
> >
> > hm, apache answers with the full file and 200 if the range header is
> > syntactically incorrect or if end < start. As far as I can tell it
> > only answers 416 if the range actually lies outside of the file.
>
> I am confused about the RFC here, section 4.4 states that 416 is
> returned if "...the set of ranges requested has been rejected due
> to invalid ranges..." and a note follows immediately, "Because
> servers are free to ignore Range, many implementations will simply
> respond with the entire selected representation in a 200 response"
>
> As you noted apache returns 200 while nginx returns 416 for an
> incorrect range. What should httpd do ideally?

Thanks for checking nginx, I don't have one around.
I noted this for 2 reasons:
1) I guess apache is the defacto standard, so if it behaves
differently there might be a reason, maybe broken clients in the wild.
2) The way the code is currently structured it will require a bit of
shuffling around if we ever want apache's behaviour.

That being said, my understanding of the RFC is that your
implementation is correct. And with nginx behaving this way, too, it
shouldn't bite us in the future. :)

>
> > > +
> > > + /* Accept byte ranges */
> > > + if (code == 200 &&
> > > +    kv_add(&resp->http_headers, "Accept-Ranges", "bytes") == NULL)
> > >   return (-1);
> >
> > I don't think we should advertise ranges for all 200 results, for
> > example we don't support it for directory indexes.
>
> Agree, since RFC says "server MAY send Accept-Range header" and
> "clients MAY generate range requests without having received this
> header field", dropping this header shouldn't make a difference.
>
> Comments?

This looks good.

OK florian@ if someone wants to commit it. Or give me an OK and I'll
commit.

>
> Index: server_file.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 server_file.c
> --- server_file.c 25 Apr 2015 14:40:35 -0000 1.52
> +++ server_file.c 3 May 2015 11:18:07 -0000
> @@ -36,12 +36,25 @@
>  
>  #define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
>  #define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b))
> +#define MAX_RANGES 4
>  
> -int server_file_access(struct httpd *, struct client *, char *, size_t);
> -int server_file_request(struct httpd *, struct client *, char *,
> -    struct stat *);
> -int server_file_index(struct httpd *, struct client *, struct stat *);
> -int server_file_method(struct client *);
> +struct range {
> + off_t start;
> + off_t end;
> +};
> +
> +int server_file_access(struct httpd *, struct client *,
> +    char *, size_t);
> +int server_file_request(struct httpd *, struct client *,
> +    char *, struct stat *);
> +int server_partial_file_request(struct httpd *, struct client *,
> +    char *, struct stat *, char *);
> +int server_file_index(struct httpd *, struct client *,
> +    struct stat *);
> +int server_file_method(struct client *);
> +int parse_range_spec(char *, size_t, struct range *);
> +struct range *parse_range(char *, size_t, int *);
> +int buffer_add_range(int, struct evbuffer *, struct range *);
>  
>  int
>  server_file_access(struct httpd *env, struct client *clt,
> @@ -50,6 +63,7 @@ server_file_access(struct httpd *env, st
>   struct http_descriptor *desc = clt->clt_descreq;
>   struct server_config *srv_conf = clt->clt_srv_conf;
>   struct stat st;
> + struct kv *r, key;
>   char *newpath;
>   int ret;
>  
> @@ -123,7 +137,13 @@ server_file_access(struct httpd *env, st
>   goto fail;
>   }
>  
> - return (server_file_request(env, clt, path, &st));
> + key.kv_key = "Range";
> + r = kv_find(&desc->http_headers, &key);
> + if (r != NULL)
> + return (server_partial_file_request(env, clt, path, &st,
> +    r->kv_value));
> + else
> + return (server_file_request(env, clt, path, &st));
>  
>   fail:
>   switch (errno) {
> @@ -262,6 +282,143 @@ server_file_request(struct httpd *env, s
>  }
>  
>  int
> +server_partial_file_request(struct httpd *env, struct client *clt, char *path,
> +    struct stat *st, char *range_str)
> +{
> + struct http_descriptor *resp = clt->clt_descresp;
> + struct http_descriptor *desc = clt->clt_descreq;
> + struct media_type *media, multipart_media;
> + struct range *range;
> + struct evbuffer *evb = NULL;
> + size_t content_length;
> + int code = 500, fd = -1, i, nranges, ret;
> + uint32_t boundary;
> + char content_range[64];
> + const char *errstr = NULL;
> +
> + /* Ignore range request for methods other than GET */
> + if (desc->http_method != HTTP_METHOD_GET)
> + return server_file_request(env, clt, path, st);
> +
> + if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
> + code = 416;
> + (void)snprintf(content_range, sizeof(content_range),
> +    "bytes */%lld", st->st_size);
> + errstr = content_range;
> + goto abort;
> + }
> +
> + /* Now open the file, should be readable or we have another problem */
> + if ((fd = open(path, O_RDONLY)) == -1)
> + goto abort;
> +
> + media = media_find(env->sc_mediatypes, path);
> + if ((evb = evbuffer_new()) == NULL) {
> + errstr = "failed to allocate file buffer";
> + goto abort;
> + }
> +
> + if (nranges == 1) {
> + (void)snprintf(content_range, sizeof(content_range),
> +        "bytes %lld-%lld/%lld", range->start, range->end,
> +    st->st_size);
> + if (kv_add(&resp->http_headers, "Content-Range",
> +    content_range) == NULL)
> + goto abort;
> +
> + content_length = range->end - range->start + 1;
> + if (buffer_add_range(fd, evb, range) == 0)
> + goto abort;
> +
> + } else {
> + content_length = 0;
> + boundary = arc4random();
> + /* Generate a multipart payload of byteranges */
> + while (nranges--) {
> + if ((i = evbuffer_add_printf(evb, "\r\n--%ud\r\n",
> +    boundary)) == -1)
> + goto abort;
> +
> + content_length += i;
> + if ((i = evbuffer_add_printf(evb,
> +    "Content-Type: %s/%s\r\n",
> +        media == NULL ? "application" : media->media_type,
> +        media == NULL ?
> +    "octet-stream" : media->media_subtype)) == -1)
> + goto abort;
> +
> + content_length += i;
> + if ((i = evbuffer_add_printf(evb,
> +    "Content-Range: bytes %lld-%lld/%lld\r\n\r\n",
> +    range->start, range->end, st->st_size)) == -1)
> + goto abort;
> +
> + content_length += i;
> + if (buffer_add_range(fd, evb, range) == 0)
> + goto abort;
> +
> + content_length += range->end - range->start + 1;
> + range++;
> + }
> +
> + if ((i = evbuffer_add_printf(evb, "\r\n--%ud--\r\n",
> +    boundary)) == -1)
> + goto abort;
> +
> + content_length += i;
> +
> + /* prepare multipart/byteranges media type */
> + (void)strlcpy(multipart_media.media_type, "multipart",
> +        sizeof(multipart_media.media_type));
> + (void)snprintf(multipart_media.media_subtype,
> +        sizeof(multipart_media.media_subtype),
> +        "byteranges; boundary=%ud", boundary);
> + media = &multipart_media;
> + }
> +
> + ret = server_response_http(clt, 206, media, content_length,
> +    MINIMUM(time(NULL), st->st_mtim.tv_sec));
> + switch (ret) {
> + case -1:
> + goto fail;
> + case 0:
> + /* Connection is already finished */
> + close(fd);
> + evbuffer_free(evb);
> + evb = NULL;
> + goto done;
> + default:
> + break;
> + }
> +
> + if (server_bufferevent_write_buffer(clt, evb) == -1)
> + goto fail;
> +
> + evbuffer_free(evb);
> + evb = NULL;
> +
> + bufferevent_enable(clt->clt_bev, EV_READ|EV_WRITE);
> + if (clt->clt_persist)
> + clt->clt_toread = TOREAD_HTTP_HEADER;
> + else
> + clt->clt_toread = TOREAD_HTTP_NONE;
> + clt->clt_done = 0;
> +
> + done:
> + server_reset_http(clt);
> + return (0);
> + fail:
> + bufferevent_disable(clt->clt_bev, EV_READ|EV_WRITE);
> + bufferevent_free(clt->clt_bev);
> + clt->clt_bev = NULL;
> + abort:
> + if (errstr == NULL)
> + errstr = strerror(errno);
> + server_abort_http(clt, code, errstr);
> + return (-1);
> +}
> +
> +int
>  server_file_index(struct httpd *env, struct client *clt, struct stat *st)
>  {
>   char  path[PATH_MAX];
> @@ -469,3 +626,112 @@ server_file_error(struct bufferevent *be
>   server_close(clt, "unknown event error");
>   return;
>  }
> +
> +struct range *
> +parse_range(char *str, size_t file_sz, int *nranges)
> +{
> + static struct range ranges[MAX_RANGES];
> + int i = 0;
> + char *p, *q;
> +
> + /* Extract range unit */
> + if ((p = strchr(str, '=')) == NULL)
> + return (NULL);
> +
> + *p++ = '\0';
> + /* Check if it's a bytes range spec */
> + if (strcmp(str, "bytes") != 0)
> + return (NULL);
> +
> + while ((q = strchr(p, ',')) != NULL) {
> + *q++ = '\0';
> +
> + /* Extract start and end positions */
> + if (parse_range_spec(p, file_sz, &ranges[i]) == 0)
> + continue;
> +
> + i++;
> + if (i == MAX_RANGES)
> + return (NULL);
> +
> + p = q;
> + }
> +
> + if (parse_range_spec(p, file_sz, &ranges[i]) != 0)
> + i++;
> +
> + *nranges = i;
> + return (i ? ranges : NULL);
> +}
> +
> +int
> +parse_range_spec(char *str, size_t size, struct range *r)
> +{
> + size_t start_str_len, end_str_len;
> + char *p, *start_str, *end_str;
> + const char *errstr;
> +
> + if ((p = strchr(str, '-')) == NULL)
> + return (0);
> +
> + *p++ = '\0';
> + start_str = str;
> + end_str = p;
> + start_str_len = strlen(start_str);
> + end_str_len = strlen(end_str);
> +
> + /* Either 'start' or 'end' is optional but not both */
> + if ((start_str_len == 0) && (end_str_len == 0))
> + return (0);
> +
> + if (end_str_len) {
> + r->end = strtonum(end_str, 0, LLONG_MAX, &errstr);
> + if (errstr)
> + return (0);
> +
> + if ((size_t)r->end >= size)
> + r->end = size - 1;
> + } else
> + r->end = size - 1;
> +
> + if (start_str_len) {
> + r->start = strtonum(start_str, 0, LLONG_MAX, &errstr);
> + if (errstr)
> + return (0);
> +
> + if ((size_t)r->start >= size)
> + return (0);
> + } else {
> + r->start = size - r->end;
> + r->end = size - 1;
> + }
> +
> + if (r->end < r->start)
> + return (0);
> +
> + return (1);
> +}
> +
> +int
> +buffer_add_range(int fd, struct evbuffer *evb, struct range *range)
> +{
> + char buf[BUFSIZ];
> + size_t n, range_sz;
> + ssize_t nread;
> +
> + if (lseek(fd, range->start, SEEK_SET) == -1)
> + return (0);
> +
> + range_sz = range->end - range->start + 1;
> + while (range_sz) {
> + n = MINIMUM(range_sz, sizeof(buf));
> + if ((nread = read(fd, buf, n)) == -1)
> + return (0);
> +
> + evbuffer_add(evb, buf, nread);
> + range_sz -= nread;
> + }
> +
> + return (1);
> +}
> +
> Index: server_http.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> retrieving revision 1.78
> diff -u -p -r1.78 server_http.c
> --- server_http.c 18 Apr 2015 09:27:54 -0000 1.78
> +++ server_http.c 3 May 2015 11:18:07 -0000
> @@ -788,6 +788,13 @@ server_abort_http(struct client *clt, u_
>   extraheader = NULL;
>   }
>   break;
> + case 416:
> + if (asprintf(&extraheader,
> +    "Content-Range: %s\r\n", msg) == -1) {
> + code = 500;
> + extraheader = NULL;
> + }
> + break;
>   default:
>   /*
>   * Do not send details of the error.  Traditionally,

--
I'm not entirely sure you are real.

Reply | Threaded
Open this post in threaded view
|

Re: Byte range implementation for httpd(8)

Sebastian Benoit-3
Florian Obser([hidden email]) on 2015.05.03 12:39:02 +0000:

> On Sun, May 03, 2015 at 01:46:56PM +0200, Sunil Nimmagadda wrote:
> > On Sat, May 02, 2015 at 02:49:30PM +0000, Florian Obser wrote:
> > > Sorry for the very late reply, I'm currently very busy :/
> >
> > Thank you for taking time to review it. A new patch with style nits
> > fixed and a gratuitous NULL check removed.
> >
> > [trimming some text]
> >  
> > > this is missing the server_file_method() song and dance from
> > > server_file_request()
> >
> > Fixed in a slightly different way than using server_file_method().
> > Since range request should be ignored for methods other than GET,
> > fallback to server_file_request() for further processing/validation.
>
> yep, this is good.
>
> >
> > > > + if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
> > > > + code = 416;
> > > > + (void)snprintf(content_range, sizeof(content_range),
> > > > +    "bytes */%lld", st->st_size);
> > > > + errstr = content_range;
> > > > + goto abort;
> > > > + }
> > >
> > > hm, apache answers with the full file and 200 if the range header is
> > > syntactically incorrect or if end < start. As far as I can tell it
> > > only answers 416 if the range actually lies outside of the file.
> >
> > I am confused about the RFC here, section 4.4 states that 416 is
> > returned if "...the set of ranges requested has been rejected due
> > to invalid ranges..." and a note follows immediately, "Because
> > servers are free to ignore Range, many implementations will simply
> > respond with the entire selected representation in a 200 response"
> >
> > As you noted apache returns 200 while nginx returns 416 for an
> > incorrect range. What should httpd do ideally?
>
> Thanks for checking nginx, I don't have one around.
> I noted this for 2 reasons:
> 1) I guess apache is the defacto standard, so if it behaves
> differently there might be a reason, maybe broken clients in the wild.
> 2) The way the code is currently structured it will require a bit of
> shuffling around if we ever want apache's behaviour.
>
> That being said, my understanding of the RFC is that your
> implementation is correct. And with nginx behaving this way, too, it
> shouldn't bite us in the future. :)
>
> >
> > > > +
> > > > + /* Accept byte ranges */
> > > > + if (code == 200 &&
> > > > +    kv_add(&resp->http_headers, "Accept-Ranges", "bytes") == NULL)
> > > >   return (-1);
> > >
> > > I don't think we should advertise ranges for all 200 results, for
> > > example we don't support it for directory indexes.
> >
> > Agree, since RFC says "server MAY send Accept-Range header" and
> > "clients MAY generate range requests without having received this
> > header field", dropping this header shouldn't make a difference.
> >
> > Comments?
>
> This looks good.
>
> OK florian@ if someone wants to commit it. Or give me an OK and I'll
> commit.
>

fwiw i dont know much about ranges, but this reads ok. two whitespace bits
below. otherwise ok benno.

one question though: whats the reasoning behind MAX_RANGES 4? nginx seems to
have a default of "unlimited" (which i think questionable), but what is
reasonably seen on the internet?

> >
> > Index: server_file.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> > retrieving revision 1.52
> > diff -u -p -r1.52 server_file.c
> > --- server_file.c 25 Apr 2015 14:40:35 -0000 1.52
> > +++ server_file.c 3 May 2015 11:18:07 -0000
> > @@ -36,12 +36,25 @@
> >  
> >  #define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
> >  #define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b))
> > +#define MAX_RANGES 4
> >  
> > -int server_file_access(struct httpd *, struct client *, char *, size_t);
> > -int server_file_request(struct httpd *, struct client *, char *,
> > -    struct stat *);
> > -int server_file_index(struct httpd *, struct client *, struct stat *);
> > -int server_file_method(struct client *);
> > +struct range {
> > + off_t start;
> > + off_t end;
> > +};
> > +
> > +int server_file_access(struct httpd *, struct client *,
> > +    char *, size_t);
> > +int server_file_request(struct httpd *, struct client *,
> > +    char *, struct stat *);
> > +int server_partial_file_request(struct httpd *, struct client *,
> > +    char *, struct stat *, char *);
> > +int server_file_index(struct httpd *, struct client *,
> > +    struct stat *);
> > +int server_file_method(struct client *);
> > +int parse_range_spec(char *, size_t, struct range *);
> > +struct range *parse_range(char *, size_t, int *);
> > +int buffer_add_range(int, struct evbuffer *, struct range *);
> >  
> >  int
> >  server_file_access(struct httpd *env, struct client *clt,
> > @@ -50,6 +63,7 @@ server_file_access(struct httpd *env, st
> >   struct http_descriptor *desc = clt->clt_descreq;
> >   struct server_config *srv_conf = clt->clt_srv_conf;
> >   struct stat st;
> > + struct kv *r, key;
> >   char *newpath;
> >   int ret;
> >  
> > @@ -123,7 +137,13 @@ server_file_access(struct httpd *env, st
> >   goto fail;
> >   }
> >  
> > - return (server_file_request(env, clt, path, &st));
> > + key.kv_key = "Range";
> > + r = kv_find(&desc->http_headers, &key);
> > + if (r != NULL)
> > + return (server_partial_file_request(env, clt, path, &st,
> > +    r->kv_value));
> > + else
> > + return (server_file_request(env, clt, path, &st));
> >  
> >   fail:
> >   switch (errno) {
> > @@ -262,6 +282,143 @@ server_file_request(struct httpd *env, s
> >  }
> >  
> >  int
> > +server_partial_file_request(struct httpd *env, struct client *clt, char *path,
> > +    struct stat *st, char *range_str)
> > +{
> > + struct http_descriptor *resp = clt->clt_descresp;
> > + struct http_descriptor *desc = clt->clt_descreq;
> > + struct media_type *media, multipart_media;
> > + struct range *range;
> > + struct evbuffer *evb = NULL;
> > + size_t content_length;
> > + int code = 500, fd = -1, i, nranges, ret;
> > + uint32_t boundary;
> > + char content_range[64];
> > + const char *errstr = NULL;
> > +
> > + /* Ignore range request for methods other than GET */
> > + if (desc->http_method != HTTP_METHOD_GET)
> > + return server_file_request(env, clt, path, st);
> > +
> > + if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
> > + code = 416;
> > + (void)snprintf(content_range, sizeof(content_range),
> > +    "bytes */%lld", st->st_size);
> > + errstr = content_range;
> > + goto abort;
> > + }
> > +
> > + /* Now open the file, should be readable or we have another problem */
> > + if ((fd = open(path, O_RDONLY)) == -1)
> > + goto abort;
> > +
> > + media = media_find(env->sc_mediatypes, path);
> > + if ((evb = evbuffer_new()) == NULL) {
> > + errstr = "failed to allocate file buffer";
> > + goto abort;
> > + }
> > +
> > + if (nranges == 1) {
> > + (void)snprintf(content_range, sizeof(content_range),
> > +        "bytes %lld-%lld/%lld", range->start, range->end,
> > +    st->st_size);
> > + if (kv_add(&resp->http_headers, "Content-Range",
> > +    content_range) == NULL)
> > + goto abort;
> > +
> > + content_length = range->end - range->start + 1;
> > + if (buffer_add_range(fd, evb, range) == 0)
> > + goto abort;
> > +
> > + } else {
> > + content_length = 0;
> > + boundary = arc4random();
> > + /* Generate a multipart payload of byteranges */
> > + while (nranges--) {
> > + if ((i = evbuffer_add_printf(evb, "\r\n--%ud\r\n",
> > +    boundary)) == -1)
> > + goto abort;
> > +
> > + content_length += i;
> > + if ((i = evbuffer_add_printf(evb,
> > +    "Content-Type: %s/%s\r\n",
> > +        media == NULL ? "application" : media->media_type,
> > +        media == NULL ?
> > +    "octet-stream" : media->media_subtype)) == -1)
> > + goto abort;
> > +
> > + content_length += i;
> > + if ((i = evbuffer_add_printf(evb,
> > +    "Content-Range: bytes %lld-%lld/%lld\r\n\r\n",
> > +    range->start, range->end, st->st_size)) == -1)
> > + goto abort;
> > +

whitespace on blan line

> > + content_length += i;
> > + if (buffer_add_range(fd, evb, range) == 0)
> > + goto abort;
> > +
> > + content_length += range->end - range->start + 1;
> > + range++;
> > + }
> > +
> > + if ((i = evbuffer_add_printf(evb, "\r\n--%ud--\r\n",
> > +    boundary)) == -1)
> > + goto abort;
> > +
> > + content_length += i;
> > +
> > + /* prepare multipart/byteranges media type */
> > + (void)strlcpy(multipart_media.media_type, "multipart",
> > +        sizeof(multipart_media.media_type));
> > + (void)snprintf(multipart_media.media_subtype,
> > +        sizeof(multipart_media.media_subtype),
> > +        "byteranges; boundary=%ud", boundary);
> > + media = &multipart_media;
> > + }
> > +
> > + ret = server_response_http(clt, 206, media, content_length,
> > +    MINIMUM(time(NULL), st->st_mtim.tv_sec));
> > + switch (ret) {
> > + case -1:
> > + goto fail;
> > + case 0:
> > + /* Connection is already finished */
> > + close(fd);
> > + evbuffer_free(evb);
> > + evb = NULL;
> > + goto done;
> > + default:
> > + break;
> > + }
> > +
> > + if (server_bufferevent_write_buffer(clt, evb) == -1)
> > + goto fail;
> > +
> > + evbuffer_free(evb);
> > + evb = NULL;
> > +
> > + bufferevent_enable(clt->clt_bev, EV_READ|EV_WRITE);
> > + if (clt->clt_persist)
> > + clt->clt_toread = TOREAD_HTTP_HEADER;
> > + else
> > + clt->clt_toread = TOREAD_HTTP_NONE;
> > + clt->clt_done = 0;
> > +
> > + done:
> > + server_reset_http(clt);
> > + return (0);
> > + fail:
> > + bufferevent_disable(clt->clt_bev, EV_READ|EV_WRITE);
> > + bufferevent_free(clt->clt_bev);
> > + clt->clt_bev = NULL;
> > + abort:
> > + if (errstr == NULL)
> > + errstr = strerror(errno);
> > + server_abort_http(clt, code, errstr);
> > + return (-1);
> > +}
> > +
> > +int
> >  server_file_index(struct httpd *env, struct client *clt, struct stat *st)
> >  {
> >   char  path[PATH_MAX];
> > @@ -469,3 +626,112 @@ server_file_error(struct bufferevent *be
> >   server_close(clt, "unknown event error");
> >   return;
> >  }
> > +
> > +struct range *
> > +parse_range(char *str, size_t file_sz, int *nranges)
> > +{
> > + static struct range ranges[MAX_RANGES];
> > + int i = 0;
> > + char *p, *q;
> > +
> > + /* Extract range unit */
> > + if ((p = strchr(str, '=')) == NULL)
> > + return (NULL);
> > +
> > + *p++ = '\0';
> > + /* Check if it's a bytes range spec */
> > + if (strcmp(str, "bytes") != 0)
> > + return (NULL);
> > +
> > + while ((q = strchr(p, ',')) != NULL) {
> > + *q++ = '\0';
> > +
> > + /* Extract start and end positions */
> > + if (parse_range_spec(p, file_sz, &ranges[i]) == 0)
> > + continue;
> > +
> > + i++;
> > + if (i == MAX_RANGES)
> > + return (NULL);
> > +
> > + p = q;
> > + }
> > +
> > + if (parse_range_spec(p, file_sz, &ranges[i]) != 0)
> > + i++;
> > +
> > + *nranges = i;
> > + return (i ? ranges : NULL);
> > +}
> > +
> > +int
> > +parse_range_spec(char *str, size_t size, struct range *r)
> > +{
> > + size_t start_str_len, end_str_len;
> > + char *p, *start_str, *end_str;
> > + const char *errstr;
> > +
> > + if ((p = strchr(str, '-')) == NULL)
> > + return (0);
> > +
> > + *p++ = '\0';
> > + start_str = str;
> > + end_str = p;
> > + start_str_len = strlen(start_str);
> > + end_str_len = strlen(end_str);
> > +
> > + /* Either 'start' or 'end' is optional but not both */
> > + if ((start_str_len == 0) && (end_str_len == 0))
> > + return (0);
> > +
> > + if (end_str_len) {
> > + r->end = strtonum(end_str, 0, LLONG_MAX, &errstr);
> > + if (errstr)
> > + return (0);
> > +
> > + if ((size_t)r->end >= size)
> > + r->end = size - 1;
> > + } else
> > + r->end = size - 1;
> > +
> > + if (start_str_len) {
> > + r->start = strtonum(start_str, 0, LLONG_MAX, &errstr);
> > + if (errstr)
> > + return (0);
> > +
> > + if ((size_t)r->start >= size)
> > + return (0);
> > + } else {
> > + r->start = size - r->end;
> > + r->end = size - 1;
> > + }
> > +
> > + if (r->end < r->start)
> > + return (0);
> > +
> > + return (1);
> > +}
> > +
> > +int
> > +buffer_add_range(int fd, struct evbuffer *evb, struct range *range)
> > +{
> > + char buf[BUFSIZ];
> > + size_t n, range_sz;
> > + ssize_t nread;
> > +
> > + if (lseek(fd, range->start, SEEK_SET) == -1)
> > + return (0);
> > +
> > + range_sz = range->end - range->start + 1;
> > + while (range_sz) {
> > + n = MINIMUM(range_sz, sizeof(buf));
> > + if ((nread = read(fd, buf, n)) == -1)
> > + return (0);
> > +
> > + evbuffer_add(evb, buf, nread);
> > + range_sz -= nread;
> > + }
> > +

whitespace on blank line

> > + return (1);
> > +}
> > +
> > Index: server_http.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> > retrieving revision 1.78
> > diff -u -p -r1.78 server_http.c
> > --- server_http.c 18 Apr 2015 09:27:54 -0000 1.78
> > +++ server_http.c 3 May 2015 11:18:07 -0000
> > @@ -788,6 +788,13 @@ server_abort_http(struct client *clt, u_
> >   extraheader = NULL;
> >   }
> >   break;
> > + case 416:
> > + if (asprintf(&extraheader,
> > +    "Content-Range: %s\r\n", msg) == -1) {
> > + code = 500;
> > + extraheader = NULL;
> > + }
> > + break;
> >   default:
> >   /*
> >   * Do not send details of the error.  Traditionally,
>
> --
> I'm not entirely sure you are real.
>

Reply | Threaded
Open this post in threaded view
|

Re: Byte range implementation for httpd(8)

Florian Obser-2
On Sun, May 03, 2015 at 08:14:25PM +0200, Sebastian Benoit wrote:
> one question though: whats the reasoning behind MAX_RANGES 4? nginx seems to
> have a default of "unlimited" (which i think questionable), but what is

Wasn't there a cve about this last year or so? You can try to burn cpu
and io on the server by requesting stupid ranges, like one byte at a
time, backwards for the whole file or something...

> reasonably seen on the internet?

my best guess is one range, from some byte position to the end, when
you resume a transfer.

--
I'm not entirely sure you are real.

Reply | Threaded
Open this post in threaded view
|

Re: Byte range implementation for httpd(8)

Ian Mcwilliam-5
This might be what your thinking of.

https://httpd.apache.org/security/CVE-2011-3192.txt

Description:
============

A denial of service vulnerability has been found in the way the multiple
overlapping ranges are handled by the Apache HTTPD server prior to version
2.2.20:

     http://seclists.org/fulldisclosure/2011/Aug/175

An attack tool is circulating in the wild. Active use of this tool has
been observed.

The attack can be done remotely and with a modest number of requests can
cause very significant memory and CPU usage on the server.



Ian McWilliam

________________________________________
From: [hidden email] [[hidden email]] on behalf of Florian Obser [[hidden email]]
Sent: Monday, 4 May 2015 4:34 AM
To: [hidden email]
Cc: Sunil Nimmagadda
Subject: Re: Byte range implementation for httpd(8)

On Sun, May 03, 2015 at 08:14:25PM +0200, Sebastian Benoit wrote:
> one question though: whats the reasoning behind MAX_RANGES 4? nginx seems to
> have a default of "unlimited" (which i think questionable), but what is

Wasn't there a cve about this last year or so? You can try to burn cpu
and io on the server by requesting stupid ranges, like one byte at a
time, backwards for the whole file or something...

> reasonably seen on the internet?

my best guess is one range, from some byte position to the end, when
you resume a transfer.

--
I'm not entirely sure you are real.