http: return 416 errors in more cases for bad Ranges

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.

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"