Fixes #8629: Allows generation-time javascript eval in directive para… #1144
Conversation
ncharles
force-pushed
the
ust_8629/allows_generation_time_javascript_eval_in_directive_parameters
branch
from
0f37831
to
67b1d53
Compare
| ) { | ||
|
|
||
| ///// simple hash algorithms ///// | ||
| private val hash = new JsLibHash() |
There was a problem hiding this comment.
Why do you neet a private val if you have a getter ? I would only have a val
There was a problem hiding this comment.
i suspect this is for visibility issue. With the getHash, it gives access to hash - but if hash is public, there might be some java magic going on
There was a problem hiding this comment.
because Java conventions are used for the scripting engine
ncharles
force-pushed
the
ust_8629/allows_generation_time_javascript_eval_in_directive_parameters
branch
from
67b1d53
to
2ac33b5
Compare
|
Commit modified |
| final val DEFAULT_EVAL = "eval" | ||
| final val EVALJS = "evaljs" | ||
|
|
||
| final val default_pattern = """(?ms)(.*)\$\{eval\s+(.*)}(.*)""".r |
There was a problem hiding this comment.
Parsing ${evaljs ...} with regex won't work in intersting fashion, especially with rudder variables and js blocks.
In all case, it will have a massive impact on generation time - we switch from a "startWith" method, dicidable in constant(EVALJS.size) time, to an unbound one - at least o(n), but with extrem backtracking bad case, see stackoferflow recent outage.
Finally, I don't even think we should have {}. It add visual clutters for no reason, because really, we want that the whole input is a script or not. Supporting multiple script need to have a ROBUST input parser, understanding each language to know if the closing } is its or ours - something I don't even want to start coding. And if it is to have part of the input "as is", here you go : javascript is able to manage strings !
So to sum-up:
- I don't thing we should have {} letting the user think it can have several eval in an input ;
- I don't thing we should autorise $evaljs at an other place than the strict begining of the field.
There was a problem hiding this comment.
(yes, the second statement mean that input fields need adaptation to handle that. But is it more than the password one ? And if really we can't specialise input field to handle $evaljs correctly, perhaps we should had a "authorised ignored starting string before $evaljs, declared for each field")
|
PR updated |
|
i'm rebasing it |
…of the string for performance reason
ncharles
force-pushed
the
ust_8629/allows_generation_time_javascript_eval_in_directive_parameters
branch
from
390af65
to
dcedf8e
Compare
|
pr updated |
|
This PR is not mergeable to upper versions. |
|
OK, merging this PR |
…meters