Skip to content

Commit 142cfc8

Browse files
committed
Reimplement httpd's support for byte ranges.
The previous implementation loaded all the output into a single output buffer and used its size to determine the Content-Length of the body. The new implementation calculates the body length first and writes the individual ranges in an async way using the bufferevent mechanism. This prevents httpd from using too much memory and applies the watermark and throttling mechanisms to range requests. Problem reported by Pierre Kim (pierre.kim.sec at gmail.com) OK benno@ sunil@
1 parent 9bb95fa commit 142cfc8

File tree

3 files changed

+183
-98
lines changed

3 files changed

+183
-98
lines changed

Diff for: usr.sbin/httpd/httpd.h

+23-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: httpd.h,v 1.126 2017/01/31 12:21:27 reyk Exp $ */
1+
/* $OpenBSD: httpd.h,v 1.127 2017/01/31 14:39:47 reyk Exp $ */
22

33
/*
44
* Copyright (c) 2006 - 2015 Reyk Floeter <reyk@openbsd.org>
@@ -73,6 +73,7 @@
7373
#define SERVER_MAX_PREFETCH 256
7474
#define SERVER_MIN_PREFETCHED 32
7575
#define SERVER_HSTS_DEFAULT_AGE 31536000
76+
#define SERVER_MAX_RANGES 4
7677

7778
#define MEDIATYPE_NAMEMAX 128 /* file name extension */
7879
#define MEDIATYPE_TYPEMAX 64 /* length of type/subtype */
@@ -93,7 +94,8 @@ enum httpchunk {
9394
TOREAD_HTTP_HEADER = -2,
9495
TOREAD_HTTP_CHUNK_LENGTH = -3,
9596
TOREAD_HTTP_CHUNK_TRAILER = -4,
96-
TOREAD_HTTP_NONE = -5
97+
TOREAD_HTTP_NONE = -5,
98+
TOREAD_HTTP_RANGE = TOREAD_HTTP_CHUNK_LENGTH
9799
};
98100

99101
#if DEBUG
@@ -295,6 +297,22 @@ struct fcgi_data {
295297
int headersdone;
296298
};
297299

