Skip to content

Commit

Permalink
fix(middleware): ensure Range headers adhere more closely to RFC 2616
Browse files Browse the repository at this point in the history
Added one new dependency, use `range-parser` (Express dependency) to compute range.
It is more well-tested in the wild.

Fixes #2310
  • Loading branch information
kahwee committed Aug 16, 2016
1 parent 322f3b3 commit 8b1b4b1
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 34 deletions.
59 changes: 27 additions & 32 deletions lib/middleware/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

var mime = require('mime')
var _ = require('lodash')
var parseRange = require('range-parser')

var log = require('../logger').create('web-server')

Expand Down Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions test/unit/middleware/source_files.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})

Expand Down

0 comments on commit 8b1b4b1

Please sign in to comment.