quark

commit db4e35d3d5cbd5bfac61cfb8edadd47c4608a864
parent 6b508a0e073fd4e29540f8808d919ff72c1893fb
Author: Laslo Hunhold <dev@frign.de>
Date:   Thu, 23 Jul 2020 18:16:08 +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 <ericshane@eradman.com>, who kindly reported the latter bug
to me.

Signed-off-by: Laslo Hunhold <dev@frign.de>

Diffstat:
M
http.c
 | 
109
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------

1 file changed, 84 insertions(+), 25 deletions(-)
diff --git 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 */