- Notifications
You must be signed in to change notification settings - Fork20.6k
Ajax: Avoid CSP errors in the script transport for async requests#4763
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
I believe this is not a breaking change. Can we include it in 3.6.0? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
One thing we do lose here is the ability to set XHR stuff like headers on a script request. I would definitely consider that a breaking change but since it's set for 4.0 it's fair game. At the moment though, there doesn't seem to be an easy way to get the old behavior back and use the old transport.
src/ajax/script.js Outdated
| // These types of requests are handled via a script tag | ||
| // so force their methods to GET. | ||
| if ( s.async || s.crossDomain || s.scriptAttrs ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Did you optimize this order for gzip compression? If not, I suspect placings.crossDomain first might chop off a byte or two.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I checked various reorderings & what you propose (+ a few others) were the smallest. Smaller by... one byte compared to my original version. 😄
PR updated. +10 bytes now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM pending Krinkle's comment
Adding the |
Until now, the AJAX script transport only used a script tag to load scriptsfor cross-domain requests or ones with `scriptAttrs` set. This commit makesit also used for all async requests to avoid CSP errors arising from usageof inline scripts. This also makes `jQuery.getScript` not trigger CSP errorsas it uses the AJAX script transport under the hood.For sync requests such a change is impossible and that's what `jQuery._evalUrl`uses. Fixing that is tracked injquerygh-1895.The commit also makes other type of requests using the script tag version of thescript transport set its type to "GET", namely async scripts & ones with`scriptAttrs` set in addition to the existing cross-domain ones.Fixesjquerygh-3969
Uh oh!
There was an error while loading.Please reload this page.
Summary
Until now, the AJAX script transport only used a script tag to load scripts
for cross-domain requests or ones with
scriptAttrsset. This commit makesit also used for all async requests to avoid CSP errors arising from usage
of inline scripts. This also makes
jQuery.getScriptnot trigger CSP errorsas it uses the AJAX script transport under the hood.
For sync requests such a change is impossible and that's what
jQuery._evalUrluses. Fixing that is tracked ingh-1895.
The commit also makes other type of requests using the script tag version of the
script transport set its type to "GET", namely async scripts & ones with
scriptAttrsset in addition to the existing cross-domain ones.Fixesgh-3969
Checklist
If needed, a docs issue/PR was created athttps://github.com/jquery/api.jquery.com