From 3ed5969e87315788750a01626479be65ff1e318a Mon Sep 17 00:00:00 2001 From: Stephen McLaughlin <44235289+Steve-Mcl@users.noreply.github.com> Date: Mon, 28 Nov 2022 19:31:44 +0000 Subject: [PATCH 1/3] add msg.payloadHandling for get requests --- .../nodes/core/network/21-httprequest.js | 35 +++---- .../nodes/core/network/21-httprequest_spec.js | 95 +++++++++++++++++++ 2 files changed, 109 insertions(+), 21 deletions(-) diff --git a/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js b/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js index 9eac4da91..f7b17e6d1 100644 --- a/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js +++ b/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js @@ -69,8 +69,6 @@ in your Node-RED user directory (${RED.settings.userDir}). var nodeUrl = n.url; var isTemplatedUrl = (nodeUrl||"").indexOf("{{") != -1; var nodeMethod = n.method || "GET"; - var paytoqs = false; - var paytobody = false; var redirectList = []; var sendErrorsToCatch = n.senderr; node.headers = n.headers || []; @@ -78,15 +76,12 @@ in your Node-RED user directory (${RED.settings.userDir}). if (n.tls) { var tlsNode = RED.nodes.getNode(n.tls); } - this.ret = n.ret || "txt"; - this.authType = n.authType || "basic"; - if (RED.settings.httpRequestTimeout) { this.reqTimeout = parseInt(RED.settings.httpRequestTimeout) || 120000; } - else { this.reqTimeout = 120000; } - - if (n.paytoqs === true || n.paytoqs === "query") { paytoqs = true; } - else if (n.paytoqs === "body") { paytobody = true; } - + node.ret = n.ret || "txt"; + node.authType = n.authType || "basic"; + if (RED.settings.httpRequestTimeout) { node.reqTimeout = parseInt(RED.settings.httpRequestTimeout) || 120000; } + else { node.reqTimeout = 120000; } node.insecureHTTPParser = n.insecureHTTPParser + node.paytoqs = n.paytoqs var prox, noprox; if (process.env.http_proxy) { prox = process.env.http_proxy; } @@ -196,20 +191,20 @@ in your Node-RED user directory (${RED.settings.userDir}). } } - var method = nodeMethod.toUpperCase() || "GET"; + /** @type {boolean|'query'|'body'} */ + let payloadHandling = node.paytoqs + let method = nodeMethod.toUpperCase() || "GET"; if (msg.method && n.method && (n.method !== "use")) { // warn if override option not set node.warn(RED._("common.errors.nooverride")); } if (msg.method && n.method && (n.method === "use")) { - method = msg.method.toUpperCase(); // use the msg parameter + method = msg.method.toUpperCase(); // use the msg parameter + payloadHandling = msg.payloadHandling } + if (payloadHandling === true) { payloadHandling = "query" } - // var isHttps = (/^https/i.test(url)); - + /** @type {import('got').Options} */ var opts = {}; - // set defaultport, else when using HttpsProxyAgent, it's defaultPort of 443 will be used :(. - // Had to remove this to get http->https redirect to work - // opts.defaultPort = isHttps?443:80; opts.timeout = node.reqTimeout; opts.throwHttpErrors = false; // TODO: add UI option to auto decompress. Setting to false for 1.x compatibility @@ -472,7 +467,7 @@ in your Node-RED user directory (${RED.settings.userDir}). } - if (method == 'GET' && typeof msg.payload !== "undefined" && paytoqs) { + if (method == "GET" && typeof msg.payload !== "undefined" && payloadHandling === "query") { if (typeof msg.payload === "object") { try { if (url.indexOf("?") !== -1) { @@ -481,18 +476,16 @@ in your Node-RED user directory (${RED.settings.userDir}). url += "?" + querystring.stringify(msg.payload); } } catch(err) { - node.error(RED._("httpin.errors.invalid-payload"),msg); nodeDone(); return; } } else { - node.error(RED._("httpin.errors.invalid-payload"),msg); nodeDone(); return; } - } else if ( method == "GET" && typeof msg.payload !== "undefined" && paytobody) { + } else if ( method == "GET" && typeof msg.payload !== "undefined" && payloadHandling === "body") { opts.allowGetBody = true; if (typeof msg.payload === "object") { opts.body = JSON.stringify(msg.payload); diff --git a/test/nodes/core/network/21-httprequest_spec.js b/test/nodes/core/network/21-httprequest_spec.js index c6c6ac4ac..b0307d7f4 100644 --- a/test/nodes/core/network/21-httprequest_spec.js +++ b/test/nodes/core/network/21-httprequest_spec.js @@ -294,6 +294,18 @@ describe('HTTP Request Node', function() { url: req.originalUrl }); }) + testApp.get('/getBodyParams', function(req,res) { + // Either body-parser or express is discarding the body OR + // GOT never sent a body. (there is nothing in params/query/body) + // Oddly, if we set options.json (instead of options.body) in the GOT + // request, then req.body will have the values! + // Has the 21-httprequest node ever been able to send a payload in body + // for a GET request? + res.json({ + // body:JSON.parse(req.body), + url: req.originalUrl + }); + }) testApp.get('/returnError/:code', function(req,res) { res.status(parseInt(req.params.code)).json({gotError:req.params.code}); }) @@ -1117,6 +1129,89 @@ describe('HTTP Request Node', function() { }); }); + it('should allow the payload to be sent in the body for a GET request', function(done) { + var flow = [{id:"n1",type:"http request",wires:[["n2"]],method:"GET",paytoqs:"body",ret:"obj",url:getTestURL('/getBodyParams')}, + {id:"n2", type:"helper"}]; + helper.load(httpRequestNode, flow, function() { + var n1 = helper.getNode("n1"); + var n2 = helper.getNode("n2"); + const payload = {a:"one",b:true,c:3} + n2.on("input", function(msg) { + try { + // Either Express does not deliver the body of a GET request OR GOT doesn't send it! + // That means we cannot test this! Oddly, if the HTTP-Req node sets options.json instead + // of options.body, then the body is delivered. + // msg.should.have.property('payload',{ + // body:payload, + // url: '/getBodyParams' + // }); + msg.should.have.property('payload').and.be.an.Object() + msg.payload.should.have.property('url', '/getBodyParams') + + msg.should.have.property('statusCode',200); + msg.should.have.property('headers'); + done(); + } catch(err) { + done(err); + } + }); + n1.receive({payload:payload}); + }); + }); + + it('should allow the message to specify that the payload be append to querystring for a GET request', function(done) { + var flow = [{id:"n1",type:"http request",wires:[["n2"]],method:"use",ret:"obj",url:getTestURL('/getQueryParams')}, + {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.should.have.property('payload',{ + query:{a:'1',b:'2',c:'3'}, + url: '/getQueryParams?a=1&b=2&c=3' + }); + msg.should.have.property('statusCode',200); + msg.should.have.property('headers'); + done(); + } catch(err) { + done(err); + } + }); + n1.receive({payload:{a:1,b:2,c:3},method:"get",payloadHandling:'query'}); + }); + }); + + + it('should allow the message to specify that the payload be sent in the body for a GET request', function(done) { + var flow = [{id:"n1",type:"http request",wires:[["n2"]],method:"use",ret:"obj",url:getTestURL('/getBodyParams')}, + {id:"n2", type:"helper"}]; + helper.load(httpRequestNode, flow, function() { + var n1 = helper.getNode("n1"); + var n2 = helper.getNode("n2"); + const payload = {a:"one",b:true,c:3} + n2.on("input", function(msg) { + try { + // Either Express does not deliver the body of a GET request OR GOT doesn't send it! + // That means we cannot test this! Oddly, if the HTTP-Req node sets options.json instead + // of options.body, then the body is delivered. + // msg.should.have.property('payload',{ + // payload: payload, + // url: '/getBodyParams' + // }); + msg.should.have.property('payload').and.be.an.Object() + msg.payload.should.have.property('url', '/getBodyParams') + msg.should.have.property('statusCode',200); + msg.should.have.property('headers'); + done(); + } catch(err) { + done(err); + } + }); + n1.receive({payload:payload,method:"get",payloadHandling:'body'}); + }); + }); + it('should send a msg for non-2xx response status - 400', function(done) { var flow = [{id:"n1",type:"http request",wires:[["n2"]],method:"GET",ret:"obj",url:getTestURL('/returnError/400')}, {id:"n2", type:"helper"}]; From 3a5cdbc8ae2e9317b766574e8dd6831d52478aa1 Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Tue, 29 Nov 2022 13:38:09 +0000 Subject: [PATCH 2/3] improve UI around payloadHandling --- .../nodes/core/network/21-httprequest.html | 5 +++-- .../@node-red/nodes/core/network/21-httprequest.js | 14 ++++++++++---- .../@node-red/nodes/locales/de/messages.json | 4 +++- .../@node-red/nodes/locales/en-US/messages.json | 4 +++- .../locales/en-US/network/21-httprequest.html | 4 ++++ .../@node-red/nodes/locales/ja/messages.json | 4 +++- .../@node-red/nodes/locales/ko/messages.json | 8 +++++++- .../@node-red/nodes/locales/ru/messages.json | 4 +++- .../@node-red/nodes/locales/zh-CN/messages.json | 8 +++++++- .../@node-red/nodes/locales/zh-TW/messages.json | 8 +++++++- test/nodes/core/network/21-httprequest_spec.js | 7 ++++--- 11 files changed, 54 insertions(+), 16 deletions(-) diff --git a/packages/node_modules/@node-red/nodes/core/network/21-httprequest.html b/packages/node_modules/@node-red/nodes/core/network/21-httprequest.html index eb0aeaf47..3572be09d 100644 --- a/packages/node_modules/@node-red/nodes/core/network/21-httprequest.html +++ b/packages/node_modules/@node-red/nodes/core/network/21-httprequest.html @@ -33,11 +33,12 @@
- +
@@ -290,7 +291,7 @@ RED.tray.resize(); }); $("#node-input-method").on("change", function() { - if ($(this).val() == "GET") { + if ($(this).val() == "GET" || $(this).val() == "use") { $(".node-input-paytoqs-row").show(); } else { $(".node-input-paytoqs-row").hide(); diff --git a/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js b/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js index f7b17e6d1..911f4daba 100644 --- a/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js +++ b/packages/node_modules/@node-red/nodes/core/network/21-httprequest.js @@ -16,7 +16,7 @@ module.exports = function(RED) { "use strict"; - const got = require("got"); + const got = require("got").default; const {CookieJar} = require("tough-cookie"); const { HttpProxyAgent, HttpsProxyAgent } = require('hpagent'); const FormData = require('form-data'); @@ -191,15 +191,21 @@ in your Node-RED user directory (${RED.settings.userDir}). } } - /** @type {boolean|'query'|'body'} */ - let payloadHandling = node.paytoqs let method = nodeMethod.toUpperCase() || "GET"; if (msg.method && n.method && (n.method !== "use")) { // warn if override option not set node.warn(RED._("common.errors.nooverride")); } if (msg.method && n.method && (n.method === "use")) { method = msg.method.toUpperCase(); // use the msg parameter - payloadHandling = msg.payloadHandling + } + + /** @type {boolean|'query'|'body'|'setby'} */ + let payloadHandling = node.paytoqs + if (msg.payloadHandling && payloadHandling && (payloadHandling !== "setby")) { // warn if override option not set + node.warn(RED._("common.errors.nooverride")); + } + if (msg.payloadHandling && payloadHandling && (payloadHandling === "setby")) { + payloadHandling = msg.payloadHandling // use the msg parameter } if (payloadHandling === true) { payloadHandling = "query" } diff --git a/packages/node_modules/@node-red/nodes/locales/de/messages.json b/packages/node_modules/@node-red/nodes/locales/de/messages.json index 65f251e98..c09cced60 100755 --- a/packages/node_modules/@node-red/nodes/locales/de/messages.json +++ b/packages/node_modules/@node-red/nodes/locales/de/messages.json @@ -449,9 +449,11 @@ "headers": "Kopfzeilen", "other": "andere", "paytoqs": { + "label": "Payload (GET)", "ignore": "Ignorieren", "query": "Anfügen an query-string-Parameter", - "body": "Senden als request-body" + "body": "Senden als request-body", + "setby": "Durch msg.payloadHandling festgelegt" }, "utf8String": "UTF-8-String", "binaryBuffer": "Binärer Buffer", diff --git a/packages/node_modules/@node-red/nodes/locales/en-US/messages.json b/packages/node_modules/@node-red/nodes/locales/en-US/messages.json index 7fcd5eadc..f0094de8d 100644 --- a/packages/node_modules/@node-red/nodes/locales/en-US/messages.json +++ b/packages/node_modules/@node-red/nodes/locales/en-US/messages.json @@ -509,9 +509,11 @@ "headers": "Headers", "other": "other", "paytoqs": { + "label": "Payload (GET)", "ignore": "Ignore", "query": "Append to query-string parameters", - "body": "Send as request body" + "body": "Send as request body", + "setby": "- set by msg.payloadHandling -" }, "utf8String": "UTF8 string", "binaryBuffer": "binary buffer", diff --git a/packages/node_modules/@node-red/nodes/locales/en-US/network/21-httprequest.html b/packages/node_modules/@node-red/nodes/locales/en-US/network/21-httprequest.html index b8d7fc048..4fb39ac71 100644 --- a/packages/node_modules/@node-red/nodes/locales/en-US/network/21-httprequest.html +++ b/packages/node_modules/@node-red/nodes/locales/en-US/network/21-httprequest.html @@ -30,6 +30,10 @@
If set, can be used to send cookies with the request.
payload
Sent as the body of the request.
+
payloadHandling string
+
Only valid with GET requests. If set to "- use msg.payloadHandling -" in the node configuration, this property + indicates how the payload will be sent. msg.payloadHandling should contain either + "query" or "body" otherwise the payload will not be sent with the GET request
rejectUnauthorized
If set to false, allows requests to be made to https sites that use self signed certificates.
diff --git a/packages/node_modules/@node-red/nodes/locales/ja/messages.json b/packages/node_modules/@node-red/nodes/locales/ja/messages.json index b7fa20b02..e722f5e19 100644 --- a/packages/node_modules/@node-red/nodes/locales/ja/messages.json +++ b/packages/node_modules/@node-red/nodes/locales/ja/messages.json @@ -509,9 +509,11 @@ "headers": "ヘッダ", "other": "その他", "paytoqs": { + "label": "ペイロード (GET)", "ignore": "無視", "query": "クエリパラメータに追加", - "body": "リクエストボディとして送信" + "body": "リクエストボディとして送信", + "setby": "- msg.payloadHandlingに定義 -" }, "utf8String": "UTF8文字列", "binaryBuffer": "バイナリバッファ", diff --git a/packages/node_modules/@node-red/nodes/locales/ko/messages.json b/packages/node_modules/@node-red/nodes/locales/ko/messages.json index 15b7bee8e..2b0a88695 100755 --- a/packages/node_modules/@node-red/nodes/locales/ko/messages.json +++ b/packages/node_modules/@node-red/nodes/locales/ko/messages.json @@ -387,7 +387,13 @@ "status": "상태코드", "headers": "헤더", "other": "그 외", - "paytoqs" : "msg.payload를 쿼리 파라미터에 추가", + "paytoqs": { + "label": "페이로드(GET)", + "ignore": "무시", + "query": "msg.payload를 쿼리 파라미터에 추가", + "body": "본문에 msg.payload 추가", + "setby": "- msg.payloadHandling에 의해 설정됨 -" + }, "utf8String": "UTF8문자열", "binaryBuffer": "바이너리 버퍼", "jsonObject": "JSON오브젝트", diff --git a/packages/node_modules/@node-red/nodes/locales/ru/messages.json b/packages/node_modules/@node-red/nodes/locales/ru/messages.json index 32364b458..b41be9457 100755 --- a/packages/node_modules/@node-red/nodes/locales/ru/messages.json +++ b/packages/node_modules/@node-red/nodes/locales/ru/messages.json @@ -411,9 +411,11 @@ "headers": "Заголовки", "other": "другое", "paytoqs": { + "label": "Данные (GET)", "ignore": "Игнорировать", "query": "Добавлять к параметрам строки запроса", - "body": "Отправлять как тело запроса" + "body": "Отправлять как тело запроса", + "setby": "- устанавливается через msg.payloadHandling -" }, "utf8String": "Строка UTF8", "binaryBuffer": "двоичный буфер", diff --git a/packages/node_modules/@node-red/nodes/locales/zh-CN/messages.json b/packages/node_modules/@node-red/nodes/locales/zh-CN/messages.json index b649163b7..7d3775f8f 100644 --- a/packages/node_modules/@node-red/nodes/locales/zh-CN/messages.json +++ b/packages/node_modules/@node-red/nodes/locales/zh-CN/messages.json @@ -407,7 +407,13 @@ "status": "状态码", "headers": "头", "other": "其他", - "paytoqs": "将msg.payload附加为查询字符串参数", + "paytoqs": { + "label": "有效负载（仅限 GET）", + "ignore": "漠视", + "query": "将msg.payload附加为查询字符串参数", + "body": "在请求正文中发送负载", + "setby": "- 用 msg.payloadHandling 设定 -" + }, "utf8String": "UTF8格式的字符串", "binaryBuffer": "二进制buffer", "jsonObject": "解析的JSON对象", diff --git a/packages/node_modules/@node-red/nodes/locales/zh-TW/messages.json b/packages/node_modules/@node-red/nodes/locales/zh-TW/messages.json index d63c47865..63f358b17 100644 --- a/packages/node_modules/@node-red/nodes/locales/zh-TW/messages.json +++ b/packages/node_modules/@node-red/nodes/locales/zh-TW/messages.json @@ -411,7 +411,13 @@ "status": "狀態碼", "headers": "Header", "other": "其他", - "paytoqs": "將msg.payload附加為查詢字符串參數", + "paytoqs": { + "label": "有效負載（僅限 GET）", + "ignore": "漠視", + "query": "將msg.payload附加為查詢字符串參數", + "body": "在請求正文中發送負載", + "setby": "- 用 msg.payloadHandling 設定 -" + }, "utf8String": "UTF8格式的字符串", "binaryBuffer": "二進制buffer", "jsonObject": "解析的JSON對象", diff --git a/test/nodes/core/network/21-httprequest_spec.js b/test/nodes/core/network/21-httprequest_spec.js index b0307d7f4..412ef105a 100644 --- a/test/nodes/core/network/21-httprequest_spec.js +++ b/test/nodes/core/network/21-httprequest_spec.js @@ -299,10 +299,11 @@ describe('HTTP Request Node', function() { // GOT never sent a body. (there is nothing in params/query/body) // Oddly, if we set options.json (instead of options.body) in the GOT // request, then req.body will have the values! - // Has the 21-httprequest node ever been able to send a payload in body - // for a GET request? + //I suspect the GOT lib *is* sending body on a GET request since an + // error is thrown if we try to set options.body on a GET request without + // setting options.allowGetBody to true. res.json({ - // body:JSON.parse(req.body), + // body:JSON.parse(req.body), //req.body is empty! url: req.originalUrl }); }) From 20a63a3292530c01cfcd799b4adcbcadcbbf48c1 Mon Sep 17 00:00:00 2001 From: Steve-Mcl Date: Tue, 29 Nov 2022 13:50:39 +0000 Subject: [PATCH 3/3] fix tests after adding 'set by msg.payloadHandling' --- test/nodes/core/network/21-httprequest_spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/nodes/core/network/21-httprequest_spec.js b/test/nodes/core/network/21-httprequest_spec.js index 412ef105a..2b21feb4a 100644 --- a/test/nodes/core/network/21-httprequest_spec.js +++ b/test/nodes/core/network/21-httprequest_spec.js @@ -1161,7 +1161,7 @@ describe('HTTP Request Node', function() { }); it('should allow the message to specify that the payload be append to querystring for a GET request', function(done) { - var flow = [{id:"n1",type:"http request",wires:[["n2"]],method:"use",ret:"obj",url:getTestURL('/getQueryParams')}, + var flow = [{id:"n1",type:"http request",wires:[["n2"]],method:"use",paytoqs:"setby",ret:"obj",url:getTestURL('/getQueryParams')}, {id:"n2", type:"helper"}]; helper.load(httpRequestNode, flow, function() { var n1 = helper.getNode("n1"); @@ -1185,7 +1185,7 @@ describe('HTTP Request Node', function() { it('should allow the message to specify that the payload be sent in the body for a GET request', function(done) { - var flow = [{id:"n1",type:"http request",wires:[["n2"]],method:"use",ret:"obj",url:getTestURL('/getBodyParams')}, + var flow = [{id:"n1",type:"http request",wires:[["n2"]],method:"use",paytoqs:"setby",ret:"obj",url:getTestURL('/getBodyParams')}, {id:"n2", type:"helper"}]; helper.load(httpRequestNode, flow, function() { var n1 = helper.getNode("n1");