Exec Node: Allow msg.payload to be redirected to stdin for the process by gorenje · Pull Request #4880 · node-red/node-red

gorenje · 2024-09-13T14:32:02Z

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Proposed changes

This adds the ability to pass the msg.payload into the stdin of a process started using the exec node.

This was discussed in the forum. The idea is that instead of doing who | wc -l in an exec node, it's more Node-RED-like to use an exec node that calls who, which pushes stdout to msg.payload, the msg is then passed to a exec node that passes msg.payload to stdin for the wc -l command.

This works best if the value (i.e. msg.payload) is a string, no provision is made to convert a value to a string that can be passed into a stream.

Checklist

I have read the contribution guidelines
For non-bugfix PRs, I have discussed this change on the forum/slack team.
I have run npm run test to verify the unit tests pass

  2757 passing (2m)
  61 pending


Done.

=============================== Coverage summary ===============================
Statements   : 69.48% ( 12045/17336 )
Branches     : 62.69% ( 6845/10919 )
Functions    : 65.13% ( 1642/2521 )
Lines        : 69.55% ( 11671/16780 )
================================================================================

Done.

I have added suitable unit tests to cover the new/changed functionality

knolleary

Looks good on review. Have proposed a couple changes to ensure the value passed to the stdin stream is a String or Buffer.

packages/node_modules/@node-red/nodes/core/function/90-exec.js

gorenje · 2025-02-24T21:20:08Z

@knolleary Updated with your suggestions and also added some specs for testing the feature.

The tests work on my linux box but I don't have a windows box to test whether the same is the case on windows - I suspect no since stdin is most probably not handled the same.

Using cmd /C cat as command on windows, no idea whether this is correct.

I would correct it if you have a suggestion.

knolleary · 2025-03-10T15:52:35Z

@Steve-Mcl can I get you to test this PR on windows to verify the stdin handling works as expected?

knolleary · 2025-06-06T15:33:41Z

@Steve-Mcl bumping this one - would love a validation this is happy on Windows before I merge.

Steve-Mcl

~~Firstly, it kinda works for Windows.~~

There is an issue where some piped commands (like piping to SORT or FIND) result in an error of "Cannot find file". The reason for this is unclear. I have spent far to long trying to figure out why some piping on windows node REPL works but fails in Node-RED exec node!

~~To be clear, this problem already exists. e.g. try running dir /B | sort /R in an exec node today results in this error so its not something new with this PR~~

See next comment where I realised gitbash was to blame!

My main hold out here is the following:

The built in docs are not update to reflect new functionality
Since the piping mechanism is not immediately intuitive (to me) (and I dont know how we could arrange the UI to make it more obvious), I strongly feel examples for this feature should be included.

To help get this over the line, I have created an example flow that I would hope gets included:

