Throw NetworkError for cross-origin importScripts() exceptions#166
Throw NetworkError for cross-origin importScripts() exceptions#166
Conversation
|
It does seem the wording here could be further improved since "create a script" already has "report the error" which presumably is used for parsing purposes. So it's unclear how that error could suddenly find itself outside that algorithm. Does it end up being reported twice? @Hixie any thoughts? |
|
@annevk What exactly does it mean that I'm the assignee here? |
|
@bzbarsky I was hoping you'd review it since it provides a fix (of sorts) for https://www.w3.org/Bugs/Public/show_bug.cgi?id=28961. If you have suggestions for "create a script" already reporting the exception that would be most welcome too. |
|
I couldn't get github to show me the diff for #95, so I'm not really sure what any of these terms mean any more. |
|
#95 isn't a PR; did you mean #144? https://patch-diff.githubusercontent.com/raw/whatwg/html/pull/144.diff will show the raw diff. |
|
@jdm ah, thanks. |
|
Note that the "create a script" setup has remained the same. So that reporting errors to the global while |
domenic
force-pushed
the
importscripts
branch
from
f29b22b to
04ba30e
Compare
There was a problem hiding this comment.
nit: comma after settings object
|
This LGTM. Will leave it up to you to decide if further review is needed. |
|
LGTM too, assuming something already sets the CORS-cross-origin flag for these requests correctly. |
This fixes the following issues from #164: * importScripts() can fetch “no-cors” cross-origin resources. If those fail to parse or throw an exception we shouldn’t simply rethrow that without muting it as that would leak data that <script> does not leak. This makes them throw a NetworkError instead. * This also clarifies that a NetworkError is thrown if the response is not an ok status using the Fetch terminology.
annevk
force-pushed
the
importscripts
branch
from
04ba30e to
79c2e37
Compare
|
1e865ed is the commit to master. Noticed an error in a prior commit to master just before push. |
5 participants
This fixes the following issues from #164:
fail to parse or throw an exception we shouldn’t simply rethrow that
without muting it as that would leak data that
<script>does not leak.This makes them throw a NetworkError instead.
not an ok status using the Fetch terminology.