Closed mozfreddyb closed 4 years ago
I'm not a JS developer, but I've done enough C/C++ that the syntax was familiar :)
I think a switch statement might look good there with a default of allowed=false which might be easier to maintain in the future and give a little performance bonus?
I can certainly contribute if you'd like, and the 4000 JS files I have to run it against seem to vary enough to trigger and fail there are any logic errors (especially now that i've had a successful run and know exactly how many errors and warnings the results should spit out)
Would be great if there was more input from folks who keep using this plugin. I've become less involved with Firefox OS frontend-security and real-world input is probably more useful.
(By the way, I've added a test in this commit ( https://github.com/mozfreddyb/eslint-plugin-no-unsafe-innerhtml/commit/3a0ff0635a589b91b3dae3cf6affc56737ac90cb#diff-2022531c4157fcf26990445f35b712f1R291) that we should have included in your previous pull request.)
@jonathankingston said
- [ ] Generalised method to get identifier of the function for stuff we don't know about it throws too. Because of this my
CallExpression
and
I think the ruleHelper is providing a majority of this.
When we check the expression on the right side of the innerHTML assignment (or the parameter within insertAdjacentHTML), we have some duplicate that looks for function names when the code contains a function call (see https://github.com/mozfreddyb/eslint-plugin-no-unsafe-innerhtml/blob/3a0ff0635a589b91b3dae3cf6affc56737ac90cb/lib/rules/no-unsafe-innerhtml.js#L66-L78).
We should remove the duplicate code and make it a bit simpler (and thus easier to read).
(@LockeLamora If you are looking to further contribute to this, please let me know :-))