Polymer / polymer-resin

XSS mitigation for Polymer webcomponents that uses safe html type contracts
BSD 3-Clause "New" or "Revised" License
18 stars 5 forks source link

Recent closure compiler version reports error: ERROR - Variable referenced before declaration: b #7

Closed davido closed 5 years ago

davido commented 5 years ago

After the upgrade of closure-compiler from v20180805 to the version v20190325 in rules_closure in https://github.com/bazelbuild/rules_closure/pull/347, Gerrit Code Review cannot built any more: [1].

The workaround is to add new suppression to the JSC_REFERENCE_BEFORE_DECLARE to the closure_js_library function invocation:

diff --git a/polygerrit-ui/app/rules.bzl b/polygerrit-ui/app/rules.bzl
index f836cf5c49..443149ec58 100644
--- a/polygerrit-ui/app/rules.bzl
+++ b/polygerrit-ui/app/rules.bzl
@@ -30,6 +30,7 @@ def polygerrit_bundle(name, srcs, outs, app):
         # and remove this supression
         suppress = [
             "JSC_JSDOC_MISSING_TYPE_WARNING",
+            "JSC_REFERENCE_BEFORE_DECLARE",
             "JSC_UNNECESSARY_ESCAPE",
             "JSC_UNUSED_LOCAL_ASSIGNMENT",
         ],

The affected part of the code seems to be: for(b in a)d[b]=a[b] in https://github.com/Polymer/polymer-resin/blob/master/standalone/polymer-resin.js. FWIW, I don't see the corresponding part in https://github.com/Polymer/polymer-resin/blob/master/standalone/polymer-resin-debug.js.

[1] http://paste.openstack.org/show/748913.

mikesamuel commented 5 years ago

It's possible that CC optimizing goog.object.equals reordered the two loops since less work is done in the second loop, they both return the same value, and don't side-effect without proxy traps or setters.

That's pretty hand wavey though, so I'll dig in when I get a chance.

davido commented 5 years ago

@mikesamuel Thanks for looking into it.

paladox commented 5 years ago

I belive that polymer-resin is broken with polymer 1.x now :( (Or rather closure compiler broke it).

I am seeing alot of this

Failed to sanitize attribute of <li>: <li aria-label="
"2222"
">

Though i cannot reproduce this on https://gerrit-review.googlesource.com/c/plugins/owners/+/223319 (which makes me think they are using a different version of resin?)

You can reproduce this at https://gerrit.git.wmflabs.org/r/c/All-Projects/+/2946

paladox commented 5 years ago

resin 2.0 fixes it. Im going to upgrade gerrit to resin 2.0.