Closed
Bug 1242472
Opened 9 years ago
Closed 9 years ago
Tracking protection: Tracking pixel not blocked in e10s
Categories
(Toolkit :: Safe Browsing, defect)
Tracking
()
RESOLVED
FIXED
mozilla47
People
(Reporter: mwobensmith, Assigned: mrbkap)
References
()
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
application/x-javascript
|
Details | |
(deleted),
patch
|
ckerschb
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
On cnn.com's home page, a tracking pixel from a domain on the Disconnect blocklist loads, but only when e10s is enabled.
This appears to be the behavior from Fx42 onwards.
If e10s is disabled, the resource is correctly blocked.
Resource URL:
http://b.scorecardresearch.com/r?c2=6035748&d.c=gif&d.o=cnn-adbp-intl&d.x=38362791&d.t=page&d.u=http%3A%2F%2Fedition.cnn.com%2F
Updated•9 years ago
|
Assignee: nobody → mrbkap
tracking-e10s:
--- → m8+
Comment 1•9 years ago
|
||
I spent a few hours debugging this with NSPR_LOG_MODULES="nsHttp:5" and also looking at the Initiator column in the Chrome devtools.
I found that the offending gif from scorecardresearch is the result of a 302:
[0x7fbf7776a340]: I/nsHttp http response [
[0x7fbf7776a340]: I/nsHttp HTTP/1.1 302 Found
[0x7fbf7776a340]: I/nsHttp Date: Mon, 25 Jan 2016 18:57:06 GMT
[0x7fbf7776a340]: I/nsHttp Server: Omniture DC/2.0.0
[0x7fbf7776a340]: I/nsHttp Access-Control-Allow-Origin: *
[0x7fbf7776a340]: I/nsHttp Set-Cookie: s_vi=[CS]v1|2B533801051D3D22-4000190F8000197D[CE]; Expires=Wed, 24 Jan 2018 18:57:06 GMT; Domain=cnn.com; Path=/
[0x7fbf7776a340]: I/nsHttp X-C: ms-4.9.10
[0x7fbf7776a340]: I/nsHttp Expires: Sun, 24 Jan 2016 18:57:06 GMT
[0x7fbf7776a340]: I/nsHttp Last-Modified: Tue, 26 Jan 2016 18:57:06 GMT
[0x7fbf7776a340]: I/nsHttp Cache-Control: no-cache, no-store, max-age=0, no-transform, private
[0x7fbf7776a340]: I/nsHttp Pragma: no-cache
[0x7fbf7776a340]: I/nsHttp Etag: "56A67002-9294-1E2388CD"
[0x7fbf7776a340]: I/nsHttp Vary: *
[0x7fbf7776a340]: I/nsHttp P3P: policyref="/w3c/p3p.xml", CP="NOI DSP COR NID PSA OUR IND COM NAV STA"
[0x7fbf7776a340]: I/nsHttp Location: http://b.scorecardresearch.com/r?c2=6035748&d.c=gif&d.o=cnn-adbp-domestic&d.x=48376774&d.t=page&d.u=http%3A%2F%2Fwww.cnn.com%2F
[0x7fbf7776a340]: I/nsHttp xserver: www931
[0x7fbf7776a340]: I/nsHttp Content-Length: 0
[0x7fbf7776a340]: I/nsHttp Keep-Alive: timeout=15
[0x7fbf7776a340]: I/nsHttp Connection: Keep-Alive
[0x7fbf7776a340]: I/nsHttp Content-Type: text/plain
[0x7fbf7776a340]: I/nsHttp ]
I'm not 100% sure, but I think the source of that 302 is another 302:
[0x7fbf7776a340]: D/nsHttp nsHttpTransaction::HandleContentStart [this=7fbf3e721000]
[0x7fbf7776a340]: I/nsHttp http response [
[0x7fbf7776a340]: I/nsHttp HTTP/1.1 302 Found
[0x7fbf7776a340]: I/nsHttp Date: Mon, 25 Jan 2016 18:57:06 GMT
[0x7fbf7776a340]: I/nsHttp Server: Omniture DC/2.0.0
[0x7fbf7776a340]: I/nsHttp Access-Control-Allow-Origin: *
[0x7fbf7776a340]: I/nsHttp Set-Cookie: s_vi=[CS]v1|2B533801051D3D22-4000190F8000197D[CE]; Expires=Wed, 24 Jan 2018 18:57:06 GMT; Domain=cnn.com; Path=/
[0x7fbf7776a340]: I/nsHttp Location: http://metrics.cnn.com/b/ss/cnn-adbp-domestic/1/H.26.1/s61817340381443?AQB=1&pccr=true&vidn=2B533801051D3D22-4000190F8000197D&&ndh=1&t=25%2F0%2F2016%2010%3A57%3A5%201%20480&fid=1813C5000C9ECF0C-3B9B8DB4EACA3F44&ce=UTF-8&ns=cnn&pageName=cnn%3Ain%3A%2F&g=http%3A%2F%2Fwww.cnn.com%2F&cc=USD&ch=cnn%20homepage&server=cnn.com&events=event26&c5=nvs&v5=D%3Dc5&c8=new%3A1&v8=D%3Dc8&c9=nvs&v9=D%3Dc9&c13=section&v13=D%3Dc13&c17=anonymous&v17=D%3Dc17&c20=11&v20=D%3Dc20&v22=0&c25=nvs&v25=D%3Dc25&c26=www.cnn.com%2F&v26=D%3DpageName&v27=D%3Dch&c28=cnn%20homepage%3Anvs&v28=D%3Dc28&v29=cnn.com&c30=cnn%20domestic&v30=D%3Dc30&c32=adbp%3Aindex&v32=D%3Dc32&c33=none&v33=D%3Dc33&c35=cnnexpan.126.5518.20160112%3A0&v35=D%3Dc35&c37=desktop&v37=D%3Dc37&c46=14537482252292655977683703&v46=D%3Dc46&c47=56a670000d1a180a3c8ef73f32008596&v47=D%3Dc47&c56=portrait&v56=D%3Dc56&c64=cnn%20news&v64=D%3Dc64&c65=nvs&c73=956%20X%201086&v73=D%3Dc73&h1=news%7Ccnn%7Ccnn%20domestic%7Ccnn.com%7Ccnn%20homepage%7Ccnn%20homepage%3Anvs&l1=D%3Dc65&s=1920x1200&c=24&j=1.8.5&v=N&k=Y&bw=956&bh=1086&AQE=1
[0x7fbf7776a340]: I/nsHttp X-C: ms-4.9.10
[0x7fbf7776a340]: I/nsHttp Expires: Sun, 24 Jan 2016 18:57:06 GMT
[0x7fbf7776a340]: I/nsHttp Last-Modified: Tue, 26 Jan 2016 18:57:06 GMT
[0x7fbf7776a340]: I/nsHttp Cache-Control: no-cache, no-store, max-age=0, no-transform, private
[0x7fbf7776a340]: I/nsHttp Pragma: no-cache
[0x7fbf7776a340]: I/nsHttp P3P: policyref="/w3c/p3p.xml", CP="NOI DSP COR NID PSA OUR IND COM NAV STA"
[0x7fbf7776a340]: I/nsHttp xserver: www924
[0x7fbf7776a340]: I/nsHttp Content-Length: 0
[0x7fbf7776a340]: I/nsHttp Keep-Alive: timeout=15
[0x7fbf7776a340]: I/nsHttp Connection: Keep-Alive
[0x7fbf7776a340]: I/nsHttp Content-Type: text/plain
[0x7fbf7776a340]: I/nsHttp ]
I couldn't actually find the source of that one but I found metrics.cnn.com in none of the CSS or HTML files loaded from the CNN homepage and only in a single JS file. It appears several times in
http://z.cdn.turner.com/analytics/cnnexpan/jsmd.min.js which is loaded by a script element on the CNN homepage:
<script src="http://z.cdn.turner.com/analytics/cnnexpan/jsmd.min.js"></script>
Attached is the version of jsmd.min.js that I ran through http://jsbeautifier.org/.
Assignee | ||
Comment 2•9 years ago
|
||
I noticed this when debugging.
Christoph, I hope you don't mind reviewing this. I don't know of a better reviewer.
Attachment #8713439 -
Flags: review?(mozilla)
Assignee | ||
Comment 3•9 years ago
|
||
I haven't written a test for this yet, but comment 1 is right on – we lose our top window URI somewhere in the redirect chain and the tracking protection code bails. I wonder if the topWindowURI is something that we should stick on the loadinfo instead of on the channel.
Attachment #8713443 -
Flags: review?(mozilla)
Comment 4•9 years ago
|
||
Comment on attachment 8713439 [details] [diff] [review]
Add missing newlines to log messages
Review of attachment 8713439 [details] [diff] [review]:
-----------------------------------------------------------------
The Channel Classifier is part of the URL Classifier (i.e. Safe Browsing) module, so I'm happy to review these for you.
Does the newline actually matter? If so, I'm gonna have to change a lot more of these because I've never put newlines in my PR_LOG messages either :(
Attachment #8713439 -
Flags: review?(mozilla) → review+
Comment 5•9 years ago
|
||
Comment on attachment 8713443 [details] [diff] [review]
Fix
Review of attachment 8713443 [details] [diff] [review]:
-----------------------------------------------------------------
The changes look fine, and I agree, we should move the topWindowURI in the loadInfo at some point. Seems like a better fit, especially since we are already having the windowID in the loadInfo. The pixel tracking is apparently only an issue for http channels, right? Just want to make sure, because there is also the redirect within nsBaseChannel where we manually have to propagate some of the flags needed for the new channel [1].
[1] http://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsBaseChannel.cpp#73
Attachment #8713443 -
Flags: review?(mozilla) → review+
Comment 6•9 years ago
|
||
(In reply to François Marier [:francois] from comment #4)
> Does the newline actually matter? If so, I'm gonna have to change a lot more
> of these because I've never put newlines in my PR_LOG messages either :(
The logging code will add the newline for you if you don't have one. (When we switched some things over to mozilla::LazyLogModule, we forgot the newline behavior, but we quickly corrected it...)
Assignee | ||
Comment 7•9 years ago
|
||
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #5)
> apparently only an issue for http channels, right? Just want to make sure,
> because there is also the redirect within nsBaseChannel where we manually
> have to propagate some of the flags needed for the new channel [1].
Yeah, we only have mTopWindowURI on HttpBaseChannel so there's nothing to propagate in the other SetupRedirectChannel impl.
(In reply to François Marier [:francois] from comment #4)
> Does the newline actually matter? If so, I'm gonna have to change a lot more
> of these because I've never put newlines in my PR_LOG messages either :(
I was misreading the existing logging output, I've deleted this changeset locally.
Assignee | ||
Updated•9 years ago
|
Attachment #8713439 -
Attachment is obsolete: true
Assignee | ||
Comment 8•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc1de8384bddd778576e1c90fa4c595900edded8
Bug 1242472 - Properly propagate mTopWindowURI through redirects. r=francois/ckerschb
Comment 9•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Is this still an m8 blocker? Should it uplift to 46? Thanks.
status-firefox46:
--- → affected
Flags: needinfo?(mrbkap)
Assignee | ||
Comment 11•9 years ago
|
||
Comment on attachment 8713443 [details] [diff] [review]
Fix
Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: Users with tracking protection enabled will still be able to be tracked.
[Describe test coverage new/current, TreeHerder]: The feature has a bunch of tests on treeherder.
[Risks and why]: Low risk, propagates
[String/UUID change made/needed]: n/a
Flags: needinfo?(mrbkap)
Attachment #8713443 -
Flags: approval-mozilla-aurora?
Comment on attachment 8713443 [details] [diff] [review]
Fix
Please uplift to aurora; we'd like to make sure e10s users for 46 have tracking protection working.
Attachment #8713443 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 13•9 years ago
|
||
bugherder uplift |
You need to log in
before you can comment on or make changes to this bug.
Description
•