[{"id":"6b6b965c0173c9a0","type":"exec","z":"56ae6fc26f43b023","command":"tasklist","addpay":"","addpayTo":"parameter","append":"","useSpawn":"false","timer":"","winHide":false,"oldrc":false,"name":"","x":270,"y":340,"wires":[["b488a29ee1b28f65"],[],[]]},{"id":"b488a29ee1b28f65","type":"exec","z":"56ae6fc26f43b023","command":"findstr","addpay":"payload","addpayTo":"stdin","append":"/i \"node\"","useSpawn":"false","timer":"","winHide":false,"oldrc":false,"name":"","x":400,"y":340,"wires":[["ea381e89d7c11370"],[],[]]},{"id":"2c869f5b32b9c485","type":"inject","z":"56ae6fc26f43b023","name":"","props":[],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","x":175,"y":340,"wires":[["6b6b965c0173c9a0"]],"l":false},{"id":"ea381e89d7c11370","type":"debug","z":"56ae6fc26f43b023","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"payload","targetType":"msg","statusVal":"","statusType":"auto","x":570,"y":340,"wires":[]},{"id":"20910f9b01a72b32","type":"comment","z":"56ae6fc26f43b023","name":"ps aux | grep \"node\"","info":"","x":370,"y":140,"wires":[]},{"id":"a25b0de60d1287cc","type":"comment","z":"56ae6fc26f43b023","name":"tasklist | findstr /i \"node\"","info":"","x":380,"y":280,"wires":[]},{"id":"f407dc49122444a6","type":"comment","z":"56ae6fc26f43b023","name":"linux","info":"","x":190,"y":140,"wires":[]},{"id":"d2a0d5724175dd79","type":"comment","z":"56ae6fc26f43b023","name":"windows","info":"","x":200,"y":280,"wires":[]},{"id":"7ac7d3ff2c617d40","type":"exec","z":"56ae6fc26f43b023","command":"ps","addpay":"","addpayTo":"parameter","append":"aux","useSpawn":"false","timer":"","winHide":false,"oldrc":false,"name":"","x":270,"y":200,"wires":[["e5345b90069d923a"],[],[]]},{"id":"83280d96df77ee2c","type":"inject","z":"56ae6fc26f43b023","name":"","props":[],"repeat":"","crontab":"","once":false,"onceDelay":0.1,"topic":"","x":175,"y":200,"wires":[["7ac7d3ff2c617d40"]],"l":false},{"id":"e5345b90069d923a","type":"exec","z":"56ae6fc26f43b023","command":"grep","addpay":"payload","addpayTo":"stdin","append":"node","useSpawn":"false","timer":"","winHide":false,"oldrc":false,"name":"","x":400,"y":200,"wires":[["55d070f7234c9e60"],[],[]]},{"id":"55d070f7234c9e60","type":"debug","z":"56ae6fc26f43b023","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"payload","targetType":"msg","statusVal":"","statusType":"auto","x":570,"y":200,"wires":[]},{"id":"3a0c6b43d3e8eaca","type":"comment","z":"56ae6fc26f43b023","name":"piping results to stdin","info":"","x":220,"y":80,"wires":[]}]

Steve-Mcl · 2025-06-07T15:00:12Z

packages/node_modules/@node-red/nodes/core/function/90-exec.js

+        this.addpayTo = n.addpayTo;
+        if ( this.addpayTo === undefined ) {
+            this.addpayTo = "parameter"
+        }


Suggested change

this.addpayTo = n.addpayTo;

if ( this.addpayTo === undefined ) {

this.addpayTo = "parameter"

}

this.addpayTo = n.addpayTo

if (['parameter', 'stdin'].indexOf(this.addpayTo) < 0) {

this.addpayTo = 'parameter' // ensure sane value

}

While under normal conditions, addpayTo would only ever be "parameter" or "stdin". However, being a bit more defensive to bad JSON (looking at you AI generated flows) defensive is not a bad thing :)

PS, if we stick with a simple undefined test, it is is generally considered the more robust and "better" practice to use typeof e.g. if (typeof this.addpayTo === 'undefined' ) {

Steve-Mcl · 2025-06-08T10:55:12Z

There is an issue where some piped commands (like piping to SORT or FIND) result in an error of "Cannot find file". The reason for this is unclear. I have spent far to long trying to figure out why some piping on windows node REPL works but fails in Node-RED exec node!

To be clear, this problem already exists. e.g. try running dir /B | sort /R in an exec node today results in this error so its not something new with this PR

Ok, no sooner than I post my review do i find the answer!

I was running Node-RED from gitbash on windows which was causing the OS to use the GNU variants of sort and find - likely due to path ordering!!!

So, in short, it seems to work well for windows.

gorenje · 2025-06-08T11:05:17Z

So, in short, it seems to work well for windows.

Good news indeed!

I just came over to say that if it doesn't work consistently or support for this in core is not desired since no one wants to use it, then there is always the alternative to have an extra node package that implements this feature.

At the moment, there is a rough draft for the node over here, it wouldn't take much to make a proper node package out of it - haven't done so because of this PR.

Plus the package can have clear warning that it might or might work under windows, use at own risk software.

But if no one is really interested in this - and that seems to be the case - then probably better to leave it out of core. The best code is the code that doesn't exist!

Thanks @Steve-Mcl for the effort of testing the feature 👍

Steve-Mcl · 2025-06-10T12:15:55Z

@gorenje with NR v4.1 first beta about to get finalised, do you think you could action those 2 items on this PR please?

My main hold out here is the following:

The built in docs are not update to reflect new functionality

Since the piping mechanism is not immediately intuitive (to me) (and I dont know how we could arrange the UI to make it more obvious), I strongly feel examples for this feature should be included.

To help get this over the line, I have created an example flow that I would hope gets included:

gorenje · 2025-06-10T13:02:53Z

@Steve-Mcl I probably won't spend more time on this feature since in the months of this PR being open, no one has shown any interest in this feature, therefore I doubt it would get used.

As I wrote above, the best code is the code that isn't written, so I'd keep this out of core to limit the maintenance effort required.

Sorry for the wasted effort but why put code into core that no one will use. I have definitely come to realise that this isn't a feature that will get a lot of traction.

Steve-Mcl · 2025-06-11T08:19:35Z

Fair enough @gorenje - Appreciate the PR and the decision you made to close it.

changes to add stdin support to the exec node

fb4f515

knolleary reviewed Feb 24, 2025

View reviewed changes

packages/node_modules/@node-red/nodes/core/function/90-exec.js Show resolved Hide resolved

packages/node_modules/@node-red/nodes/core/function/90-exec.js Show resolved Hide resolved

gorenje added 3 commits February 24, 2025 19:15

Merge remote-tracking branch 'nodered/master' into exec-with-stdin

516faf2

check for value type and convert anything that isn't a buffer or string

ad230c9

added spec tests for the payload as stdin for the exec node

bf6febf

knolleary requested a review from Steve-Mcl June 6, 2025 15:33

Merge branch 'master' into exec-with-stdin

a0c0bd9

Steve-Mcl requested changes Jun 8, 2025

View reviewed changes

gorenje closed this Jun 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exec Node: Allow msg.payload to be redirected to stdin for the process#4880

Exec Node: Allow msg.payload to be redirected to stdin for the process#4880
gorenje wants to merge 5 commits intonode-red:masterfrom
gorenje:exec-with-stdin

gorenje commented Sep 13, 2024

Uh oh!

knolleary left a comment

Uh oh!

Uh oh!

Uh oh!

gorenje commented Feb 24, 2025

Uh oh!

knolleary commented Mar 10, 2025

Uh oh!

knolleary commented Jun 6, 2025

Uh oh!

Steve-Mcl left a comment •

edited

Loading

Uh oh!

Steve-Mcl Jun 7, 2025

Uh oh!

Steve-Mcl commented Jun 8, 2025

Uh oh!

gorenje commented Jun 8, 2025

Uh oh!

Steve-Mcl commented Jun 10, 2025

Uh oh!

gorenje commented Jun 10, 2025

Uh oh!

Steve-Mcl commented Jun 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

gorenje commented Sep 13, 2024

Proposed changes

Checklist

Uh oh!

knolleary left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gorenje commented Feb 24, 2025

Uh oh!

knolleary commented Mar 10, 2025

Uh oh!

knolleary commented Jun 6, 2025

Uh oh!

Steve-Mcl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Steve-Mcl Jun 7, 2025

Choose a reason for hiding this comment

Uh oh!

Steve-Mcl commented Jun 8, 2025

Uh oh!

gorenje commented Jun 8, 2025

Uh oh!

Steve-Mcl commented Jun 10, 2025

Uh oh!

gorenje commented Jun 10, 2025

Uh oh!

Steve-Mcl commented Jun 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Steve-Mcl left a comment •

edited

Loading