Amphtml: Request to limit scope of functions in validator.js

Created on 25 Jan 2018  路  5Comments  路  Source: ampproject/amphtml

What's the issue?

It seems that functions in validator.js are escaping into the global namespace.

How do we reproduce the issue?

  1. Navigate to an AMP page like this, with #development=1 appended to the URL.
  2. Open the browser's console, and enter wp.
  3. Expected: wp is not defined
  4. Actual: wp is a function:
茠 wp(a){rp.call(this);this.name=a;this.value=[];this.b=!1;this.o=34}

It looks like this script leaks functions into the global namespace:

https://cdn.ampproject.org/v0/validator.js

There are other functions leaked, like aa.

Maybe that script is intended for development only, as it seems that it's not requested if #development=1 isn't appended. But it would really help us with this issue on the AMP WordPress plugin:

https://github.com/Automattic/amp-wp/issues/843

We're working on validating AMP in the WordPress editor.

What browsers are affected?

Chrome 63.0.3239.132

Which AMP version is affected?

1516337355291

Thanks a lot for your help 馃槃

/cc @amedina, @Gregable, @pbakaus

High Priority caching

Most helpful comment

I think we can get a fix out for this fairly quickly. @powdercloud has a prototype, but we need to get it through a release. Thanks for the report.

All 5 comments

I think we can get a fix out for this fairly quickly. @powdercloud has a prototype, but we need to get it through a release. Thanks for the report.

Thanks a lot, @Gregable! That would really help.

This is a high priority issue but it hasn't been updated in awhile. @Gregable Do you have any updates?

Yeah this is done. I just confirmed by looking at https://cdn.ampproject.org/v0/validator.js and verifying that the whole thing is wrapped into a javascript scope with the usual pattern. Please reopen / complain if you see further issues.

Thank You

Hi @powdercloud and @Gregable,
Thanks a lot for fixing this. As you mentioned, validator.js is wrapped in a function, and functions like wp aren't leaked.

Was this page helpful?
0 / 5 - 0 ratings