Closed Bug 1206964 Opened 5 years ago Closed 5 years ago

Use channel->AsyncOpen2() in netwerk/base/nsNetUtil.cpp

Categories

(Core :: DOM: Security, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: ckerschb, Assigned: ckerschb)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Assignee: nobody → mozilla
Blocks: 1182535
Status: NEW → ASSIGNED
Attachment #8682118 - Flags: review?(jonas)
Comment on attachment 8682118 [details] [diff] [review]
bug_1206964_asyncopen2_nsnetutil.patch

Review of attachment 8682118 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/xul/XULDocument.cpp
@@ +3258,5 @@
> +    //                         false);
> +    // if (NS_FAILED(rv)) {
> +    //   *aBlock = false;
> +    //   return rv;
> +    // }

I'd say you can remove this if you change add a |isChromeDoc &&| condition to this line:

http://mxr.mozilla.org/mozilla-central/source/dom/xul/XULDocument.cpp#3269

If so we'll always go through the normal AsyncOpen2 call for non-chrome documents.

@@ +3286,5 @@
>                                  aScriptProto->mSrcURI,
>                                  this, // aObserver
>                                  this, // aRequestingContext
> +                                nsILoadInfo::SEC_REQUIRE_SAME_ORIGIN_DATA_INHERITS,
> +                                nsIContentPolicy::TYPE_OTHER, // should that be some other contentpolicyType??

Might as well change this to use TYPE_SCRIPT_INTERNAL. We'll probably never call nsIContentPolicy here anyway since this will generally be called with a system principal.

::: netwerk/base/nsNetUtil.cpp
@@ +582,5 @@
> +/**
> + * Helper class used only within NS_NewStreamLoaderInternal
> + * to proxy the context when calling AsyncOpen2()
> + */
> +class ContextProxy : public nsIStreamLoaderObserver

Don't do this in nsNetUtil.cpp. Are any callsites actually using the context argument? If so, make them do something like this instead.
Attachment #8682118 - Flags: review?(jonas) → review-
Jonas, as discussed on IRC we can actually remove the 'aContext' argument completely.
Attachment #8682118 - Attachment is obsolete: true
Attachment #8682275 - Flags: review?(jonas)
https://hg.mozilla.org/mozilla-central/rev/c6eb37d7c93e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Depends on: 1221067
Blocks: 1221067
No longer depends on: 1221067
You need to log in before you can comment on or make changes to this bug.