1158208 - Enforce that tiles HTTP requests don't have cookies

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[Benjamin Smedberg]

Currently tiles makes the following HTTP(s) requests:

* requesting the source JSON document
* requesting the tile images
* sending the data JSON ping

Right now we promise that the servers will not set cookies for each of these requests. We should enforce on the client that we don't ever send cookies with these requests. I believe that we have a way to do this already, but I don't remember/can't find the details.

Comment 1

[Benjamin Smedberg]

Just found XHR.mozAnon, which is what we should be using.

Comment 2

[Ed Lee :Mardak]

Attachment: v1

Some reason it looks like the internal interface doesn't allow parameters but the webidl interface does:

http://mxr.mozilla.org/mozilla-central/source/dom/base/nsXMLHttpRequest.h#208

So to get the appropriate constructor, I grabbed XHR from the hidden window.

Comment 3

[Ed Lee :Mardak]

Attachment: v1.1

Update docs

Comment 4

[Drew Willcoxon :adw]

Comment on attachment 8604387 [details] [diff] [review]
v1.1

Review of attachment 8604387 [details] [diff] [review]:
-----------------------------------------------------------------

There may be a "more correct" way to do this using nsIRequest/nsIChannel and load flags like LOAD_ANONYMOUS (which I bet is what this XHR mozAnon boils down to), but this is fine with me.

::: browser/modules/DirectoryLinksProvider.jsm
@@ +347,5 @@
>    /**
> +   * Create a new XMLHttpRequest that is anonymous, i.e., doesn't send cookies
> +   */
> +  _newXHR() {
> +    // Use XHR with WebIDL that accepts extra parameters

Could you please make it more clear that only the WebIDL XHR appears to support mozAnon?

Comment 5

[Ed Lee :Mardak]

Meh. My hack isn't super useful because... there's no hidden window from xpcshell tests! ;)

Perhaps I can try/catch hidden window and fall back to createInstance. xpcshell test won't pass mozAnon == true but mochitest should....

Comment 6

[Ed Lee :Mardak]

Attachment: v1.2 broken

Just to attach this for now..

Comment 7

[Ed Lee :Mardak]

Attachment: v2 depends on bug 1163898

This depends on bug 1163898, which will probably be difficult to uplift to 40/39, so perhaps we'll just ride the trains on this functionality in 41.

Comment 8

[Drew Willcoxon :adw]

Comment on attachment 8604771 [details] [diff] [review]
v2 depends on bug 1163898

Review of attachment 8604771 [details] [diff] [review]:
-----------------------------------------------------------------

You could do both and uplift the hidden window one.  The test coverage isn't really buying much anyway.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cca3ce6947c2

Comment 10

[Ed Lee :Mardak]

To be clear, it's not entirely straightforward to just use hidden window fix because various xpcshell tests rely on XHR object existing (not just the newly added test-anonymous), but trying to access hiddenwindow.XHR throws.

Comment 11

[Ed Lee :Mardak]

ni? per https://wiki.mozilla.org/Firefox/Data_Collection

(In reply to Pulsebot from comment #9)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/cca3ce6947c2
No additional data is being collected. Firefox is adding an extra no-cookie safeguard, so updating .rst.

Comment 12

[Benjamin Smedberg]

f+

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/cca3ce6947c2