Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider allowing method name checks over getSource check #41

Closed
jonathanKingston opened this issue Mar 21, 2017 · 4 comments
Closed

Consider allowing method name checks over getSource check #41

jonathanKingston opened this issue Mar 21, 2017 · 4 comments

Comments

@jonathanKingston
Copy link
Collaborator

Currently the code checks for context.getSource(node.callee) for write and writeln but node.callee.property.name for insertAdjacentHTML.

This might be a very breaking code change to add to make write use the property.name as some code bases use this method name. Making this change however will catch any other reference to document using the write method which is somewhat a common pattern in non-malice™ code:

let d = document;
d.writeln("blah");
d.writeln("blah");
d.writeln("blah");
d.writeln("blah");

Without this making the code configurable will be harder #29

@mozfreddyb
Copy link
Collaborator

Your example makes sense, but I doubt a lot of people will save a reference to document in unminified code. My main concern is that write is such a generic name.
Maybe we should check some popular libraries, to see how common it is?

On the other hand, our worry about false positives should not be so big, if people can just disable eslint with a tiny comment on top of the line.

I'm not sure what my position here is.

@jonathanKingston
Copy link
Collaborator Author

I'm not sure what my position here is.

Mine neither, I could check big codebases on how often document assignment is within unminified code. I have seen it done myself certainly with frames:

const parentDoc = document.parent.document;
parentDoc.write("a");

@mozfreddyb
Copy link
Collaborator

mozfreddyb commented Mar 24, 2017

Another data point:
There's lots of .write() for streams (stdin, stdout) and files in our js and jsm files according to [searchfox](http://searchfox.org/mozilla-central/search?q=%5C.write%5C(&case=false&regexp=true&path=.js*)

@mozfreddyb
Copy link
Collaborator

This will be fixed by the new format allowing regexes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants