Prevent unmodified msg.headers from breaking HTTP Request flows

Closed #1015

nodes/core/io/21-httpin.js

@@ -26,6 +26,7 @@ module.exports = function(RED) {
     var onHeaders = require('on-headers');
     var typer = require('media-typer');
     var isUtf8 = require('is-utf8');
+    var hashSum = require("hash-sum");
 
     function rawBodyParser(req, res, next) {
         if (req.skipRawBodyParser) { next(); } // don't parse this if told to skip
@@ -276,6 +277,15 @@ module.exports = function(RED) {
         this.on("input",function(msg) {
             if (msg.res) {
                 var headers = RED.util.cloneMessage(node.headers);
+                if (msg.headers) {
+                    if (msg.headers.hasOwnProperty('x-node-red-request-node')) {
+                        var headerHash = msg.headers['x-node-red-request-node'];
+                        delete msg.headers['x-node-red-request-node'];
+                        var hash = hashSum(msg.headers);
+                        if (hash === headerHash) {
+                            delete msg.headers;
+                        }
+                    }
                     if (msg.headers) {
                         for (var h in msg.headers) {
                             if (msg.headers.hasOwnProperty(h) && !headers.hasOwnProperty(h)) {
@@ -283,6 +293,7 @@ module.exports = function(RED) {
                             }
                         }
                     }
+                }
                 if (Object.keys(headers).length > 0) {
                     msg.res._res.set(headers);
                 }

nodes/core/io/21-httprequest.html

@@ -108,6 +108,12 @@
     <code>example.com/{{{topic}}}</code>, it will have the value of <code>msg.topic</code> automatically inserted.
     Using {{{...}}} prevents mustache from escaping characters like / & etc.</p>
     <p><b>Note</b>: If running behind a proxy, the standard <code>http_proxy=...</code> environment variable should be set and Node-RED restarted.</p>
+    <h4>Using multiple HTTP Request nodes</h4>
+    <p>In order to use more than one of these nodes in the same flow, care must be taken with
+    the <code>msg.headers</code> property. The first node will set this property with
+    the response headers. The next node will then use those headers for its request - this
+    is not usually the right thing to do. The <code>msg.headers</code> property <b>must</b>
+    be deleted with a <b>change</b> node in order for the requests to work as expected.</p>
     <h4>Cookie handling</h4>
     <p>The <code>cookies</code> property passed to the node must be an object of name/value pairs.
     The value can be either a string to set the value of the cookie or it can be an

nodes/core/io/21-httprequest.js

@@ -22,6 +22,7 @@ module.exports = function(RED) {
     var mustache = require("mustache");
     var querystring = require("querystring");
     var cookie = require("cookie");
+    var hashSum = require("hash-sum");
 
     function HTTPRequest(n) {
         RED.nodes.createNode(this,n);
@@ -82,6 +83,15 @@ module.exports = function(RED) {
             opts.headers = {};
             var ctSet = "Content-Type"; // set default camel case
             var clSet = "Content-Length";
+            if (msg.headers) {
+                if (msg.headers.hasOwnProperty('x-node-red-request-node')) {
+                    var headerHash = msg.headers['x-node-red-request-node'];
+                    delete msg.headers['x-node-red-request-node'];
+                    var hash = hashSum(msg.headers);
+                    if (hash === headerHash) {
+                        delete msg.headers;
+                    }
+                }
                 if (msg.headers) {
                     for (var v in msg.headers) {
                         if (msg.headers.hasOwnProperty(v)) {
@@ -97,6 +107,7 @@ module.exports = function(RED) {
                         }
                     }
                 }
+            }
             if (msg.cookies) {
                 var cookies = [];
                 if (opts.headers.hasOwnProperty('cookie')) {
@@ -207,7 +218,7 @@ module.exports = function(RED) {
                     })
 
                 }
-
+                msg.headers['x-node-red-request-node'] = hashSum(msg.headers);
                 // msg.url = url;   // revert when warning above finally removed
                 res.on('data',function(chunk) {
                     if (!Buffer.isBuffer(chunk)) {

package.json

@@ -39,6 +39,7 @@
         "follow-redirects":"1.2.4",
         "fs-extra": "1.0.0",
         "fs.notify":"0.0.4",
+        "hash-sum":"1.0.2",
         "i18next":"1.10.6",
         "is-utf8":"0.2.1",
         "js-yaml": "3.8.4",
@@ -48,7 +49,7 @@
         "mqtt": "2.9.0",
         "multer": "1.3.0",
         "mustache": "2.3.0",
-        "nopt": "4.0.1",
+        "nopt": "3.0.6",
         "oauth2orize":"1.8.0",
         "on-headers":"1.0.1",
         "passport":"0.3.2",

test/nodes/core/io/21-httprequest_spec.js

@@ -21,6 +21,7 @@ var express = require("express");
 var bodyParser = require('body-parser');
 var helper = require("../../helper.js");
 var httpRequestNode = require("../../../../nodes/core/io/21-httprequest.js");
+var hashSum = require("hash-sum");
 
 describe('HTTP Request Node', function() {
     var testApp;
@@ -399,4 +400,46 @@ describe('HTTP Request Node', function() {
         });
     })
 
+    it('ignores unmodified msg.headers property', function(done) {
+        var flow = [{id:"n1",type:"http request",wires:[["n2"]],method:"POST",ret:"obj",url:getTestURL('/postInspect')},
+                    {id:"n2", type:"helper"}];
+        helper.load(httpRequestNode, flow, function() {
+            var n1 = helper.getNode("n1");
+            var n2 = helper.getNode("n2");
+            n2.on("input", function(msg) {
+                try {
+                    msg.payload.headers.should.have.property('content-type').which.startWith('application/json');
+                    msg.payload.headers.should.not.have.property('x-node-red-request-node');
+                    done();
+                } catch(err) {
+                    done(err);
+                }
+            });
+            // Pass in a headers property with an unmodified x-node-red-request-node hash
+            // This should cause the node to ignore the headers
+            n1.receive({payload:{foo:"bar"}, headers: { 'content-type': 'text/plain', "x-node-red-request-node":"67690139"}});
+        });
+    })
+
+    it('uses modified msg.headers property', function(done) {
+        var flow = [{id:"n1",type:"http request",wires:[["n2"]],method:"POST",ret:"obj",url:getTestURL('/postInspect')},
+                    {id:"n2", type:"helper"}];
+        helper.load(httpRequestNode, flow, function() {
+            var n1 = helper.getNode("n1");
+            var n2 = helper.getNode("n2");
+            n2.on("input", function(msg) {
+                try {
+                    msg.payload.headers.should.have.property('content-type').which.startWith('text/plain');
+                    msg.payload.headers.should.not.have.property('x-node-red-request-node');
+                    done();
+                } catch(err) {
+                    done(err);
+                }
+            });
+            // Pass in a headers property with a x-node-red-request-node hash that doesn't match the contents
+            // This should cause the node to use the headers
+            n1.receive({payload:{foo:"bar"}, headers: { 'content-type': 'text/plain', "x-node-red-request-node":"INVALID_SUM"}});
+        });
+    })
+
 });