bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Update remaining calles of newChannel to newChannel2 in netwerk/base/nsProtocolProxyService.cpp

RESOLVED DUPLICATE of bug 1144270

Status

()

Core
DOM: Security
RESOLVED DUPLICATE of bug 1144270
3 years ago
3 years ago

People

(Reporter: ckerschb, Assigned: ckerschb)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Updated

3 years ago
Assignee: nobody → mozilla
Blocks: 1087720
Status: NEW → ASSIGNED
Depends on: 1125372
(Assignee)

Comment 1

3 years ago
Created attachment 8578827 [details] [diff] [review]
bug_1144274_protocolproxyservice.patch

Gijs, by now you know what arguments we are looking for, right? Please find a description of all the arguments here:
http://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsNetUtil.h#190

Please note that using the systemPrincipal is not that great, it would be better to find a more accurate loading principal.
Attachment #8578827 - Flags: review?(gijskruitbosch+bugs)

Comment 2

3 years ago
Comment on attachment 8578827 [details] [diff] [review]
bug_1144274_protocolproxyservice.patch

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

I am not a netwerk peer, so I really shouldn't be reviewing this, sorry.
Attachment #8578827 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 3

3 years ago
(In reply to :Gijs Kruitbosch from comment #2)
> Comment on attachment 8578827 [details] [diff] [review]
> bug_1144274_protocolproxyservice.patch
> 
> Review of attachment 8578827 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I am not a netwerk peer, so I really shouldn't be reviewing this, sorry.

Hey Gijs, the reason I asked you is because you wrote that code in bug 1125372, so I was thinking that you might be best suited to help me find the right principal to use.

Once that is done I can ask a network peer(e.g. sworkman) to sign off on the final patch.
Flags: needinfo?(gijskruitbosch+bugs)

Comment 4

3 years ago
Comment on attachment 8578827 [details] [diff] [review]
bug_1144274_protocolproxyservice.patch

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

::: netwerk/base/nsProtocolProxyService.cpp
@@ +1287,5 @@
>          }
>  
>          // make a temporary channel from the URI
> +        nsCOMPtr<nsIScriptSecurityManager> secMan(
> +            do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv));

Hrmpf. Wonder if you could get this added to the C++ mozilla::services instead. (add a macro in ServiceList.h). Maybe that should be a followup bug, I don't know how difficult it is to get things in there and/or how people deal with that.

@@ +1295,5 @@
> +        NS_ENSURE_SUCCESS(rv, rv);
> +
> +        rv = NS_NewChannel(getter_AddRefs(channel),
> +                           uri,
> +                           systemPrincipal,

Right... I don't think there's any way to get a better principal here - all we get is a URI. We should audit callers in-tree inasmuch as they haven't been already to just pass channels instead.
Attachment #8578827 - Flags: feedback+

Updated

3 years ago
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 5

3 years ago
Just chatted with sworkman on IRC, it seems we can combine this patch with the patch in bug 1144270. Hence marking this as a duplicate.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1144270
You need to log in before you can comment on or make changes to this bug.