300+
struct range {
301+
off_t start;
302+
off_t end;
303+
};
304+
305+
struct range_data {
306+
struct range range[SERVER_MAX_RANGES];
307+
int range_count;
308+
int range_index;
309+
off_t range_toread;
310+
311+
/* For the Content headers in each part */
312+
struct media_type *range_media;
313+
size_t range_total;
314+
};
315+
298316
struct client {
299317
uint32_t clt_id;
300318
pid_t clt_pid;
@@ -313,6 +331,7 @@ struct client {
313331
void *clt_descreq;
314332
void *clt_descresp;
315333
int clt_sndbufsiz;
334+
uint64_t clt_boundary;
316335

317336
int clt_fd;
318337
struct tls *clt_tls_ctx;
@@ -327,6 +346,7 @@ struct client {
327346
int clt_done;
328347
int clt_chunk;
329348
int clt_inflight;
349+
struct range_data clt_ranges;
330350
struct fcgi_data clt_fcgi;
331351
char *clt_remote_user;
332352
struct evbuffer *clt_srvevb;
@@ -601,6 +621,7 @@ const char
601621
*server_httperror_byid(unsigned int);
602622
void server_read_httpcontent(struct bufferevent *, void *);
603623
void server_read_httpchunks(struct bufferevent *, void *);
624+
void server_read_httprange(struct bufferevent *, void *);
604625
int server_writeheader_http(struct client *clt, struct kv *, void *);
605626
int server_headers(struct client *, void *,
606627
int (*)(struct client *, struct kv *, void *), void *);

Diff for: usr.sbin/httpd/server_file.c

+63-94
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
/* $OpenBSD: server_file.c,v 1.63 2017/01/30 09:54:41 reyk Exp $ */
1+
/* $OpenBSD: server_file.c,v 1.64 2017/01/31 14:39:47 reyk Exp $ */
22

33
/*
4-
* Copyright (c) 2006 - 2015 Reyk Floeter <reyk@openbsd.org>
4+
* Copyright (c) 2006 - 2017 Reyk Floeter <reyk@openbsd.org>
55
*
66
* Permission to use, copy, modify, and distribute this software for any
77
* purpose with or without fee is hereby granted, provided that the above
@@ -36,12 +36,6 @@
3636

3737
#define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
3838
#define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b))
39-
#define MAX_RANGES 4
40-
41-
struct range {
42-
off_t start;
43-
off_t end;
44-
};
4539

4640
int server_file_access(struct httpd *, struct client *,
4741
char *, size_t);
@@ -55,8 +49,7 @@ int server_file_modified_since(struct http_descriptor *,
5549
struct stat *);
5650
int server_file_method(struct client *);
5751
int parse_range_spec(char *, size_t, struct range *);
58-
struct range *parse_range(char *, size_t, int *);
59-
int buffer_add_range(int, struct evbuffer *, struct range *);
52+
int parse_ranges(struct client *, char *, size_t);
6053

6154
int
6255
server_file_access(struct httpd *env, struct client *clt,
@@ -303,19 +296,18 @@ server_partial_file_request(struct httpd *env, struct client *clt, char *path,
303296
struct http_descriptor *resp = clt->clt_descresp;
304297
struct http_descriptor *desc = clt->clt_descreq;
305298
struct media_type *media, multipart_media;
299+
struct range_data *r = &clt->clt_ranges;
306300
struct range *range;
307-
struct evbuffer *evb = NULL;
308-
size_t content_length;
301+
size_t content_length = 0;
309302
int code = 500, fd = -1, i, nranges, ret;
310-
uint32_t boundary;
311303
char content_range[64];
312304
const char *errstr = NULL;
313305

314306
/* Ignore range request for methods other than GET */
315307
if (desc->http_method != HTTP_METHOD_GET)
316308
return server_file_request(env, clt, path, st);
317309

318-
if ((range = parse_range(range_str, st->st_size, &nranges)) == NULL) {
310+
if ((nranges = parse_ranges(clt, range_str, st->st_size)) < 1) {
319311
code = 416;
320312
(void)snprintf(content_range, sizeof(content_range),
321313
"bytes */%lld", st->st_size);
@@ -328,69 +320,57 @@ server_partial_file_request(struct httpd *env, struct client *clt, char *path,
328320
goto abort;
329321

330322
media = media_find_config(env, srv_conf, path);
331-
if ((evb = evbuffer_new()) == NULL) {
332-
errstr = "failed to allocate file buffer";
333-
goto abort;
334-
}
323+
r->range_media = media;
335324

336325
if (nranges == 1) {
326+
range = &r->range[0];
337327
(void)snprintf(content_range, sizeof(content_range),
338328
"bytes %lld-%lld/%lld", range->start, range->end,
339329
st->st_size);
340330
if (kv_add(&resp->http_headers, "Content-Range",
341331
content_range) == NULL)
342332
goto abort;
343333

344-
content_length = range->end - range->start + 1;
345-
if (buffer_add_range(fd, evb, range) == 0)
346-
goto abort;
347-
334+
range = &r->range[0];
335+
content_length += range->end - range->start + 1;
348336
} else {
349-
content_length = 0;
350-
boundary = arc4random();
351-
/* Generate a multipart payload of byteranges */
352-
while (nranges--) {
353-
if ((i = evbuffer_add_printf(evb, "\r\n--%ud\r\n",
354-
boundary)) == -1)
355-
goto abort;
337+
/* Add boundary, all parts will be handled by the callback */
338+
arc4random_buf(&clt->clt_boundary, sizeof(clt->clt_boundary));
356339

357-
content_length += i;
358-
if ((i = evbuffer_add_printf(evb,
359-
"Content-Type: %s/%s\r\n",
360-
media->media_type, media->media_subtype)) == -1)
361-
goto abort;
340+
/* Calculate Content-Length of the complete multipart body */
341+
for (i = 0; i < nranges; i++) {
342+
range = &r->range[i];
362343

363-
content_length += i;
364-
if ((i = evbuffer_add_printf(evb,
344+
/* calculate Content-Length of the complete body */
345+
if ((ret = snprintf(NULL, 0,
346+
"\r\n--%llu\r\n"
347+
"Content-Type: %s/%s\r\n"
365348
"Content-Range: bytes %lld-%lld/%lld\r\n\r\n",
366-
range->start, range->end, st->st_size)) == -1)
349+
clt->clt_boundary,
350+
media->media_type, media->media_subtype,
351+
range->start, range->end, st->st_size)) < 0)
367352
goto abort;
368353

369-
content_length += i;
370-
if (buffer_add_range(fd, evb, range) == 0)
371-
goto abort;
354+
/* Add data length */
355+
content_length += ret + range->end - range->start + 1;
372356

373-
content_length += range->end - range->start + 1;
374-
range++;
375357
}
376-
377-
if ((i = evbuffer_add_printf(evb, "\r\n--%ud--\r\n",
378-
boundary)) == -1)
358+
if ((ret = snprintf(NULL, 0, "\r\n--%llu--\r\n",
359+
clt->clt_boundary)) < 0)
379360
goto abort;
380-
381-
content_length += i;
361+
content_length += ret;
382362

383363
/* prepare multipart/byteranges media type */
384364
(void)strlcpy(multipart_media.media_type, "multipart",
385365
sizeof(multipart_media.media_type));
386366
(void)snprintf(multipart_media.media_subtype,
387367
sizeof(multipart_media.media_subtype),
388-
"byteranges; boundary=%ud", boundary);
368+
"byteranges; boundary=%llu", clt->clt_boundary);
389369
media = &multipart_media;
390370
}
391371

392-
close(fd);
393-
fd = -1;
372+
/* Start with first range */
373+
r->range_toread = TOREAD_HTTP_RANGE;
394374

395375
ret = server_response_http(clt, 206, media, content_length,
396376
MINIMUM(time(NULL), st->st_mtim.tv_sec));
@@ -399,32 +379,41 @@ server_partial_file_request(struct httpd *env, struct client *clt, char *path,
399379
goto fail;
400380
case 0:
401381
/* Connection is already finished */
382+
close(fd);
402383
goto done;
403384
default:
404385
break;
405386
}
406387

407-
if (server_bufferevent_write_buffer(clt, evb) == -1)
388+
clt->clt_fd = fd;
389+
if (clt->clt_srvbev != NULL)
390+
bufferevent_free(clt->clt_srvbev);
391+
392+
clt->clt_srvbev_throttled = 0;
393+
clt->clt_srvbev = bufferevent_new(clt->clt_fd, server_read_httprange,
394+
server_write, server_file_error, clt);
395+
if (clt->clt_srvbev == NULL) {
396+
errstr = "failed to allocate file buffer event";
408397
goto fail;
398+
}
409399

410-
bufferevent_enable(clt->clt_bev, EV_READ|EV_WRITE);
411-
if (clt->clt_persist)
412-
clt->clt_toread = TOREAD_HTTP_HEADER;
413-
else
414-
clt->clt_toread = TOREAD_HTTP_NONE;
415-
clt->clt_done = 0;
400+
/* Adjust read watermark to the socket output buffer size */
401+
bufferevent_setwatermark(clt->clt_srvbev, EV_READ, 0,
402+
clt->clt_sndbufsiz);
403+
404+
bufferevent_settimeout(clt->clt_srvbev,
405+
srv_conf->timeout.tv_sec, srv_conf->timeout.tv_sec);
406+
bufferevent_enable(clt->clt_srvbev, EV_READ);
407+
bufferevent_disable(clt->clt_bev, EV_READ);
416408

417409
done:
418-
evbuffer_free(evb);
419410
server_reset_http(clt);
420411
return (0);
421412
fail:
422413
bufferevent_disable(clt->clt_bev, EV_READ|EV_WRITE);
423414
bufferevent_free(clt->clt_bev);
424415
clt->clt_bev = NULL;
425416
abort:
426-
if (evb != NULL)
427-
evbuffer_free(evb);
428417
if (fd != -1)
429418
close(fd);
430419
if (errstr == NULL)
@@ -668,41 +657,44 @@ server_file_modified_since(struct http_descriptor *desc, struct stat *st)
668657
return (-1);
669658
}
670659

671-
struct range *
672-
parse_range(char *str, size_t file_sz, int *nranges)
660+
int
661+
parse_ranges(struct client *clt, char *str, size_t file_sz)
673662
{
674-
static struct range ranges[MAX_RANGES];
675663
int i = 0;
676664
char *p, *q;
665+
struct range_data *r = &clt->clt_ranges;
666+
667+
memset(r, 0, sizeof(*r));
677668

678669
/* Extract range unit */
679670
if ((p = strchr(str, '=')) == NULL)
680-
return (NULL);
671+
return (-1);
681672

682673
*p++ = '\0';
683674
/* Check if it's a bytes range spec */
684675
if (strcmp(str, "bytes") != 0)
685-
return (NULL);
676+
return (-1);
686677

687678
while ((q = strchr(p, ',')) != NULL) {
688679
*q++ = '\0';
689680

690681
/* Extract start and end positions */
691-
if (parse_range_spec(p, file_sz, &ranges[i]) == 0)
682+
if (parse_range_spec(p, file_sz, &r->range[i]) == 0)
692683
continue;
693684

694685
i++;
695-
if (i == MAX_RANGES)
696-
return (NULL);
686+
if (i == SERVER_MAX_RANGES)
687+
return (-1);
697688

698689
p = q;
699690
}
700691

701-
if (parse_range_spec(p, file_sz, &ranges[i]) != 0)
692+
if (parse_range_spec(p, file_sz, &r->range[i]) != 0)
702693
i++;
703694

704-
*nranges = i;
705-
return (i ? ranges : NULL);
695+
r->range_total = file_sz;
696+
r->range_count = i;
697+
return (i);
706698
}
707699

708700
int
@@ -752,26 +744,3 @@ parse_range_spec(char *str, size_t size, struct range *r)
752744

753745
return (1);
754746
}
755-
756-
int
757-
buffer_add_range(int fd, struct evbuffer *evb, struct range *range)
758-
{
759-
char buf[BUFSIZ];
760-
size_t n, range_sz;
761-
ssize_t nread;
762-
763-
if (lseek(fd, range->start, SEEK_SET) == -1)
764-
return (0);
765-
766-
range_sz = range->end - range->start + 1;
767-
while (range_sz) {
768-
n = MINIMUM(range_sz, sizeof(buf));
769-
if ((nread = read(fd, buf, n)) == -1)
770-
return (0);
771-
772-
evbuffer_add(evb, buf, nread);
773-
range_sz -= nread;
774-
}
775-
776-
return (1);
777-
}

0 commit comments

Comments
 (0)