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.
