Closed
Bug 392322
Opened 16 years ago
Closed 16 years ago
XMLHttpRequest crashes on local file retrieval [@ nsCrossSiteListenerProxy::OnStartRequest]
Categories
(Core :: DOM: Core & HTML, defect, P1)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: crosson, Assigned: bent.mozilla)
References
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file, 1 obsolete file)
15.66 KB,
patch
|
sicking
:
review+
sicking
:
superreview+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7) Gecko/2007081419 Minefield/3.0a7 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7) Gecko/2007081419 Minefield/3.0a7 When getting a local file via XMLHttpRequest in a C++ component, crashes in nsCrossSiteListenerProxy::OnStartRequest with a NULL mRequestingURI Reproducible: Always Steps to Reproduce: 1. Request local file, say "resource:/config.xml" 2. 3. Actual Results: crashes in nsCrossSiteListenerProxy::OnStartRequest with a NULL mRequestingURI Expected Results: config.xml is retrieved.
![]() |
||
Comment 1•16 years ago
|
||
This is a regression from bug 389508. If the principal is the system principal, it has no URI, and things break badly.
Blocks: xxx
Status: UNCONFIRMED → NEW
Component: General → DOM: Mozilla Extensions
Ever confirmed: true
Flags: blocking1.9+
OS: Windows XP → All
QA Contact: general → general
Hardware: PC → All
![]() |
||
Comment 2•16 years ago
|
||
Jonas, want to patch? I can promise speedy reviews!
Assignee: nobody → jonas
Comment 3•16 years ago
|
||
No longer crashes it seems?
Keywords: crash,
regression
Summary: XMLHttpRequest crashes on local file retrieval → XMLHttpRequest crashes on local file retrieval [@ nsCrossSiteListenerProxy::OnStartRequest]
Version: unspecified → Trunk
![]() |
||
Comment 4•16 years ago
|
||
I don't see why it wouldn't...
Priority: -- → P1
Priority: P1 → P2
(In reply to comment #5) > *** Bug 404818 has been marked as a duplicate of this bug. *** > I saw this bug and didn't think my issue was the same because in my case there wasn't a principal to begin with.
![]() |
||
Comment 7•16 years ago
|
||
It's really the same issue. This code assumes it can get a principal with a URI, which is an incorrect assumption.
Assignee | ||
Comment 8•16 years ago
|
||
This adds a native Init method for C++ callers to specify their own principal. Callers will probably just pass the system principal.
Attachment #301419 -
Flags: superreview+
Attachment #301419 -
Flags: review?(jonas)
Attachment #301419 -
Flags: review+
Comment 9•16 years ago
|
||
This patch still crashes for me, in nsCrossSiteListenerProxy::AddRequestHeaders because the system principal (mPrincipal) has no uri.
Comment 10•16 years ago
|
||
Comment on attachment 301419 [details] [diff] [review] Patch, v1 Can you add some xpcshell tests, please, so we know what's guaranteed not to crash (especially the scenario Neil mentions) and ensure it won't regress in the future?
Assignee | ||
Comment 11•16 years ago
|
||
(In reply to comment #9) > This patch still crashes for me, in nsCrossSiteListenerProxy::AddRequestHeaders > because the system principal (mPrincipal) has no uri. Oh, sorry Neil. If you're using the system principal you're never supposed to get there, but I failed to update our IsSameOrigin function. Patch in a sec.
Assignee | ||
Comment 12•16 years ago
|
||
This should do it. Also added a (pretty simple) C++ test so we can hopefully avoid these in the future.
Attachment #301419 -
Attachment is obsolete: true
Attachment #301733 -
Flags: review?(jonas)
Assignee | ||
Comment 13•16 years ago
|
||
Comment on attachment 301733 [details] [diff] [review] Patch, v2 > static PRBool > IsSameOrigin(nsIPrincipal* aPrincipal, nsIChannel* aChannel) > ... >- NS_ENSURE_SUCCESS(rv, rv); >+ NS_ENSURE_SUCCESS(rv, PR_FALSE); Oh, and notice how that was going to return true if anything failed! Fixed those and removed a random bit of commented code.
Assignee | ||
Updated•16 years ago
|
Priority: P2 → P1
I don't understand how Neil could hit a crash with a null principal there. The nullchecks at the top of Open and Send should catch that, no? That said, I don't see where you added checks to protect against that?
Comment 15•16 years ago
|
||
(In reply to comment #14) > I don't understand how Neil could hit a crash with a null principal there. The principal isn't null; it's the system principal but it has a null uri.
Assignee | ||
Comment 16•16 years ago
|
||
Jonas, this is the real fix: - if (!(mState & XML_HTTP_REQUEST_XSITEENABLED) && - !IsSameOrigin(mPrincipal, mChannel)) { + if (IsSystemPrincipal(mPrincipal)) { + // Cchrome callers are always allowed to read from different origins. + mState |= XML_HTTP_REQUEST_XSITEENABLED; + } + else if (!(mState & XML_HTTP_REQUEST_XSITEENABLED) && + !IsSameOrigin(mPrincipal, mChannel)) { mState |= XML_HTTP_REQUEST_USE_XSITE_AC; } That change prevents us from ever hooking up an nsCrossSiteListenerProxy (which does assume that it can get a URI from its principal).
Comment on attachment 301733 [details] [diff] [review] Patch, v2 Ah, that makes sense then.
Attachment #301733 -
Flags: superreview+
Attachment #301733 -
Flags: review?(jonas)
Attachment #301733 -
Flags: review+
Assignee | ||
Comment 18•16 years ago
|
||
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 19•16 years ago
|
||
Backed out due to weird test failure, I'll investigate today.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 20•16 years ago
|
||
Checked in again.
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Flags: in-testsuite+
Comment 21•16 years ago
|
||
Bug 404818, which has been marked to be a duplicate of this bug, is not yet fixed. The attached testcase URL gives an "Unable to load schema" error, while it is working in FF2.
Comment 22•16 years ago
|
||
(In reply to comment #21) > Bug 404818, which has been marked to be a duplicate of this bug, is not yet > fixed. The attached testcase URL gives an "Unable to load schema" error, while > it is working in FF2. > I debugged this down. We aren't crashing anymore, which is good, but nsSchemaLoader::LoadAsync's call to nsXMLHttpRequest::OpenRequest (to load the schema file) is failing because nsXMLHttpRequest's mPrincipal is nsnull.
Comment 23•16 years ago
|
||
(In reply to comment #22) > I debugged this down. We aren't crashing anymore, which is good, but > nsSchemaLoader::LoadAsync's call to nsXMLHttpRequest::OpenRequest (to load the > schema file) is failing because nsXMLHttpRequest's mPrincipal is nsnull. > You will need to call the new Init method of the nsXMLHttpRequest, added by this bug, and supply the principal as an argument.
Updated•13 years ago
|
Crash Signature: [@ nsCrossSiteListenerProxy::OnStartRequest]
Updated•11 years ago
|
Component: DOM: Mozilla Extensions → DOM
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
•