Skip to content

Commit

Permalink
Merge pull request #47 from isaacs/gzip-fix
Browse files Browse the repository at this point in the history 
fix bad pipe for no-gzip accepted
  • Loading branch information
@rvagg
rvagg committed Jun 18, 2014
2 parents 07c2868 + 4a4cc46 commit b750f90
Show file tree
Hide file tree
Showing 7 changed files with 245 additions and 87 deletions.
12 changes: 7 additions & 5 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,19 @@
"main": "st.js",
"bin": "bin/server.js",
"dependencies": {
"mime": "~1.2.7",
"negotiator": "~0.2.5",
"async-cache": "~0.1.2",
"fd": "~0.0.2"
"bl": "~0.8.0",
"fd": "~0.0.2",
"mime": "~1.2.7",
"negotiator": "~0.2.5"
},
"optionalDependencies": {
"graceful-fs": "~1.2"
},
"devDependencies": {
"tap": "~0.3.1",
"request": "~2.11.4"
"request": "~2.11.4",
"rimraf": "~2.2.8",
"tap": "~0.3.1"
},
"scripts": {
"test": "tap test/*.js"
Expand Down
74 changes: 42 additions & 32 deletions st.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ var http = require('http')
var AC = require('async-cache')
var util = require('util')
var FD = require('fd')
var bl = require('bl')

// default caching options
var defaultCacheOptions = {
Expand Down Expand Up @@ -200,7 +201,8 @@ Mount.prototype.getPath = function (u) {
// /a/b/c/d --> /path/to/z/d/x/d
u = u.substr(this.url.length)
if (u.charAt(0) !== '/') u = '/' + u
var p = path.join(this.path, u)

p = path.join(this.path, u)
return p
}

Expand Down Expand Up @@ -293,7 +295,7 @@ Mount.prototype.serve = function (req, res, next) {
}

// only set headers once we're sure we'll be serving this request
if (!res.getHeader('cache-control'))
if (!res.getHeader('cache-control') && this._cacheControl)
res.setHeader('cache-control', this._cacheControl)
res.setHeader('last-modified', stat.mtime.toUTCString())
res.setHeader('etag', etag)
Expand Down Expand Up @@ -372,14 +374,13 @@ Mount.prototype.file = function (p, fd, stat, etag, req, res, end) {

Mount.prototype.cachedFile = function (p, stat, etag, req, res) {
var key = stat.size + ':' + etag

if (this.opt.gzip !== false) {
var gz = getGz(p, req)
}
var gz = this.opt.gzip !== false && getGz(p, req)

this.cache.content.get(key, function (er, content) {
if (er) return this.error(er, res)
res.statusCode = 200
if (this.opt.cachedHeader)
res.setHeader('x-from-cache', 'true')
if (gz && content.gz) {
res.setHeader('content-encoding', 'gzip')
res.setHeader('content-length', content.gz.length)
Expand All @@ -396,6 +397,15 @@ Mount.prototype.streamFile = function (p, fd, stat, etag, req, res, end) {
var stream = fs.createReadStream(p, streamOpt)
stream.destroy = function () {}

// gzip only if not explicitly turned off or client doesn't accept it
var gzOpt = this.opt.gzip !== false
var gz = gzOpt && getGz(p, req)
var cachable = this.cache.content._cache.max > stat.size
var gzstr

// need a gzipped version for the cache, so do it regardless of what the client wants
if (gz || (gzOpt && cachable)) gzstr = zlib.Gzip()

// too late to effectively handle any errors.
// just kill the connection if that happens.
stream.on('error', function(e) {
Expand All @@ -404,49 +414,49 @@ Mount.prototype.streamFile = function (p, fd, stat, etag, req, res, end) {
end()
})

if (res.filter) {
stream = stream.pipe(res.filter)
}

if (this.opt.gzip !== false) {
var gzstr = zlib.Gzip()
var gz = getGz(p, req)
stream.pipe(gzstr)
}
if (res.filter) stream = stream.pipe(res.filter)

res.statusCode = 200

if (gz) {
// we don't know how long it'll be, since it will be compressed.
res.setHeader('content-encoding', 'gzip')
gzstr.pipe(res)
stream.pipe(gzstr).pipe(res)
} else {
if (!res.filter) res.setHeader('content-length', stat.size)
stream.pipe(res)
if (gzstr)
stream.pipe(gzstr) // for cache
}

stream.on('end', function () {
process.nextTick(end)
})

if (this.cache.content._cache.max > stat.size) {
if (cachable) {
// collect it, and put it in the cache
var key = fd + ':' + stat.size + ':' + etag
var bufs = []
stream.on('data', function (c) {
bufs.push(c)
})

if (gzstr) {
var gzbufs = []
gzstr.on('data', function (c) {
gzbufs.push(c)
})
gzstr.on('end', function () {
var content = Buffer.concat(bufs)
content.gz = Buffer.concat(gzbufs)
var calls = 0

// called by bl() for both the raw stream and gzipped stream if we're
// caching gzipped data
var collectEnd = function () {
if (++calls == (gzOpt ? 2 : 1)) {
var content = bufs.slice()
content.gz = gzbufs && gzbufs.slice()
this.cache.content.set(key, content)
}.bind(this))
}
}.bind(this)

var key = stat.size + ':' + etag
var bufs = bl(collectEnd)
var gzbufs

stream.pipe(bufs)

if (gzstr) {
gzbufs = bl(collectEnd)
gzstr.pipe(gzbufs)
}
}
}
Expand Down Expand Up @@ -555,7 +565,7 @@ Mount.prototype._loadStat = function (key, cb) {
}
}

Mount.prototype._loadContent = function (key, cb) {
Mount.prototype._loadContent = function () {
// this function should never be called.
// we check if the thing is in the cache, and if not, stream it in
// manually. this.cache.content.get() should not ever happen.
Expand Down
71 changes: 21 additions & 50 deletions test/basic.js
View file
Original file line number Diff line number Diff line change
@@ -1,53 +1,18 @@
var path = require('path')
var fs = require('fs')
var zlib = require('zlib')
var http = require('http')
var server
var st = require('../st.js')
var request = require('request')
var tap = require('tap')
var test = tap.test
var port = process.env.PORT || 1337
var util = require('util')

var opts = util._extend({
autoindex: true,
path: path.dirname(__dirname),
url: '/test'
}, global.options || {})

var stExpect = fs.readFileSync(require.resolve('../st.js')).toString()

var mount = st(opts)
exports.mount = mount
exports.req = req
exports.stExpect = stExpect

function req (url, headers, cb) {
if (typeof headers === 'function') cb = headers, headers = {}
request({ encoding: null,
url: 'http://localhost:' + port + url,
headers: headers }, cb)
}
var test = require('tap').test
var common = require('./common')

test('setup', function (t) {
server = http.createServer(function (req, res) {
if (!mount(req, res)) {
res.statusCode = 404
return res.end('Not a match: ' + req.url)
}
}).listen(port, function () {
t.pass('listening')
t.end()
})
})
var req = common.req
var stExpect = common.stExpect
var opts = common.opts

tap.tearDown(function() {
server.close()
})
var stEtag

module.exports.mount = common.mount
module.exports.req = common.req
module.exports.stExpect = common.stExpect


var stEtag
test('simple request', function (t) {
req('/test/st.js', function (er, res, body) {
t.equal(res.statusCode, 200)
Expand All @@ -59,6 +24,7 @@ test('simple request', function (t) {
})
})


test('304 request', function (t) {
req('/test/st.js', {'if-none-match':stEtag}, function (er, res, body) {
t.equal(res.statusCode, 304)
Expand All @@ -67,6 +33,7 @@ test('304 request', function (t) {
})
})


if (opts.gzip !== false) {
test('gzip', function (t) {
req('/test/st.js', {'accept-encoding':'gzip'},
Expand All @@ -82,6 +49,7 @@ if (opts.gzip !== false) {
})
}


test('multiball!', function (t) {
var n = 6
req('/test/st.js', then)
Expand All @@ -91,7 +59,7 @@ test('multiball!', function (t) {
req('/test/favicon.ico', then)
req('/test/bin/server.js', then)

function then (er, res, body) {
function then (er, res) {
if (er)
throw er
t.equal(res.statusCode, 200)
Expand All @@ -109,7 +77,7 @@ test('multiball!', function (t) {
}, 200)
}

function then2 (er, res, body) {
function then2 (er, res) {
if (er)
throw er

Expand All @@ -118,7 +86,7 @@ test('multiball!', function (t) {
if (opts.cache === false)
t.equal(res.headers['cache-control'], 'no-cache')
else if (opts.cache && opts.cache.content && opts.cache.content.maxAge === false)
t.notOk(res.headers['cache-control'])
t.ok(res.headers['cache-control'] === undefined)
else if (opts.cache && opts.cache.content && opts.cache.content.cacheControl)
t.equal(res.headers['cache-control'], opts.cache.content.cacheControl)
else
Expand All @@ -129,6 +97,7 @@ test('multiball!', function (t) {
}
})


test('space in filename', function (t) {
req('/test/test/fixtures/space in filename.txt', function (er, res, body) {
t.equal(res.statusCode, 200)
Expand All @@ -137,15 +106,17 @@ test('space in filename', function (t) {
})
})


test('malformed URL', function (t) {
req('/test%2E%git', function (er, res, body) {
req('/test%2E%git', function (er, res) {
t.equal(res.statusCode, 404)
t.end()
})
})


test('shenanigans', function(t) {
req('/%2e%2E/%2e%2E/%2e%2E/%2e%2E/%2e%2E/%2e%2E/etc/passwd', function(er, res, body) {
req('/%2e%2E/%2e%2E/%2e%2E/%2e%2E/%2e%2E/%2e%2E/etc/passwd', function(er, res) {
if (er)
throw er
t.equal(res.statusCode, 403)
Expand Down
54 changes: 54 additions & 0 deletions test/common.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
var path = require('path')
var fs = require('fs')
var http = require('http')
var util = require('util')
var request = require('request')
var tap = require('tap')

var st = require('../st.js')

var port = process.env.PORT || 1337
var test = tap.test

var server

var opts = util._extend({
autoindex: true,
path: path.dirname(__dirname),
url: '/test'
}, global.options || {})

var stExpect = fs.readFileSync(require.resolve('../st.js'), 'utf8')
var mount = st(opts)


function req (url, headers, cb) {
if (typeof headers === 'function') cb = headers, headers = {}
request({ encoding: null,
url: 'http://localhost:' + port + url,
headers: headers }, cb)
}


test('setup', function (t) {
server = http.createServer(function (req, res) {
if (!mount(req, res)) {
res.statusCode = 404
return res.end('Not a match: ' + req.url)
}
}).listen(port, function () {
t.pass('listening')
t.end()
})
})


tap.tearDown(function() {
server.close()
})


module.exports.mount = mount
module.exports.req = req
module.exports.stExpect = stExpect
module.exports.opts = opts
Loading

0 comments on commit b750f90

Please sign in to comment.