Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
unreleased
==========

1.7.0 / 2017-01-04
==================
* Callbacks available in `write` and `end` when supported in nodejs (requires nodejs >= 0.12.x)
* deps: [email protected]
* deps: compressible@~2.0.9
- Fix regex fallback to not override `compressible: false` in db
Expand Down
16 changes: 9 additions & 7 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ var debug = require('debug')('compression')
var onHeaders = require('on-headers')
var vary = require('vary')
var zlib = require('zlib')
var http = require('http')

/**
* Module exports.
Expand All @@ -35,6 +36,7 @@ module.exports.filter = shouldCompress
*/

var cacheControlNoTransformRegExp = /(?:^|,)\s*?no-transform\s*?(?:,|$)/
var nodeHasCallbacks = (http.OutgoingMessage.prototype.write.length === 3 && http.OutgoingMessage.prototype.end.length === 3)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @whitingj . Looking at the changes, I feel like someone else did the exact same change and it is problematic. I found my response to that, if you can take it into consideration: #80 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And that comment was in response to the following change: 0f617c7 which is basically identical to your change here.


/**
* Compress response data with gzip / deflate.
Expand Down Expand Up @@ -74,7 +76,7 @@ function compression (options) {

// proxy

res.write = function write (chunk, encoding) {
res.write = function write (chunk, encoding, cb) {
if (ended) {
return false
}
Expand All @@ -84,11 +86,11 @@ function compression (options) {
}

return stream
? stream.write(new Buffer(chunk, encoding))
: _write.call(this, chunk, encoding)
? stream.write(new Buffer(chunk, encoding), nodeHasCallbacks ? cb : undefined)
: _write.call(this, chunk, encoding, nodeHasCallbacks ? cb : undefined)
}

res.end = function end (chunk, encoding) {
res.end = function end (chunk, encoding, cb) {
if (ended) {
return false
}
Expand All @@ -103,16 +105,16 @@ function compression (options) {
}

if (!stream) {
return _end.call(this, chunk, encoding)
return _end.call(this, chunk, encoding, nodeHasCallbacks ? cb : undefined)
}

// mark ended
ended = true

// write Buffer for Node.js 0.8
return chunk
? stream.end(new Buffer(chunk, encoding))
: stream.end()
? stream.end(new Buffer(chunk, encoding), nodeHasCallbacks ? cb : undefined)
: stream.end(null, null, nodeHasCallbacks ? cb : undefined)
}

res.on = function on (type, listener) {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "compression",
"description": "Node.js compression middleware",
"version": "1.6.2",
"version": "1.7.0",
"contributors": [
"Douglas Christopher Wilson <[email protected]>",
"Jonathan Ong <[email protected]> (http://jongleberry.com)"
Expand Down
75 changes: 75 additions & 0 deletions test/compression.js
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,81 @@ describe('compression()', function () {
.end()
})
})

describe('when callbacks are used', function () {
it('should call the passed callbacks in the order passed when compressing', function (done) {
var hasCallbacks = false
var callbackOutput = []
var server = createServer(null, function (req, res) {
hasCallbacks = (http.OutgoingMessage.prototype.write.length === 3 && http.OutgoingMessage.prototype.end.length === 3)
res.setHeader('Content-Type', 'text/plain')
res.write('Hello', null, function () {
callbackOutput.push(0)
})
res.write(' World', null, function () {
callbackOutput.push(1)
})
res.end(null, null, function () {
callbackOutput.push(2)
})
})

request(server)
.get('/')
.set('Accept-Encoding', 'gzip')
.expect('Content-Encoding', 'gzip')
.end(function (err) {
if (err) {
throw new Error(err)
}
if (hasCallbacks) {
assert.equal(callbackOutput.length, 3)
assert.deepEqual(callbackOutput, [0, 1, 2])
} else {
// nodejs 0.10 zlib has callbacks but OutgoingMessage doesn't, assert that no callbacks were made
assert.equal(callbackOutput.length, 0)
}
done()
})
})

it('should call the passed callbacks in the order passed when not compressing', function (done) {
var hasCallbacks = false
var callbackOutput = []
var server = createServer(null, function (req, res) {
hasCallbacks = (http.OutgoingMessage.prototype.write.length === 3 && http.OutgoingMessage.prototype.end.length === 3)
res.setHeader('Cache-Control', 'no-transform')
res.setHeader('Content-Type', 'text/plain')
res.write('hello,', null, function () {
callbackOutput.push(0)
})
res.write(' world', null, function () {
callbackOutput.push(1)
})
res.end(null, null, function () {
callbackOutput.push(2)
})
})

request(server)
.get('/')
.set('Accept-Encoding', 'gzip')
.expect('Cache-Control', 'no-transform')
.expect(shouldNotHaveHeader('Content-Encoding'))
.end(function (err) {
if (err) {
throw new Error(err)
}
if (hasCallbacks) {
assert.equal(callbackOutput.length, 3)
assert.deepEqual(callbackOutput, [0, 1, 2])
} else {
assert.equal(callbackOutput.length, 0)
}
done()
})
})
})
})

function createServer (opts, fn) {
Expand Down