Closed
Bug 1076896
Opened 10 years ago
Closed 10 years ago
Add newChannel2 to nsIoService that allows loadinfo propagation
Categories
(Core :: DOM: Security, defect)
Core
DOM: Security
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: ckerschb, Assigned: ckerschb)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 3 obsolete files)
11.74 KB,
patch
|
ckerschb
:
review+
|
Details | Diff | Splinter Review |
Similar to Bug 1067471, and Bug 1067468, we not only have to extend the nsIProtcolHandler and nsIAboutModule by a loadInfo argument for NewChannel, but also nsIOService. In addition, newChannelFromURI2 should also take a loadInfo.
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 1•10 years ago
|
||
I think patches for (nsIProtocolHandler::newChannel2, nsIAboutModule::NewChannel including a loadinfo argument) and this one should land at the same time. Once all of those landed, we can start with the shim in bug 1087442.
Attachment #8509696 -
Flags: review?(jonas)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•10 years ago
|
||
Comment on attachment 8509696 [details] [diff] [review] shim_2_ioservice.patch As discussed in the meeting, we should pass individual arguments to the ioService and not the loadinfo. No need to review that patch, I will put up a new one incorporating that change.
Attachment #8509696 -
Flags: review?(jonas)
Assignee | ||
Comment 3•10 years ago
|
||
This is what we want, I think also defaulting to the NullPrincipal is what we want, right?
Attachment #8509696 -
Attachment is obsolete: true
Attachment #8509854 -
Flags: review?(jonas)
Comment on attachment 8509854 [details] [diff] [review] shim_2_ioservice.patch Review of attachment 8509854 [details] [diff] [review]: ----------------------------------------------------------------- r=me with that fixed. ::: netwerk/base/src/nsIOService.cpp @@ +598,5 @@ > +{ > + nsresult rv; > + nsCOMPtr<nsIPrincipal> nullPrincipal = > + do_CreateInstance("@mozilla.org/nullprincipal;1", &rv); > + NS_ENSURE_SUCCESS(rv, rv); No, loading with a nullprincipal is useless once we move security checks into AsyncOpen. So just leave this as null. @@ +679,5 @@ > +{ > + nsresult rv; > + nsCOMPtr<nsIPrincipal> nullPrincipal = > + do_CreateInstance("@mozilla.org/nullprincipal;1", &rv); > + NS_ENSURE_SUCCESS(rv, rv); Same here. @@ +723,5 @@ > +{ > + nsresult rv; > + nsCOMPtr<nsIPrincipal> nullPrincipal = > + do_CreateInstance("@mozilla.org/nullprincipal;1", &rv); > + NS_ENSURE_SUCCESS(rv, rv); And here
Attachment #8509854 -
Flags: review?(jonas) → review+
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Jonas Sicking (:sicking) from comment #4) > ::: netwerk/base/src/nsIOService.cpp > @@ +598,5 @@ > > +{ > > + nsresult rv; > > + nsCOMPtr<nsIPrincipal> nullPrincipal = > > + do_CreateInstance("@mozilla.org/nullprincipal;1", &rv); > > + NS_ENSURE_SUCCESS(rv, rv); > > No, loading with a nullprincipal is useless once we move security checks > into AsyncOpen. > > So just leave this as null. I know it's not great, but better than using systemPrincipal, right? Unfortunately we can't leave it a nullptr, because LoadInfo requires a principal in it's constructor [1]. [1] http://mxr.mozilla.org/mozilla-central/source/docshell/base/LoadInfo.cpp#26
Flags: needinfo?(jonas)
leave it null, but then make the IO service not create a loadinfo if both loadingnode and loadingprincipal is null
Flags: needinfo?(jonas)
Assignee | ||
Comment 7•10 years ago
|
||
Incorporated Jonas suggestions. Carrying over r+.
Attachment #8509854 -
Attachment is obsolete: true
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8509956 [details] [diff] [review] shim_2_ioservice.patch Now for real :-)
Attachment #8509956 -
Flags: review+
Assignee | ||
Comment 9•10 years ago
|
||
Carrying over r+. Just forgot to rev the uuid in the idl.
Attachment #8509956 -
Attachment is obsolete: true
Attachment #8509971 -
Flags: review+
Assignee | ||
Comment 10•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/400577bd3bb0
Target Milestone: --- → mozilla36
Note that this doesn't make the new newChannel*2 functions actually set the loadinfo on the created channel. That needs to be done before we start migrating callers from newChannel* to newChannel*2.
Assignee | ||
Comment 12•10 years ago
|
||
(In reply to Jonas Sicking (:sicking) from comment #11) > Note that this doesn't make the new newChannel*2 functions actually set the > loadinfo on the created channel. That needs to be done before we start > migrating callers from newChannel* to newChannel*2. Intentionally this bug just changes the interface. We are going to attach the loadInfo to the newly created channel in Bug 1087442, which blocks the bugs which are going to change callsistes of newChannel in JS.
https://hg.mozilla.org/mozilla-central/rev/400577bd3bb0
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•