Adding support for mustache-like substitution in Change node msg/flow types

[knolleary]

See #43 (comment) for an updated proposal
Proposal: add support for mustache syntax in the msg/flow/global TypedInput.

The Change node is the main node we provide for setting node properties. It is easy to use when setting a well-defined message or context property. The problem comes when you need to dynamically determine the property to set.

For example, you are receiving temperatures from a number of sensors. Each message has msg.topic set to the room the sensor is in and msg.payload is the value. You want to store the current temperature of each room in context under flow.rooms - for example flow.rooms.kitchen and flow.rooms.garage.

To do this with the Change node today requires the use of JSONata. For example:

• set flow.rooms to the Expression $merge([$flowContext('rooms'),{$.topic:$.payload}])

More likely, users will fall back to a Function node - something it would be nice to avoid.

It would be much more user friendly to be able to do:

• set flow.rooms.{{msg.topic}} to msg.payload

We do often get users asking for help because they have tried to do something like that. If we build mustache-like support into the TypedInput then all nodes that use it will benefit.

I think this would be an important usability improvement.

One open question is whether the syntax would be {{msg.topic}} or just {{topic}}. The HTTP Request node's support for mustache does not require the msg. prefix - but it cannot be used to reference context values.

I think we would be flexible. Given the value {{AAA.BBB}}:

• if AAA is msg, the lookup msg.BBB
• if AAA is flow - check if msg.flow exists. If it does, lookup msg.flow.BBB. Otherwise use flow.BBB
• if AAA is global - check if msg.global exists. If it does, lookup msg.global.BBB. Otherwise use global.BBB
• otherwise, lookup msg.AAA.BBB

I say 'mustache-like' syntax, as we're only talking about {{ }} - not the other mustache syntax.

[tfmf]

I think this is a huge improvement when you are actually developing code inside the node. My opinion is that we should keep it explicit, so the {{msg.topic}} would be my preference.

[dceejay]

So is this for all typed inputs or just in the change node ?
We have a lot of typed inputs for things like choosing which property to operate on

they tend to use getMessageProperty rather than evaluateProperty... How will a user know which typed input field can accept this new syntax and which can't ?

Will it handle arrays ? on either side of the evaluation ? eg msg.room[{{msg.index}}] and msg.room.{{msg.topic[2]}} ?

Does this open up security holes a bit like path traversal ? if say the key is msg.payload - I can get to flows.securitytoken too easily if I set my field to be flows.{{msg.payload}}

[knolleary]

they tend to use getMessageProperty rather than evaluateProperty. How will a user know which typed input field can accept this new syntax and which can't ?

That is an issue. If we introduce it as a feature of the Change node (even if the actual implementation is in the shared RED.util.evaluateNodeProperty function), rather than a generic feature of the TypedInput, then that will help set the expectation. Nodes can then migrate over to the evaluateProperty syntax if it makes sense to do so on a case by case basis.

Will it handle arrays ? on either side of the evaluation ? eg msg.room[{{msg.index}}] and msg.room.{{msg.topic[2]}} ?

msg.room[{{msg.index}}] - that will just work because it will evaluate to the string msg.room[kitchen]which is a valid identifier.

msg.room.{{msg.topic[2]}} - mustache syntax doesn't use [] for array reference. A strict mustache statement would be msg.room.{{msg.topic.2}}. However, we know that users find that counter-intuitive. So this would support either msg.topic.2 or msg.topic[2].

Does this open up security holes a bit like path traversal ?

This isn't new logic. A user could have a flow today that uses a Function node to dynamically look-up a context value. All we are doing here is providing a way to do that dynamic look-up in the Change node.

If a flow looks up a context value using msg.payload as the key into context, and that property comes from an external source, then the flow needs to sanitise the value regardless of which node does the work.

[dceejay]

Yes - making it part of the change node is good.

"This isn't new logic.... " - yes but it makes it easier to fall into the hole :-) But again - if as part of the change node then yes you are expecting things to be changed so much less of an issue. If everywhere then may be unexpected/unintended.

So yes - happy to proceed as documented.

[knolleary]

I've been poking around at this and I have a slightly different proposal - not to use mustache-syntax at all.

Taking my original example: flow.rooms.{{msg.topic}}

If you were writing the equivalent in JavaScript, you'd write flow.rooms[msg.topic].

I think that is more familiar and intuitive to a new user - no need to learn about the {{ }} mustache syntax at all.

The code that parses property expressions is already strict and would flag up flow.rooms[msg.topic] as invalid - it expects the text inside [ ] to be either a number or a properly quoted string. This means there's no risk of accidentally breaking an existing flow by introducing this new syntax.

The one downside of not using {{ }} is if you wanted to do concatenation of a property key.

Going back to the original example, if the keys were instead roomkitchen and roomgarage, then you could imaging doing: flow.room{{msg.topic}} which would first do the string substitution to get flow.roomkitchen (for example), before being evaluated to get the actual value. To be honest, that feels like an edge case too far to worry about - the Function node is still there to do that if it's really needed.

So, to summarise, the new proposal is to support msg/flow/global. property expressions appearing inside [] within a property expression.

[dceejay]

so where does that end up on the msg.topic vs just topic thought ? would it still try to handle both or is it now explicitly msg.topic (for the key) ? so flow.room[msg.topic] and not flow.room[topic]

[knolleary]

Better to be explicit I think - so msg.topic.

The fact it isn't mustache syntax means no worries of inconsistencies with other places mustache is used (eg Http request)

[dceejay]

exactly. 👍

[knolleary]

It occurs to me, the get/setMessageProperty() functions we have today will be able to Just Work with nested expressions that reference other msg properties. The function has access to the msg object so can do all the work, synchronously, to lookup any internal cross-references.

What it cannot do is handle flow/global nested expressions.

I am tempted, for an initial pass of this, to limit it only supporting msg references inside the expression. Allowing flow/global references in the expression is quite complicated to get right and for what was meant to be a couple hours work, is more time consuming that I want at this point in time.

[knolleary]

PR that adds support for nested msg references here: node-red/node-red#2822

[dceejay]

this would work fine in a change node... if you need to get a flow variable can you do "set msg.temp to flow.foo" then "set msg.room[msg.temp] to msg.payload" (or whatever) - ie two steps ?