commit db4e35d3d5cbd5bfac61cfb8edadd47c4608a864
Author: Laslo Hunhold <[email protected]>
AuthorDate: Thu Jul 23 18:16:08 2020 +0200
Commit: Laslo Hunhold <[email protected]>
CommitDate: Thu Jul 23 18:16:08 2020 +0200
Refactor range-parsing
Quark previously didn't really handle suffix-range-requests
(those of the form "-num", asking for the last num bytes) properly
and also did not catch the error when the lower in the range
"lower-upper" was actually larger than or equal to the size of the
requested file.
I always planned to refactor the parsing but got the motivation by
Eric Radman <[email protected]>, who kindly reported the latter bug
to me.
Signed-off-by: Laslo Hunhold <[email protected]>
diff --git a/http.c b/http.c
index 899862e..b8e9b69 100644
--- a/http.c
+++ b/http.c
@@ -562,36 +562,95 @@ http_send_response(int fd, struct request *r)
return http_send_status(fd, S_BAD_REQUEST);
}
*(q++) = '\0';
- if (p[0]) {
+
+ /*
+ * byte-range=first\0last...
+ * ^ ^
+ * | |
+ * p q
+ */
+
+ /*
+ * make sure we only have a single range,
+ * and not a comma separated list, which we
+ * will refuse to accept out of spite towards
+ * this horrible part of the spec
+ */
+ if (strchr(q, ',')) {
+ goto not_satisfiable;
+ }
+
+ if (p[0] != '\0') {
+ /*
+ * Range has format "first-last" or "first-",
+ * i.e. return bytes 'first' to 'last' (or the
+ * last byte if 'last' is not given),
+ * inclusively, and byte-numbering beginning at 0
+ */
lower = strtonum(p, 0, LLONG_MAX, &err);
- }
- if (!err && q[0]) {
+ if (!err) {
+ if (q[0] != '\0') {
+ upper = strtonum(q, 0, LLONG_MAX,
+ &err);
+ } else {
+ upper = st.st_size - 1;
+ }
+ }
+ if (err) {
+ /* one of the strtonum()'s failed */
+ return http_send_status(fd, S_BAD_REQUEST);
+ }
+
+ /* check ranges */
+ if (lower > upper || lower >= st.st_size) {
+ goto not_satisfiable;
+ }
+
+ /* adjust upper limit to be at most the last byte */
+ upper = MIN(upper, st.st_size - 1);
+ } else {
+ /*
+ * Range has format "-num", i.e. return the 'num'
+ * last bytes
+ */
+
+ /*
+ * use upper as a temporary storage for 'num',
+ * as we know 'upper' is st.st_size - 1
+ */
upper = strtonum(q, 0, LLONG_MAX, &err);
- }
- if (err) {
- return http_send_status(fd, S_BAD_REQUEST);
- }
+ if (err) {
+ return http_send_status(fd, S_BAD_REQUEST);
+ }
- /* check range */
- if (lower < 0 || upper < 0 || lower > upper) {
- if (dprintf(fd,
- "HTTP/1.1 %d %s\r\n"
- "Date: %s\r\n"
- "Content-Range: bytes */%zu\r\n"
- "Connection: close\r\n"
- "\r\n",
- S_RANGE_NOT_SATISFIABLE,
- status_str[S_RANGE_NOT_SATISFIABLE],
- timestamp(time(NULL), t),
- st.st_size) < 0) {
- return S_REQUEST_TIMEOUT;
+ /* determine lower */
+ if (upper > st.st_size) {
+ /* more bytes requested than we have */
+ lower = 0;
+ } else {
+ lower = st.st_size - upper;
}
- return S_RANGE_NOT_SATISFIABLE;
- }
- /* adjust upper limit */
- if (upper >= st.st_size)
- upper = st.st_size-1;
+ /* set upper to the correct value */
+ upper = st.st_size - 1;
+ }
+ goto satisfiable;
+not_satisfiable:
+ if (dprintf(fd,
+ "HTTP/1.1 %d %s\r\n"
+ "Date: %s\r\n"
+ "Content-Range: bytes */%zu\r\n"
+ "Connection: close\r\n"
+ "\r\n",
+ S_RANGE_NOT_SATISFIABLE,
+ status_str[S_RANGE_NOT_SATISFIABLE],
+ timestamp(time(NULL), t),
+ st.st_size) < 0) {
+ return S_REQUEST_TIMEOUT;
+ }
+ return S_RANGE_NOT_SATISFIABLE;
+satisfiable:
+ ;
}
/* mime */
