mozilla / eslint-plugin-no-unsanitized

Custom ESLint rule to disallows unsafe innerHTML, outerHTML, insertAdjacentHTML and alike
Mozilla Public License 2.0
223 stars 34 forks source link

Unsupported Callee for CallExpression #83

Closed audishos closed 5 years ago

audishos commented 6 years ago
const ShippingContainer = Loadable({
    loader: () => import('../panels/shipping/ShippingContainer'),
    loading: LoadingComponent,
});

It doesn't like dynamic imports 👎

mozfreddyb commented 6 years ago

Hi @audishos. Thanks for filing this issue!

I can't find a way to even parse dynamic imports with either the eslint parser or the esprima parser.

It looks to me like dynamic imports are still a proposal? Can you add some extra details in which environment you're using them?

audishos commented 6 years ago

It's supported via webpack https://webpack.js.org/guides/code-splitting/#dynamic-imports I'm using it in react so using babel with the plugins babel-eslint and babel-plugin-syntax-dynamic-import. babel-plugin-dynamic-import-node if using with node.

mozfreddyb commented 6 years ago

Thanks for clarifying. I'm not sure we want to support TC39 proposals, regardless of what stage they are in, until they are supported by eslint proper. However, if you can convince me that it's highly likely to change significantly, I'd be happy to mentor you creating a pull request for dynamic import support.

In the meantime, you could disable the rule with an eslint-specific comment (per line, per file), but I'm not sure if that will help you avoid parser failures like in this case.

mozfreddyb commented 6 years ago

According to the current draft, it seems like the callee node is called ImportCall. If you want to give this a go, you'd have to allow ImportCall callees in method.js, line 77

audishos commented 6 years ago

I tried case "ImportCall": to the list under "those are fine" but no dice. Is this what you meant?

mozfreddyb commented 6 years ago

Thank you for filing this issue @audishos. Let me know if the pull request by @spalger in #84 works for you and I'll make a new release sometime later this week.

audishos commented 6 years ago

Installed via github master branch and looks like the issue is resolved. Please let me know once the release is available.

Cheers!

mozfreddyb commented 6 years ago

Published!

npm notice 📦 eslint-plugin-no-unsanitized@3.0.1

koto commented 5 years ago

Didn't this just enable arbitrary script loads? dynamic import accepts expressions, it's even more dangerous than innerHTML in that regard, as it directly loads scripts. Perhaps it could be allowed to be called with string literals only?

spalger commented 5 years ago

Interesting point @koto, I recommend opening another issue

mozfreddyb commented 5 years ago

Yeah. That's bad.