Merge pull request #444 from cristiklein/fix-redirect-with-head-with-body

Fix redirect with head with body

Author: tomas

.editorconfig

@@ -0,0 +1,7 @@
+root = true
+
+[*]
+charset = utf-8
+end_of_line = lf
+indent_style = space
+indent_size = 2

lib/needle.js

@@ -455,20 +455,25 @@ Needle.prototype.send_request = function(count, method, uri, config, post_data,
       protocol     = request_opts.protocol == 'https:' ? https : http,
       signal       = request_opts.signal;
 
-  function done(err, resp) {
-    if (returned++ > 0)
-      return debug('Already finished, stopping here.');
-
-    if (timer) clearTimeout(timer);
+  function unlisten_errors() {
     request.removeListener('error', had_error);
-    out.done = true;
 
     // An error can still be fired after closing.  In particular, on macOS.
     // See also:
     //  - https://github.com/tomas/needle/issues/391
     //  - https://github.com/less/less.js/issues/3693
     //  - https://github.com/nodejs/node/issues/27916
     request.once('error', function() {});
+  }
+
+  function done(err, resp) {
+    if (returned++ > 0)
+      return debug('Already finished, stopping here.');
+
+    if (timer) clearTimeout(timer);
+    out.done = true;
+
+    unlisten_errors();
 
     if (callback)
       return callback(err, resp, resp ? resp.body : undefined);
@@ -559,6 +564,7 @@ Needle.prototype.send_request = function(count, method, uri, config, post_data,
 
         var redirect_url = utils.resolve_url(headers.location, uri);
         debug('Redirecting to ' +  redirect_url.toString());
+        unlisten_errors();
         return self.send_request(++count, method, redirect_url.toString(), config, post_data, out, callback);
       } else if (config.follow_max > 0) {
         return done(new Error('Max redirects reached. Possible loop in: ' + headers.location));

test/redirect_with_bad_redirector.js

@@ -0,0 +1,65 @@
+var helpers = require('./helpers'),
+    should  = require('should'),
+    sinon   = require('sinon'),
+    http    = require('http'),
+    needle  = require('./../');
+
+const port = 1234;
+
+describe('redirects with bad redirector', function() {
+
+  var spies   = {},
+      servers = {};
+
+  before(function(done) {
+    servers.http = http.createServer(function(req, res) {
+      if (req.url == '/foo/bar') {
+        res.end('Redirected successfully!');
+        return;
+      }
+
+      /* Don't judge me. I found at least one server on the
+      * Internet doing this and it triggered a bug. */
+      const body = 'Let me send you a body, although you only asked for a HEAD.';
+
+      const headers =
+        'HTTP/1.1 302 Found\r\n' +
+        'Connection: close\r\n' +
+        'Location: /foo/bar\r\n' +
+        `Content-Length: ${Buffer.byteLength(body)}\r\n` +
+        '\r\n';
+
+      res.socket.write(headers + body);
+      res.socket.destroy();
+    }).listen(port, done);
+  })
+
+  after(function(done) {
+    servers.http.close(done);
+  })
+
+  it('calls back exactly once', function (done) {
+    const opts = {
+      follow: 5,
+    }
+
+    const url = `http://localhost:${port}`
+    needle.head(url, opts, function (err, resp, body) {
+      //should(body && body.toString()).eql('Redirected successfully!');
+      done();
+    });
+  });
+
+  it('calls back exactly once with follow_keep_method', function (done) {
+    const opts = {
+      follow: 5,
+      follow_keep_method: true,
+    }
+
+    const url = `http://localhost:${port}`
+    needle.head(url, opts, function (err, resp, body) {
+      //should(body && body.toString()).eql('Redirected successfully!');
+      done();
+    });
+  });
+});

test/utils/bad-redirector.js

@@ -0,0 +1,34 @@
+var http = require('http'),
+    https = require('https'),
+    url = require('url');
+    
+var port = 1234,
+    log  = true,
+    request_auth = false;
+
+http.createServer(function(req, res) {
+
+  console.log(req.headers);
+  console.log("Got request: " + req.method + " " + req.url);
+
+  /* Don't judge me. I found at least one server on the
+  * Internet doing this and it triggered a bug. */
+  const body = 'Let me send you a body, although you only asked for a HEAD.';
+
+  const headers =
+    'HTTP/1.1 302 Found\r\n' +
+    'Connection: close\r\n' +
+    'Location: /foo/bar\r\n' +
+    `Content-Length: ${Buffer.byteLength(body)}\r\n` +
+    '\r\n';
+
+  res.socket.write(headers + body);
+  res.socket.destroy();
+}).listen(port);
+
+process.on('uncaughtException', function(err){
+  console.log('Uncaught exception!');
+  console.log(err);
+});
+
+console.log("Bad redirector listening on port " + port);