search

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

unexpected AssignmentExpression in normalizeMethodName #124

Closed gdh1995 closed 4 years ago

gdh1995 commented 4 years ago

I'm using terser (https://github.com/terser/terser) to minify JS code of my own files and 3rd-party libraries, but today when I uploaded a new version of web-extension, the auto tests reported:

Error in no-unsanitized: Unexpected callable. Please report a minimal code snippet to the developers at https://github.com/mozilla/eslint-plugin-no-unsanitized/issues/new?title=unexpected%20AssignmentExpression%20in%20normalizeMethodName
警告: Due to both security and performance concerns, this may not be set using dynamic values which have not been adequately sanitized. This can lead to security issues or fairly serious performance degradation.

lib/math_parser.js 第 14 行，第 2742 列 (Row 14, Col 2742)

But the code at that place is just:

i=u.pop(),u.push((e=this.n[n.i])(i,r));
// `2742` means `(e...`

I can not get what's wrong with this code part. Maybe this is a bug of this eslint plugin.

This is the zip file with such an error: vimium_c-1.83.0-firefox-b419664.zip . It's from https://github.com/gdh1995/vimium-c/commit/b4196642e6a17804eda4538d8a98e1f9dca7c94d and the building script is npm run firefox.

mozfreddyb commented 4 years ago

I think this might have been fixed with #114, which made it into version 3.1.1, that is in this repo but not published to npm. my bad!

mozfreddyb commented 4 years ago

version 3.1.1 should now be published on npm: https://www.npmjs.com/package/eslint-plugin-no-unsanitized

Let me know if it doesn't fix it and I'll be happy to look into your issue again.

gdh1995 commented 4 years ago

Em, when will https://addons.mozilla.org/ update its node modules? I just took a new try but this issue is still there (same line number and column number).

Test file: vimium_c-1.83.1-firefox-3d3a232.zip

gdh1995 commented 4 years ago

~@mostlygeek~ This issue still exists on https://addons.mozilla.org/ when I uploaded a new version of extensions today.

mostlygeek commented 4 years ago

@gdh1995 I think you may have pinged the wrong person.

mozfreddyb commented 4 years ago

yeah, that's an issue with the code that is consuming this rule (https://github.com/mozilla/addons-linter). paging @rpl @EnTeQuAk

rpl commented 4 years ago

@mozfreddyb I just double-checked this locally, using an addons-linter build that is already using eslint-plugin-no-unsanitized version 3.1.1 (and so it is already including #114) but the linting errors are still being triggered.

It looks that we may have to special case this scenario in the plugin itself, and then update the addons-linter to use an updated version of eslint-plugin-no-unsanitized.

I took a look into it and it seems that the reason for the error is because of how the rule is handling a "CallExpression that has as callee an AssignmentExpression with a MemberExpression on the right", e.g. (e = this.n[n.i])(i, r), I just opened #127 with some changes I've been trying locally to handling this scenario (the PR also adds a couple of test cases, and I verified locally that the addons-linter linting error isn't trigger anymore in a build that includes #127).

rpl commented 4 years ago

I just filed mozilla/addons-linter#3202 to track this issue also on the addons-linter repo.