Closed Bug 1062529 Opened 10 years ago Closed 10 years ago

Split GetChannelPrincipal into GetChannelResultPrincipal and GetChannelURIPrincipal

Categories

(Core :: DOM: Security, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: ckerschb, Assigned: ckerschb)

References

Details

Attachments

(1 file)

Initially, the patch for this bug was part of Bug 1038756 - Extending LoadInfo and storing LoadInfo on all channels created through NS_NewChannel.

Since it might take a little longer till Bug 1038756 is ready to land, we can take that independent (and already reviewed) part and land it ahead of time.
Blocks: 1038756
Blocks: 936814
Pushing that part to try to make sure everything is fine:

> You can view the progress of your build at the following URL:
> https://tbpl.mozilla.org/?tree=Try&rev=054997d3e427
> Alternatively, view them on Treeherder (experimental):
> https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=054997d3e427
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #2)
> Created attachment 8483745 [details] [diff] [review]
> bug_1062529_split_getchannelprincipal.patch
> 
> Carrying over r+ from bz, see:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1038756#c178

This looks good to me; can someone check that in for me please?
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c00ae6dd85ec
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
The patch here does more than just rename things, but there's no information about *why* this change was made. I think it was a good change, but it would be good to have documented why we did it.

Specifically, the behavioral change that was made was that the CORS code now calls GetChannelURIPrincipal, whereas it used to call GetChannelPrincipal (i.e. the same thing as the new GetChannelResultPrincipal)

I.e. why is it important that the CORS code uses GetChannelURIPrincipal? What problem was that fixing?
(In reply to Jonas Sicking (:sicking) from comment #6)
> The patch here does more than just rename things, but there's no information
> about *why* this change was made. I think it was a good change, but it would
> be good to have documented why we did it.
> 
> Specifically, the behavioral change that was made was that the CORS code now
> calls GetChannelURIPrincipal, whereas it used to call GetChannelPrincipal
> (i.e. the same thing as the new GetChannelResultPrincipal)
> 
> I.e. why is it important that the CORS code uses GetChannelURIPrincipal?
> What problem was that fixing?

We should have created that bug earlier, now all the conversation, discussion and documentation still lives in Bug 1038756 which turned out to have 250+ comments. Anyway, the interesting part about CORS starts here:
  https://bugzilla.mozilla.org/show_bug.cgi?id=1038756#c24
Chris, can you add comments to nsScriptSecurityManager with a summary?
(In reply to Tanvi Vyas [:tanvi] from comment #8)
> Chris, can you add comments to nsScriptSecurityManager with a summary?

Sure, doing that over in bug 1134096.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: