Closed Bug 408328 Opened 17 years ago Closed 17 years ago

Replace FeedWriter.js's CheckLoadURIStr with CheckLoadURIStrWithPrincipal

Categories

(Firefox Graveyard :: RSS Discovery and Preview, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 3 beta3

People

(Reporter: philor, Assigned: philor)

References

()

Details

Attachments

(1 file)

Passing a principal gets better results, the withoutprincipal versions need to go away, if you haven't heard this song before you just haven't been paying attention.

The fact that we want to pretend that we're the original window, not the scary mixture of chrome and content we really are, ought to make it more fun, and seems likely to make it more of a case where we'd rather explicitly pass the right principal rather than hope getCodebasePrincipal does the right thing.
Flags: blocking-firefox3?
Fun, fun. Since the "this._window, this._document" stuff acts like it's saving off the state before the "you got your chrome in my content!" stage, I naively thought that this._document.nodePrincipal would be just the ticket. Unfortunately, while it gives me the results I want and expect, it tells the error console that "Content at about:feeds may not load or link to file:///var/." which not only isn't what I want it saying, it makes me think I'm likely to get the wrong answer to some question I'm not thinking to ask.

I sure hope someone who understands this plans on taking this bug.
bz: can we get an opine, here?
On what, exactly?
If one had to guess (and one quite often does), "bz: can we get an opinion on whether this should block Fx 3, or more particularly, have you given up on getting bug 327244 into 1.9 yet?" would be a likely guess at the desired opinion.

While it hasn't helped me figure out what to do about it here, I did finally see where the about:feeds principal, which apparently seemed odd to you in the test for bug 395533, comes from: FeedConverter.handleResult creates a channel for about:feeds, which in http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/browser/components/feeds/src/nsAboutFeeds.cpp&rev=1.1&mark=57-74#45 creates a channel for the chrome URI of the subscribe.xhtml page, and calls SetOwner on it with an about:feeds principal.
I really have no idea on bug 327244.  I'd love to get it in, since it's likely to improve our security story, but it's sort of out of my hands now...
Assignee: nobody → philringnalda
Attached patch Fix v.1Splinter Review
One sure way to duck the "blocking?" question: just fix it.
Attachment #294397 - Flags: superreview?(bzbarsky)
Attachment #294397 - Flags: review?(sayrer)
Comment on attachment 294397 [details] [diff] [review]
Fix v.1

This looks OK if we're guaranteed that there is always a principal (e.g. if we always go through the getCodebasePrincipal codepath and an exception there makes sure we don't reach the security chek).

Otherwise we would crash when we do the security check on a null principal... perhaps we should fix CAPS to not do that.
Attachment #294397 - Flags: superreview?(bzbarsky) → superreview+
We do always go through FeedWriter.Init(), but I don't see where we crash with a null principal: I thought that the NS_ENSURE_ARG_POINTER(aPrincipal) in CheckLoadURIWithPrincipal was supposed to protect against that. And indeed, if I just comment out the line setting this._feedPrincipal, I don't crash, I just get what I would expect, nothing at all linked in the preview page because every security check call fails.
Attachment #294397 - Flags: review?(sayrer) → review+
> I thought that the NS_ENSURE_ARG_POINTER(aPrincipal) in
> CheckLoadURIWithPrincipal

Ah, I'd missed it because it came after the various asserts.  OK.
Attachment #294397 - Flags: approval1.9?
Status: NEW → ASSIGNED
Blocks: 368464
Attachment #294397 - Flags: approval1.9? → approval1.9+
browser/components/feeds/src/FeedWriter.js 1.48
browser/components/feeds/Makefile.in 1.3
browser/components/feeds/test/Makefile.in 1.1
browser/components/feeds/test/test_bug408328.html 1.1
browser/components/feeds/test/bug408328-data.xml 1.1
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: blocking-firefox3? → in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M11
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: