Closed
Bug 394390
Opened 17 years ago
Closed 17 years ago
Cross-site XMLHttpRequest reports security errors to the console even when successful
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
VERIFIED
FIXED
mozilla1.9beta1
People
(Reporter: ted, Assigned: suryaismail)
References
Details
Attachments
(2 files)
423 bytes,
text/html
|
Details | |
14.79 KB,
patch
|
sicking
:
review+
sicking
:
superreview+
mtschrep
:
approvalM9+
|
Details | Diff | Splinter Review |
Performing a cross-site-XMLHttpRequest reports multiple errors to the error console, even when the request is successful. The attached testcase tries to load http://tinderbox.mozilla.org/showbuilds.cgi?tree=Firefox&json=1 (which sends Content-Access-Control: allow <*>) via XHR. I see the following error three times: Security Error: Content at ... may not load data from http://tinderbox.mozilla.org/showbuilds.cgi?tree=Firefox&json=1.
Updated•17 years ago
|
Summary: cross-site-XMLHttpRequest reports errors to the console even when successful → Cross-site XMLHttpRequest reports security errors to the console even when successful
This is due to the security manager being too happy about reporting errors just for a simple same-origin check. We could add a flag indicating weather an error should be reported or not.
please check "Issue I" of Bug 395625 is same as this
Comment 3•17 years ago
|
||
> We could add a flag indicating weather an error should be reported or not.
Or we could use the API that doesn't report errors: SecurityCompareURIs. Or equivalent.
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9+
Assignee: nobody → jonas
Syryai, this should be an easy bug to fix. Currently the CheckSameOrigin API i'm calling here: http://lxr.mozilla.org/mozilla/source/content/base/src/nsCrossSiteListenerProxy.cpp#136 warns in the error console that a security check failed. Comment 1 and comment 3 suggests alternative solutions that avoids that problem. Let me know if you have any questions.
Oops, forgot to assign the bug as well. See comment above.
Assignee: jonas → suryaismail
Assignee | ||
Updated•17 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•17 years ago
|
||
Followed comment #1 and added a "boolean reportError" flag to CheckSameOriginURI. Security errors are gone. But there's still a JS error "Javascript error: , line 0: uncaught exception: [Exception... "Component returned failure code: 0x8004005 (NS_ERROR_FAILURE) [nsIXMLHttpRequest.send]" ....." Firefox 2 seems to do the same thing. Is this correct behaviour?
Reporter | ||
Comment 7•17 years ago
|
||
Are you seeing that error loading the testcase here? I don't see that error (but I do see the security errors) using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007092605 Minefield/3.0a9pre.
Comment 8•17 years ago
|
||
I'm still not sure why we're adding API when we already have an API that does this... especially since as I recall the plan is to change all this api stuff anyway to have a "principal same origin as uri" function, so using SecurityCompareURIs temporarily is ok... If we da change the API, we need to change the iid.
SecurityCompareURIs isn't exposed in any public API. We could do that of course, but that's as much adding API as the flag, no? I don't care much either way though, so if bz prefer that we expose on the idl instead I'm fine with that.
Comment 10•17 years ago
|
||
> SecurityCompareURIs isn't exposed in any public API. Uh... right. Bug 327243. How could I forget? ;) In that case, adding the flag is the way to go, yeah. Though I still think we want that "compare principal to URI" API here.
Comment 11•17 years ago
|
||
In fact, I said pretty much all that in bug 327243. If we really don't have time to do the right thing here, I guess adding the boolean to the existing API is fine. But if we do, I'd rather we got our API right.
Assignee | ||
Comment 12•17 years ago
|
||
CheckSameOrigin only adds 4-5 lines of error reporting code to SecurityCompareURIs. Since they're so similar, I think it'd be easier to document/maintain just one API call. I'd vote to just remove the error reporting code from CheckSameOrigin. That avoids the idl change. I think that's what bz said for bug 327243. Am I missing any side-effects of removing the error reporting code?
Hmm.. I do in fact have a patch sitting around that adds a compare-principal-to-uri API. Not sure what state it's in though. Bz, in what cases do you think we do want to have this reported to the console? I suspect the answer will affect my patch too.
Comment 14•17 years ago
|
||
I think we want to have it reported to the console any time the check actually blocks something from happening altogether (as opposed to just changing the codepath followed, as here).
Comment on attachment 283523 [details] [diff] [review] Proposed patch This would actually be awesome to get in for beta since this is something a lot of people have been complaining about when first starting to play with cross site xhr. The patch is very safe so i'm not at all worried about regressions. The only argument for not taking this patch is that we have a better one, but that one isn't safe enough for beta so I'd say lets take this for beta to enable better cross-site xhr testing.
Attachment #283523 -
Flags: superreview+
Attachment #283523 -
Flags: review+
Attachment #283523 -
Flags: approvalM9?
Comment 16•17 years ago
|
||
Comment on attachment 283523 [details] [diff] [review] Proposed patch a+ schrep since this makes a web content feature a little more useful for b1
Attachment #283523 -
Flags: approvalM9? → approvalM9+
Comment 17•17 years ago
|
||
jonas checked-in this patch at 2007-10-26 18:46. Should this bug be resolved as FIXED and a new bug opened for the past-beta fix?
Target Milestone: --- → mozilla1.9 M9
Comment 18•17 years ago
|
||
Marking FIXED. Please open a new bug if there is to be a different fix.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 19•17 years ago
|
||
I do not see the error message that luser mentions in the initial report, but I still see the error that was noted in Comment 6. Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIXMLHttpRequest.send]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: https://bugzilla.mozilla.org/attachment.cgi?id=279033 :: tryXHR :: line 7" data: no] Would like to get clarification if that is expected before verifying. Thanks.
Reporter | ||
Comment 20•17 years ago
|
||
I think the testcase got broken because the spec changed. :-/
Comment 21•17 years ago
|
||
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a9pre) Gecko/2007103004 Minefield/3.0a9pre. No security errors in the console when loading the attachment. Comment 20 addresses the issue with the error that is coming up in the console.
Status: RESOLVED → VERIFIED
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•