fix(middleware): ensure Range headers adhere more closely to RFC 2616

kahwee · kahwee · commit 8b1b4b105292 · 2016-08-15T20:46:57.000-07:00
Added one new dependency, use `range-parser` (Express dependency) to compute range. It is more well-tested in the wild. Fixes #2310
diff --git a/lib/middleware/common.js b/lib/middleware/common.js
@@ -4,6 +4,7 @@
 
 var mime = require('mime')
 var _ = require('lodash')
+var parseRange = require('range-parser')
 
 var log = require('../logger').create('web-server')
 
@@ -33,41 +34,35 @@ var createServeFile = function (fs, directory, config) {
     var responseData
 
     var convertForRangeRequest = function () {
-      // If the header is invalid, ignore
-      if (!rangeHeader.startsWith('bytes=')) {
+      var range = parseRange(responseData.length, rangeHeader)
+      if (range === -2) {
+        // malformed header string
         return 200
-      }
-
-      responseData = new Buffer(responseData)
-
-      var ranges = rangeHeader.substr(6)
-      if (ranges.indexOf(',') >= 0) {
-        // Multiple ranges are not supported.
+      } else if (range === -1) {
+        // unsatisfiable range
         responseData = new Buffer(0)
-        return 416  // Requested range not satisfiable
-      }
-      var parts = /^([0-9]*)-([0-9]*)$/.exec(ranges)
-      if (!parts || (!parts[1] && !parts[2])) {
-        return 200
-      }
-      var start, end
-      if (parts[1]) {
-        start = Number(parts[1])
-        end = parts[2] ? Number(parts[2]) : responseData.length
-      } else {
-        end = responseData.length
-        start = responseData.length - Number(parts[2])
-      }
-      if (end <= start) {
-        responseData = new Buffer(0)
-        return 416  // Requested range not satisfiable
+        return 416
+      } else if (range.type === 'bytes') {
+        responseData = new Buffer(responseData)
+        if (range.length === 1) {
+          var start = range[0].start
+          var end = range[0].end
+          response.setHeader(
+            'Content-Range',
+            'bytes ' + start + '-' + end + '/' + responseData.length
+          )
+          response.setHeader('Accept-Ranges', 'bytes')
+          response.setHeader('Content-Length', end - start + 1)
+          responseData = responseData.slice(start, end + 1)
+          return 206
+        } else {
+          // Multiple ranges are not supported. Maybe future?
+          responseData = new Buffer(0)
+          return 416
+        }
       }
-
-      response.setHeader(
-          'Content-Range',
-          'bytes ' + start + '-' + end + '/' + responseData.length)
-      responseData = responseData.slice(start, end + 1)
-      return 206
+      // All other states, ignore
+      return 200
     }
 
     if (directory) {
diff --git a/package.json b/package.json
@@ -314,6 +314,7 @@
     "minimatch": "^3.0.0",
     "optimist": "^0.6.1",
     "qjobs": "^1.1.4",
+    "range-parser": "^1.2.0",
     "rimraf": "^2.3.3",
     "socket.io": "1.4.7",
     "source-map": "^0.5.3",
diff --git a/test/unit/middleware/source_files.spec.js b/test/unit/middleware/source_files.spec.js
@@ -81,15 +81,15 @@ describe('middleware.source_files', function () {
       return request(server)
         .get('/absolute/src/some.js')
         .set('Range', 'bytes=3-')
-        .expect('Content-Range', 'bytes 3-9/9')
+        .expect('Content-Range', 'bytes 3-8/9')
         .expect(206, 'source')
     })
 
     it('allows single range with suffix', function () {
       return request(server)
         .get('/absolute/src/some.js')
         .set('Range', 'bytes=-5')
-        .expect('Content-Range', 'bytes 4-9/9')
+        .expect('Content-Range', 'bytes 4-8/9')
         .expect(206, 'ource')
     })
 
