[PATCH] http: return 416 errors in more cases for bad Ranges

[PATCH] http: return 416 errors in more cases for bad Ranges

[Eric Wong]

For completely unparseable Range: headers, we'll ignore them
entirely as nginx does.  However, if /bytes=/ is matched, we'll
start returning 416 errors instead of 400.
---
 cmogstored.h       |  3 ++-
 http_get.c         |  2 ++
 http_parser.rl     | 56 ++++++++++++++++++++++++++++++++++++------------------
 test/http_range.rb |  8 ++++++--
 4 files changed, 47 insertions(+), 22 deletions(-)

diff --git a/cmogstored.h b/cmogstored.h
index a7309b5..5aa7c01 100644
--- a/cmogstored.h
+++ b/cmogstored.h
@@ -204,9 +204,10 @@ struct mog_http {
 		unsigned has_md5:1;
 		unsigned has_content_range:1; /* for PUT */
 		unsigned has_range:1;         /* for GET */
+		unsigned bad_range:1;
 		unsigned skip_rbuf_defer:1;
 		enum mog_chunk_state chunk_state:2;
-		unsigned unused_padding:3;
+		unsigned unused_padding:2;
 		uint8_t path_tip;
 		uint8_t path_end;
 		uint16_t line_end;
diff --git a/http_get.c b/http_get.c
index bb721f1..8025093 100644
--- a/http_get.c
+++ b/http_get.c
@@ -136,6 +136,8 @@ static off_t http_get_resp_hdr(struct mog_fd *mfd, struct stat *sb)
 			offset, offset + count - 1, /* bytes M-N */
 			(long long)sb->st_size,
 			http->_p.persistent ? "keep-alive" : "close");
+	} else if (http->_p.bad_range) {
+		goto bad_range;
 	} else {
 resp_200:
 		count = (long long)sb->st_size;
diff --git a/http_parser.rl b/http_parser.rl
index 8e82828..5683b7d 100644
--- a/http_parser.rl
+++ b/http_parser.rl
@@ -21,6 +21,18 @@ static bool length_incr(off_t *len, unsigned c)
 	return false;
 }
 
+static char *skip_header(struct mog_http *http, char *buf, const char *pe)
+{
+	char *p;
+
+	assert(http->_p.line_end > 0 && "no previous request/header line");
+	assert(buf[http->_p.line_end] == '\n' && "bad http->_p.line_end");
+	p = buf + http->_p.line_end + 1;
+	assert(p <= pe && "overflow");
+
+	return p;
+}
+
 %%{
 	machine http_parser;
 	include http_common "http_common.rl";
@@ -69,20 +81,31 @@ static bool length_incr(off_t *len, unsigned c)
 			"bytes=" > {
 				http->_p.range_beg = http->_p.range_end = -1;
 			}
-			(digit*) $ {
-				if (http->_p.range_beg < 0)
-					http->_p.range_beg = 0;
-				if (!length_incr(&http->_p.range_beg, fc))
-					fbreak;
-			}
-			'-'
-			(digit*) $ {
-				if (http->_p.range_end < 0)
-					http->_p.range_end = 0;
-				if (!length_incr(&http->_p.range_end, fc))
-					fbreak;
+			(
+				(digit*) $ {
+					if (http->_p.range_beg < 0)
+						http->_p.range_beg = 0;
+					if (!length_incr(&http->_p.range_beg,
+							 fc))
+						fbreak;
+				}
+				'-'
+				(digit*) $ {
+					if (http->_p.range_end < 0)
+						http->_p.range_end = 0;
+					if (!length_incr(&http->_p.range_end,
+							 fc))
+						fbreak;
+				}
+			) $! {
+				http->_p.bad_range = 1;
+				p = skip_header(http, buf, pe);
+				fgoto ignored_header;
 			}
-                ) $! { errno = EINVAL; fbreak; }
+		) $! {
+			p = skip_header(http, buf, pe);
+			fgoto ignored_header;
+		}
 		eor @ { http->_p.has_range = 1; };
 	transfer_encoding_chunked = "Transfer-Encoding:"i sep
 		"chunked"i eor > { http->_p.chunked = 1; };
@@ -102,12 +125,7 @@ static bool length_incr(off_t *len, unsigned c)
 		  content_md5 |
 		  connection ) $!
 		{
-			assert(http->_p.line_end > 0 &&
-			       "no previous request/header line");
-			assert(buf[http->_p.line_end] == '\n' &&
-			       "bad http->_p.line_end");
-			p = buf + http->_p.line_end + 1;
-			assert(p <= pe && "overflow");
+			p = skip_header(http, buf, pe);
 			fgoto ignored_header;
 		};
 	headers = header_line* '\r''\n' > { really_done = 1; fbreak; };
diff --git a/test/http_range.rb b/test/http_range.rb
index 982d767..e19e6c5 100644
--- a/test/http_range.rb
+++ b/test/http_range.rb
@@ -161,10 +161,14 @@ class TestHTTPRange < Test::Unit::TestCase
             assert_equal nil, cm_r.body if Net::HTTP::Head === meth
           end
 
-          %w(-5-5 4qasdlkj 323).each do |bogus|
+          %w(-5-5 4qasdlkj 323 0--1).each do |bogus|
             r["Range"] = "bytes=#{bogus}"
             cm_r = cm.request(r)
-            assert_equal 400, cm_r.code.to_i, "bogus=#{bogus}"
+            assert_equal 416, cm_r.code.to_i, "bogus=#{bogus}"
+
+            r["Range"] = bogus
+            cm_r = cm.request(r)
+            assert_equal 200, cm_r.code.to_i, "no bytes=, full response"
           end
 
           r["Range"] = "bytes=5-6666"
-- 
EW