diff --git a/http_server.c b/http_server.c index 60877899..69074709 100644 --- a/http_server.c +++ b/http_server.c @@ -519,21 +519,30 @@ static inline int _parse_uint(struct http_request *r, unsigned int *uintp) static unsigned _parse_ranges(struct http_request *r, struct http_range *range, unsigned nrange) { - unsigned i; - for (i = 0; 1; ++i) { + unsigned i = 0; + while (1) { enum http_range_type type; http_size_t first = 0, last = 0; - if (_skip_literal(r, "-") && _parse_http_size_t(r, &last)) + if (_skip_literal(r, "-")) { + if (!_parse_http_size_t(r, &last)) + return 0; type = SUFFIX; - else if (_parse_http_size_t(r, &first) && _skip_literal(r, "-")) - type = (_parse_http_size_t(r, &last)) ? CLOSED : OPEN; - else + } + else if (_parse_http_size_t(r, &first) && _skip_literal(r, "-")) { + if (_parse_http_size_t(r, &last)) { + if (last < first) + return 0; + type = CLOSED; + } else + type = OPEN; + } else return 0; if (i < nrange) { range[i].type = type; range[i].first = first; range[i].last = last; } + ++i; if (!_skip_literal(r, ",")) break; _skip_optional_space(r); @@ -1291,7 +1300,9 @@ static void http_request_send_response(struct http_request *r) assert(r->response_buffer_sent <= r->response_buffer_length); if (r->response_buffer_sent == r->response_buffer_length) { if (r->response.content_generator) { - // Content generator must fill set (or re-set) response_buffer and response_buffer_length. + // Content generator must fill or partly fill response_buffer and set response_buffer_sent + // and response_buffer_length. May also malloc() a bigger buffer and set response_buffer to + // point to it. r->response_buffer_sent = r->response_buffer_length = 0; if (r->response.content_generator(r) == -1) { if (r->debug_flag && *r->debug_flag) @@ -1380,32 +1391,23 @@ static void http_server_poll(struct sched_ent *alarm) r->free(r); // after this, *r is no longer valid } -/* Count the number of bytes in a given set of ranges, given the length of the content to which the - * ranges will be applied. - * - * @author Andrew Bettison - */ -http_size_t http_range_bytes(const struct http_range *range, unsigned nranges, http_size_t resource_length) -{ - return http_range_close(NULL, range, nranges, resource_length); -} - /* Copy the array of byte ranges, closing it (converting all ranges to CLOSED) using the supplied - * resource length. + * resource length. If a range is not satisfiable it is omitted from 'dst'. Returns the number of + * closed ranges written to 'dst'. * * @author Andrew Bettison */ -http_size_t http_range_close(struct http_range *dst, const struct http_range *src, unsigned nranges, http_size_t resource_length) +unsigned http_range_close(struct http_range *dst, const struct http_range *src, unsigned nranges, http_size_t resource_length) { - http_size_t bytes = 0; unsigned i; + unsigned ndst = 0; for (i = 0; i != nranges; ++i) { http_size_t first = 0; - http_size_t last = resource_length; + http_size_t last = resource_length - 1; const struct http_range *range = &src[i]; switch (range->type) { case CLOSED: - last = range->last < resource_length ? range->last : resource_length; + last = range->last < resource_length ? range->last : resource_length - 1; case OPEN: first = range->first < resource_length ? range->first : resource_length; break; @@ -1415,10 +1417,25 @@ http_size_t http_range_close(struct http_range *dst, const struct http_range *sr default: abort(); // not reached } - assert(first <= last); - if (dst) - *dst++ = (struct http_range){ .type = CLOSED, .first=first, .last=last }; - bytes += last - first; + if (first <= last) + dst[ndst++] = (struct http_range){ .type = CLOSED, .first=first, .last=last }; + } + return ndst; +} + +/* Return the total number of bytes represented by the given ranges which must all be CLOSED and + * valid. + * + * @author Andrew Bettison + */ +http_size_t http_range_bytes(const struct http_range *range, unsigned nranges) +{ + http_size_t bytes = 0; + unsigned i; + for (i = 0; i != nranges; ++i) { + assert(range[i].type == CLOSED); + assert(range[i].last >= range[i].first); + bytes += range[i].last - range[i].first + 1; } return bytes; } @@ -1467,11 +1484,17 @@ static strbuf strbuf_append_quoted_string(strbuf sb, const char *str) */ static int _render_response(struct http_request *r) { - strbuf sb = strbuf_local(r->response_buffer, r->response_buffer_size); struct http_response hr = r->response; assert(hr.result_code != 0); + assert(hr.header.content_range_start <= hr.header.resource_length); + assert(hr.header.content_length <= hr.header.resource_length); + // To save page handlers having to decide between 200 (OK) and 206 (Partial Content), they can + // just send 200 and the content range fields, and this logic will detect if it should be 206. + if (hr.header.content_length > 0 && hr.header.content_length < hr.header.resource_length && hr.result_code == 200) + hr.result_code = 206; // Partial Content const char *result_string = httpResultString(hr.result_code); - if (hr.content == NULL) { + strbuf sb = strbuf_local(r->response_buffer, r->response_buffer_size); + if (hr.content == NULL && hr.content_generator == NULL) { strbuf cb = strbuf_alloca(100 + strlen(result_string)); strbuf_puts(cb, "

"); strbuf_puts(cb, result_string); @@ -1493,23 +1516,32 @@ static int _render_response(struct http_request *r) strbuf_puts(sb, hr.header.boundary); } strbuf_puts(sb, "\r\n"); - assert(hr.header.content_range_start <= hr.header.resource_length); - assert(hr.header.content_length <= hr.header.resource_length); - if (hr.header.content_range_start && hr.header.content_length) + if (hr.result_code == 206) { + // Must only use result code 206 (Partial Content) if the content is in fact less than the whole + // resource length. + assert(hr.header.content_length > 0); + assert(hr.header.content_length < hr.header.resource_length); strbuf_sprintf(sb, "Content-Range: bytes %"PRIhttp_size_t"-%"PRIhttp_size_t"/%"PRIhttp_size_t"\r\n", hr.header.content_range_start, hr.header.content_range_start + hr.header.content_length - 1, hr.header.resource_length ); + } strbuf_sprintf(sb, "Content-Length: %"PRIhttp_size_t"\r\n", hr.header.content_length); strbuf_puts(sb, "\r\n"); - r->response_length = strbuf_len(sb) + hr.header.content_length; - if (strbuf_overrun(sb) || (r->response_buffer_size < r->response_length)) + if (strbuf_overrun(sb)) return 0; - bcopy(hr.content, strbuf_end(sb), hr.header.content_length); + r->response_length = strbuf_len(sb) + hr.header.content_length; + if (hr.content) { + if (r->response_buffer_size < r->response_length) + return 0; + bcopy(hr.content, strbuf_end(sb), hr.header.content_length); + r->response_buffer_length = r->response_length; + } else { + r->response_buffer_length = strbuf_len(sb); + } r->response_buffer_sent = 0; - r->response_buffer_length = r->response_length; return 1; } @@ -1584,7 +1616,7 @@ static void http_request_start_response(struct http_request *r) } r->response_sent = 0; if (r->debug_flag && *r->debug_flag) - DEBUGF("Sending HTTP response: %s", alloca_toprint(160, (const char *)r->response_buffer, r->response_length)); + DEBUGF("Sending HTTP response: %s", alloca_toprint(160, (const char *)r->response_buffer, r->response_buffer_length)); r->phase = TRANSMIT; r->alarm.poll.events = POLLOUT; watch(&r->alarm); diff --git a/http_server.h b/http_server.h index ebedb0ad..3cfad0a3 100644 --- a/http_server.h +++ b/http_server.h @@ -51,8 +51,8 @@ struct http_range { http_size_t last; // only for CLOSED or SUFFIX }; -http_size_t http_range_close(struct http_range *dst, const struct http_range *src, unsigned nranges, http_size_t content_length); -http_size_t http_range_bytes(const struct http_range *range, unsigned nranges, http_size_t content_length); +unsigned http_range_close(struct http_range *dst, const struct http_range *src, unsigned nranges, http_size_t content_length); +http_size_t http_range_bytes(const struct http_range *range, unsigned nranges); #define CONTENT_LENGTH_UNKNOWN UINT64_MAX diff --git a/rhizome.h b/rhizome.h index 4188327d..e856ff54 100644 --- a/rhizome.h +++ b/rhizome.h @@ -707,7 +707,7 @@ int rhizome_fetch_status_html(struct strbuf *b); int rhizome_fetch_has_queue_space(unsigned char log2_size); struct http_response_parts { - int code; + uint16_t code; char *reason; int64_t range_start; int64_t content_length; diff --git a/rhizome_fetch.c b/rhizome_fetch.c index 6713c193..d0b61e4e 100644 --- a/rhizome_fetch.c +++ b/rhizome_fetch.c @@ -1495,9 +1495,9 @@ void rhizome_fetch_poll(struct sched_ent *alarm) rhizome_fetch_switch_to_mdp(slot); return; } - if (parts.code != 200) { + if (parts.code != 200 && parts.code != 206) { if (config.debug.rhizome_rx) - DEBUGF("Failed HTTP request: rhizome server returned %d != 200 OK", parts.code); + DEBUGF("Failed HTTP request: rhizome server returned %03u", parts.code); rhizome_fetch_switch_to_mdp(slot); return; } @@ -1576,7 +1576,7 @@ void rhizome_fetch_poll(struct sched_ent *alarm) int unpack_http_response(char *response, struct http_response_parts *parts) { IN(); - parts->code = -1; + parts->code = 0; parts->reason = NULL; parts->range_start=0; parts->content_length = -1; diff --git a/rhizome_http.c b/rhizome_http.c index b0c04668..dc83d203 100644 --- a/rhizome_http.c +++ b/rhizome_http.c @@ -381,11 +381,10 @@ static int rhizome_status_page(rhizome_http_request *r, const char *remainder) static int rhizome_file_content(struct http_request *hr) { rhizome_http_request *r = (rhizome_http_request *) hr; - assert(r->http.response_length < r->http.response_buffer_size); - assert(r->read_state.offset <= r->read_state.length); + assert(r->http.response_buffer_sent == 0); + assert(r->http.response_buffer_length == 0); + assert(r->read_state.offset < r->read_state.length); uint64_t readlen = r->read_state.length - r->read_state.offset; - if (readlen == 0) - return 0; size_t suggested_size = 64 * 1024; if (suggested_size > readlen) suggested_size = readlen; @@ -395,14 +394,13 @@ static int rhizome_file_content(struct http_request *hr) http_request_set_response_bufsize(&r->http, 1); if (r->http.response_buffer == NULL) return -1; - size_t space = r->http.response_buffer_size - r->http.response_length; - int len = rhizome_read(&r->read_state, - (unsigned char *)r->http.response_buffer + r->http.response_length, - space); + ssize_t len = rhizome_read(&r->read_state, + (unsigned char *)r->http.response_buffer, + r->http.response_buffer_size); if (len == -1) return -1; - assert(len <= space); - r->http.response_length += len; + assert((size_t) len <= r->http.response_buffer_size); + r->http.response_buffer_length += (size_t) len; return 0; } @@ -437,25 +435,24 @@ static int rhizome_file_page(rhizome_http_request *r, const char *remainder) return 1; } assert(r->read_state.length != -1); - int result_code = 200; - struct http_range closed = (struct http_range){ .first = 0, .last = r->read_state.length }; + r->http.response.header.resource_length = r->read_state.length; if (r->http.request_header.content_range_count > 0) { - if (http_range_bytes(r->http.request_header.content_ranges, - r->http.request_header.content_range_count, - r->read_state.length - ) == 0 - ) { + assert(r->http.request_header.content_range_count == 1); + struct http_range closed; + unsigned n = http_range_close(&closed, r->http.request_header.content_ranges, 1, r->read_state.length); + if (n == 0 || http_range_bytes(&closed, 1) == 0) { http_request_simple_response(&r->http, 416, NULL); // Request Range Not Satisfiable return 0; } - result_code = 206; // Partial Content - http_range_close(&closed, &r->http.request_header.content_ranges[0], 1, r->read_state.length); + r->http.response.header.content_range_start = closed.first; + r->http.response.header.content_length = closed.last - closed.first + 1; + r->read_state.offset = closed.first; + } else { + r->http.response.header.content_range_start = 0; + r->http.response.header.content_length = r->http.response.header.resource_length; + r->read_state.offset = 0; } - r->http.response.header.content_range_start = closed.first; - r->http.response.header.resource_length = closed.last; - r->http.response.header.content_length = closed.last - closed.first; - r->read_state.offset = closed.first; - http_request_response_generated(&r->http, result_code, "application/binary", rhizome_file_content); + http_request_response_generated(&r->http, 200, "application/binary", rhizome_file_content); return 0; } diff --git a/tests/rhizomeprotocol b/tests/rhizomeprotocol index 67d2bbd0..24060cf1 100755 --- a/tests/rhizomeprotocol +++ b/tests/rhizomeprotocol @@ -514,12 +514,13 @@ test_CorruptPayload() { wait_until grep -i "Stored file $FILEHASH" $LOGA } -doc_HttpFetchRange="Fetch a file range using HTTP GET." +doc_HttpFetchRange="Fetch a file range using HTTP GET" setup_HttpFetchRange() { setup_curl 7 setup_common set_instance +A - rhizome_add_file file1 + rhizome_add_file file1 100 + tail --bytes +33 file1 >file1.tail start_servald_instances +A wait_until rhizome_http_server_started +A get_rhizome_server_port PORTA +A @@ -532,9 +533,11 @@ test_HttpFetchRange() { --write-out '%{http_code}\n' \ --continue-at 32 \ "http://$addr_localhost:$PORTA/rhizome/file/$FILEHASH" - tfw_cat http.headers http.output - assertGrep http.headers "Content-range:" - assertGrep http.headers "Content-length:" + tfw_cat -v http.headers http.output + assertGrep http.headers "^Content-Range: bytes 32-99/100 $" + assertGrep http.headers "^Content-Length: 68 $" + tfw_cat -v file1.tail http.output + assert cmp file1.tail http.output } doc_HttpImport="Import bundle using HTTP POST multi-part form."