✨ amp-script: allow 3p scripts without csp hash if sandboxed#33643
✨ amp-script: allow 3p scripts without csp hash if sandboxed#33643samouri merged 11 commits intoampproject:mainfrom
Conversation
d71d334 to
447ccbc
Compare
c694612 to
ee47964
Compare
|
Hey @ampproject/wg-caching! These files were changed: |
3ac3f30 to
356566e
Compare
253e6ce to
00e9173
Compare
|
Added @rsimha to review |
| onReceiveMessage: (data) => { | ||
| dev().info(TAG, 'From worker:', data); | ||
| }, | ||
| sandbox: this.sandboxed_ && {iframeUrl}, |
There was a problem hiding this comment.
Nothing yet, its brand new.
Soon to be Permutive: #30193
| ); | ||
| const text = local.textContent; | ||
| if (this.development_) { | ||
| if (this.development_ || this.sandboxed_) { |
There was a problem hiding this comment.
Setting sandboxed attribute will skip validation, but nothing handles the new sandboxed behavior. So, essentially, it lets you skip the validation only. Doesn't this need to wait, or do something so that we don't treat a sandboxed script as if it's just regular?
There was a problem hiding this comment.
I'm confused. We don't treat it as regular. We pass worker-dom the sandboxed: {iframeUrl} configuration. This informs it to create a worker inside of an iframe instead of a Worker off the main window.
There was a problem hiding this comment.
Does that work currently?
|
@Gregable: when you get a chance (no rush) can you review the validator changes? Thanks! |
…ect#33643) * amp-script: allow scripts without csp hash if sandboxed * It works * Remove sha check * Dist build working + Lint * add integ test * update validator * greg feedback * fix integ test url * Add extra comment in html file * use esm for esm builds * lint
…ect#33643) * amp-script: allow scripts without csp hash if sandboxed * It works * Remove sha check * Dist build working + Lint * add integ test * update validator * greg feedback * fix integ test url * Add extra comment in html file * use esm for esm builds * lint
summary
Fixes: #30193. The relevant
worker-domchanges are here: ampproject/worker-dom#1042.dist.3pcalledamp-script-proxy-iframe.jssandboxedattribute for<amp-script>and allows it in the validator. When present, csp is ignored.