Closed
Bug 817531
Opened 12 years ago
Closed 12 years ago
Wrong Referer is sent for table background images
Categories
(Core :: CSS Parsing and Computation, defect)
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: mkrinke, Assigned: bzbarsky)
References
Details
(Keywords: regression)
Attachments
(2 files)
145.23 KB,
image/jpeg
|
Details | |
5.82 KB,
patch
|
khuey
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID: 20121120062532
Steps to reproduce:
Visited http://www.fotocommunity.com/contest
Actual results:
Background images for the tables (Different photo contests listed) are not beeing loaded.
Expected results:
Requests for those images are beeing made and result in a 403 since a referer check is made for those requests and the referer beeing sent along with those requests is the request url (for the image asset) itself, not the url for the website including those images.
This seems to be introduced in FF 17, we verified it on Mac, Linux and Windows.
In FF 16 it still worked.
just wanted to add example html in question:
<table width="100%" border="0" background="http://cdn.fotocommunity.com/photos/27941525.jpg" height="120" cellspacing="5" cellpadding="0">
Comment 2•12 years ago
|
||
>Background images for the tables (Different photo contests listed) are not beeing
>loaded.
Either I'm missing something our your steps to reproduce are incomplete.
The page http://www.fotocommunity.com/contest looks identical for me in IE9, FF17 and Seamonkey trunk.
Please include steps to reproduce, step by step and attach a screenshot
Is it possible that I have to be logged in ?
Comment 3•12 years ago
|
||
Matthias,
sorry, but it seems we already deployed a fix for this on our side using css.
I put up some example html: http://www.fotocommunity.net/ff-bug-817531.html
In Firebug you can validate the the Referer beeing sent with the request for http://cdn.fotocommunity.com/photos/27941525.jpg is "http://cdn.fotocommunity.com/photos/27941525.jpg", not the URL to the main request.
Thanks,
Moritz
Flags: needinfo?(mkrinke)
Comment 5•12 years ago
|
||
Firefox17: Referer: http://cdn.fotocommunity.com/photos/27941525.jpg
Firefox10ESR: Referer: http://www.fotocommunity.net/ff-bug-817531.html
We have 2 regressions here.
The first regression is that we stop loading the image over the net
Last good nightly: 2012-08-15
First bad nightly: 2012-08-16
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=86ee4deea55b&tochan
ge=50e4ff05741e
I suspect bug 697230
Some builds later we start to load the image again but with the wrong referer header
Last good nightly: 2012-08-24
First bad nightly: 2012-08-25
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1c0ac073dc65&tochan
ge=f077de66e52d
I suspect bug 783162
Blocks: 783162
Status: UNCONFIRMED → NEW
Component: Untriaged → Style System (CSS)
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
Assignee | ||
Comment 6•12 years ago
|
||
Ah, the wrong referrer thing is definitely from bug 783162. This part in nsGenericHTMLElement::ParseBackgroundAttribute:
+ mozilla::css::URLValue *url =
+ new mozilla::css::URLValue(buffer, baseURI, uri, NodePrincipal());
should be:
mozilla::css::URLValue *url =
new mozilla::css::URLValue(uri, buffer, doc->GetDocumentURI(), NodePrincipal());
I think.
Assignee | ||
Comment 7•12 years ago
|
||
Attachment #688325 -
Flags: review?(khuey)
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Attachment #688325 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 8•12 years ago
|
||
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla20
Assignee | ||
Comment 9•12 years ago
|
||
Comment on attachment 688325 [details] [diff] [review]
Fix the referrer header for background image loads.
[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 783162.
User impact if declined: Referrer for background images via HTML "background"
attributes is the image itself, not the document, possibly breaking some sites.
Testing completed (on m-c, etc.): Passes automated test.
Risk to taking this patch (and alternatives if risky): Risk should be very low.
String or UUID changes made by this patch: None
Attachment #688325 -
Flags: approval-mozilla-beta?
Attachment #688325 -
Flags: approval-mozilla-aurora?
Comment 10•12 years ago
|
||
Backed out for:
REFTEST TEST-UNEXPECTED-FAIL | http://10.250.48.232:30278/tests/layout/reftests/backgrounds/background-referrer.html | image comparison (==), max difference: 255, number of differing pixels: 800000
https://tbpl.mozilla.org/php/getParsedLog.php?id=17684273&tree=Mozilla-Inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/dee7d08baf04
Assignee | ||
Comment 11•12 years ago
|
||
I'm going to reland with the test disabled on android, for now, and work on debugging why the test fails there.
Assignee | ||
Comment 12•12 years ago
|
||
And it's a bit worrisome that I never got bugmail for comment 10. :(
Assignee | ||
Comment 13•12 years ago
|
||
Assignee | ||
Comment 14•12 years ago
|
||
Looks like Android uses a different hostname. Fixed the test to deal with that and reenabled it on Android: https://hg.mozilla.org/integration/mozilla-inbound/rev/732f04efe8f5
Comment 15•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a381a84d7af2
https://hg.mozilla.org/mozilla-central/rev/732f04efe8f5
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 16•12 years ago
|
||
Comment on attachment 688325 [details] [diff] [review]
Fix the referrer header for background image loads.
low risk regression fix, approving.
Attachment #688325 -
Flags: approval-mozilla-beta?
Attachment #688325 -
Flags: approval-mozilla-beta+
Attachment #688325 -
Flags: approval-mozilla-aurora?
Attachment #688325 -
Flags: approval-mozilla-aurora+
Comment 17•12 years ago
|
||
Comment 18•12 years ago
|
||
Tracking for QA.
status-firefox17:
--- → affected
tracking-firefox18:
--- → ?
tracking-firefox19:
--- → ?
tracking-firefox20:
--- → ?
Updated•12 years ago
|
Updated•12 years ago
|
OS: Linux → All
Comment 19•12 years ago
|
||
I confirm the fix is verified on MacOS 10.8, Windows 7 x64 and Ubuntu 12.04 on FF 20RC:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20100101 Firefox/20.0
You need to log in
before you can comment on or make changes to this bug.
Description
•