424923 - Remove Cross-Site XHR

Status: RESOLVED FIXED

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Created attachment 311514 [details] [diff] [review]
Back it out

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 311514 [details] [diff] [review]
Back it out

Jst, peterv, whoever gets to this first please r/sr.

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

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?

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Marking late-compat and cc:ing Team Evang as per project meeting, etc.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

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."

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

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!

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

(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 7

[Johnny Stenback (:jst)]

Comment on attachment 311514 [details] [diff] [review]
Back it out

r+sr=jst

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Created attachment 311707 [details] [diff] [review]
Add CrossSite listener to CC

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.

Comment 9

[Mike Schroepfer]

We ready to go on this?

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in with tests.

Comment 11

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Created attachment 311719 [details] [diff] [review]
Final backout patch

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.

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

And add back the call to
nsXMLHttpRequest::ShutdownACCache();

in nsLayoutStatics that I forgot to diff :)

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.