diff --git a/lib/redirect.js b/lib/redirect.js index 7dd6c254c..ce83ffec8 100644 --- a/lib/redirect.js +++ b/lib/redirect.js @@ -87,7 +87,8 @@ Redirect.prototype.onResponse = function (response) { // ignore any potential response body. it cannot possibly be useful // to us at this point. - if (request._paused) { + // response.resume should be defined, but check anyway before calling. Workaround for browserify. + if (response.resume) { response.resume() } diff --git a/request.js b/request.js index f8c33f49f..88cf6dd5d 100644 --- a/request.js +++ b/request.js @@ -1047,12 +1047,6 @@ Request.prototype.onRequestResponse = function (response) { response.resume() return } - if (self._paused) { - response.pause() - } else if (response.resume) { - // response.resume should be defined, but check anyway before calling. Workaround for browserify. - response.resume() - } self.response = response response.request = self @@ -1153,6 +1147,10 @@ Request.prototype.onRequestResponse = function (response) { } } + if (self._paused) { + responseContent.pause() + } + self.responseContent = responseContent self.emit('response', response) diff --git a/tests/test-gzip.js b/tests/test-gzip.js index 5c2b3c1ae..35949dd1c 100644 --- a/tests/test-gzip.js +++ b/tests/test-gzip.js @@ -7,6 +7,8 @@ var request = require('../index') , tape = require('tape') var testContent = 'Compressible response content.\n' + , testContentBig + , testContentBigGzip , testContentGzip var server = http.createServer(function(req, res) { @@ -18,6 +20,10 @@ var server = http.createServer(function(req, res) { if (req.url === '/error') { // send plaintext instead of gzip (should cause an error for the client) res.end(testContent) + } else if (req.url === '/chunks') { + res.writeHead(200) + res.write(testContentBigGzip.slice(0, 4096)) + setTimeout(function() { res.end(testContentBigGzip.slice(4096)) }, 10) } else { zlib.gzip(testContent, function(err, data) { assert.equal(err, null) @@ -30,11 +36,30 @@ var server = http.createServer(function(req, res) { }) tape('setup', function(t) { + // Need big compressed content to be large enough to chunk into gzip blocks. + // Want it to be deterministic to ensure test is reliable. + // Generate pseudo-random printable ASCII characters using MINSTD + var a = 48271 + , m = 0x7FFFFFFF + , x = 1 + testContentBig = new Buffer(10240) + for (var i = 0; i < testContentBig.length; ++i) { + x = (a * x) & m + // Printable ASCII range from 32-126, inclusive + testContentBig[i] = (x % 95) + 32 + } + zlib.gzip(testContent, function(err, data) { t.equal(err, null) testContentGzip = data - server.listen(6767, function() { - t.end() + + zlib.gzip(testContentBig, function(err, data2) { + t.equal(err, null) + testContentBigGzip = data2 + + server.listen(6767, function() { + t.end() + }) }) }) }) @@ -139,6 +164,50 @@ tape('transparently supports gzip error to pipes', function(t) { }) }) +tape('pause when streaming from a gzip request object', function(t) { + var chunks = [] + var paused = false + var options = { url: 'http://localhost:6767/chunks', gzip: true } + request.get(options) + .on('data', function(chunk) { + var self = this + + t.notOk(paused, 'Only receive data when not paused') + + chunks.push(chunk) + if (chunks.length === 1) { + self.pause() + paused = true + setTimeout(function() { + paused = false + self.resume() + }, 100) + } + }) + .on('end', function() { + t.ok(chunks.length > 1, 'Received multiple chunks') + t.ok(Buffer.concat(chunks).equals(testContentBig), 'Expected content') + t.end() + }) +}) + +tape('pause before streaming from a gzip request object', function(t) { + var paused = true + var options = { url: 'http://localhost:6767/foo', gzip: true } + var r = request.get(options) + r.pause() + r.on('data', function(data) { + t.notOk(paused, 'Only receive data when not paused') + t.equal(data.toString(), testContent) + }) + r.on('end', t.end.bind(t)) + + setTimeout(function() { + paused = false + r.resume() + }, 100) +}) + tape('cleanup', function(t) { server.close(function() { t.end() diff --git a/tests/test-pipes.js b/tests/test-pipes.js index 0e4be002c..3763f5781 100644 --- a/tests/test-pipes.js +++ b/tests/test-pipes.js @@ -131,6 +131,66 @@ tape('piping from a request object', function(t) { } }) +tape('pause when piping from a request object', function(t) { + s.once('/chunks', function(req, res) { + res.writeHead(200, { + 'content-type': 'text/plain' + }) + res.write('Chunk 1') + setTimeout(function() { res.end('Chunk 2') }, 10) + }) + + var chunkNum = 0 + var paused = false + request({ + url: s.url + '/chunks' + }) + .on('data', function(chunk) { + var self = this + + t.notOk(paused, 'Only receive data when not paused') + + ++chunkNum + if (chunkNum === 1) { + t.equal(chunk.toString(), 'Chunk 1') + self.pause() + paused = true + setTimeout(function() { + paused = false + self.resume() + }, 100) + } else { + t.equal(chunk.toString(), 'Chunk 2') + } + }) + .on('end', t.end.bind(t)) +}) + +tape('pause before piping from a request object', function(t) { + s.once('/pause-before', function(req, res) { + res.writeHead(200, { + 'content-type': 'text/plain' + }) + res.end('Data') + }) + + var paused = true + var r = request({ + url: s.url + '/pause-before' + }) + r.pause() + r.on('data', function(data) { + t.notOk(paused, 'Only receive data when not paused') + t.equal(data.toString(), 'Data') + }) + r.on('end', t.end.bind(t)) + + setTimeout(function() { + paused = false + r.resume() + }, 100) +}) + var fileContents = fs.readFileSync(__filename).toString() function testPipeFromFile(testName, hasContentLength) { tape(testName, function(t) {