The issue here is that, if Lucee/Coldfusion scope cascading is on, then when 'target' is NULL, the engine will look up the scopes for it. If you have, for instance, FORM.target, due to submitting a link building form for example (https://presidecms.atlassian.net/browse/PRESIDECMS-1634), you'll get an unexpected result out of the cache.
Wow, this is so sucky, because, I thought the local scope was the first scope looked at. Is this not the case?
It’s not a matter of the first scope to be looked in, it’s a matter of the other scopes that are searched. The code is simply ambiguous and needs to be made explicit. When local.target doesn’t exist, CF doesn’t stop, it keeps looking until it finds a variable such as form.target. Which leads to incorrect results when an unrelated variable of that name exists in another scope that is searched as Dom pointed out.
The fix is simply to add local. as Dom has suggested.
I’d also like to point out, this closes some loopholes in which a malicious user of a ColdBox site may be able to “inject” variables into an app by adding form or url vars that will get accidentally picked up by the framework. I haven’t tried it yet to see if it’s actually exploitable, but the possibility is there.
But the tremendous issue here is that we will have to scope EVERY stupid local variable, if not, we have the same issue everywhere.
No, I don’t think you understand. This does not apply to every variable ever, just variables which may be null. Dom already identified all of these and addressed this in his pull. We simply need to merge it now.