Closed Bug 424923 Opened 16 years ago Closed 16 years ago

Remove Cross-Site XHR

Categories

(Core :: XML, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9beta5

People

(Reporter: sicking, Assigned: sicking)

References

Details

(Keywords: addon-compat, relnote)

Attachments

(2 files, 1 obsolete file)

Attached patch Back it out (obsolete) — Splinter Review
      No description provided.
Flags: blocking1.9+
Attachment #311514 - Flags: superreview?(jst)
Attachment #311514 - Flags: review?(jst)
Comment on attachment 311514 [details] [diff] [review]
Back it out

Jst, peterv, whoever gets to this first please r/sr.
Attachment #311514 - Flags: superreview?(jst) → superreview?(peterv)
Target Milestone: --- → mozilla1.9beta5
Uhm, since XHR has been something we've relnoted as a feature in every previous beta, could I get a cute little snippet explaining why we're removing it?
Keywords: relnote
Marking late-compat and cc:ing Team Evang as per project meeting, etc.
Keywords: late-compat
Suggested relnote: "Due to late changes in the Cross Site XMLHttpRequest specification which made our implementation incomplete, it was decided to remove support for this technology rather than include only partial support."
We should link to the changes, then -- I don't know what they are, and I bet I and others on my team will get asked!
(In reply to comment #5)
> We should link to the changes, then -- I don't know what they are, and I bet I
> and others on my team will get asked!

I believe it's encapsulated in bug 408098, but it's hard for me to tell. Jonas?
Comment on attachment 311514 [details] [diff] [review]
Back it out

r+sr=jst
Attachment #311514 - Flags: superreview?(peterv)
Attachment #311514 - Flags: superreview+
Attachment #311514 - Flags: review?(jst)
Attachment #311514 - Flags: review+
Just sticking this here for lack of a better place. I was going to post this with all the other stuff in bug 372107 to add the XS listener be part of CC since it holds other CC-classes, but since it's getting backed out I'll just put it here for reference.
We ready to go on this?
Checked in with tests.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Testing actually found one bug, though one that wasn't a result of the backout but of the initial patch.

When denying a redirect it's important to not set a new channel. Otherwise we won't recognize the channel we're getting onStartRequest/onStopRequest on and we'll lock waiting for "our" channel to stop.
Here is the final backout patch.

When putting this back in, here is what needs to be done:

* Reland this patch
* Land bug 416957
* Add support for redirects if we think there's time
* Fix the remaining security concern (apart from the cookie issue) which is that
  code that does access-checks based on document-uri rather than
  document-principal might be exploitable.
  This can either be fixed by auditing all such code, or by making the
  document-uri and document-principal match. Or by making the document-uri
  harmless.
Attachment #311514 - Attachment is obsolete: true
And add back the call to
nsXMLHttpRequest::ShutdownACCache();

in nsLayoutStatics that I forgot to diff :)
Blocks: 408098
relnote something like this:

Cross-Site XHR has been removed due to concerns for spec stability as well as wanting to attempt to make the security model for cross-site loading of private data better.
Depends on: 426308
You need to log in before you can comment on or make changes to this bug.