69732 2011-10-09 13:53:42 -0700 <img crossorigin> should fail to load when CORS check fails 2011-11-18 23:59:57 -0800 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 webkit.review.bot abarth abarth bzbarsky darin japhet oldest_to_newest 480750 0 webkit.review.bot 2011-10-09 13:53:42 -0700 <img crossorigin> should fail to load when CORS check fails Requested by abarth on #webkit. 489039 1 112119 abarth 2011-10-23 10:54:59 -0700 Created attachment 112119 work in progress 489080 2 112130 abarth 2011-10-23 16:58:07 -0700 Created attachment 112130 Patch 489119 3 112130 darin 2011-10-23 20:31:21 -0700 Comment on attachment 112130 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=112130&action=review It seems a little strange to do the entire load and then report it as if it was a failure once the load is done. It seems the load should instead fail as soon as we get the HTTP header before reading any more data, loosely speaking. > Source/WebCore/loader/ImageLoader.cpp:243 > + if (m_element->fastHasAttribute(HTMLNames::crossoriginAttr) && !resource->passesAccessControlCheck(m_element->document()->securityOrigin())) { We normally do "using namespace HTMLNames" at the top of the file, and leave off the prefix. 489121 4 abarth 2011-10-23 20:33:51 -0700 > It seems a little strange to do the entire load and then report it as if it was a failure once the load is done. It seems the load should instead fail as soon as we get the HTTP header before reading any more data, loosely speaking. Yeah, we could do that. It's easier to do at this layer because it handles the cache-hit and cache-miss cases uniformly. I'd imagine the common case is for the check to pass though. 489145 5 112130 webkit.review.bot 2011-10-23 21:35:47 -0700 Comment on attachment 112130 Patch Clearing flags on attachment: 112130 Committed r98215: <http://trac.webkit.org/changeset/98215> 489146 6 webkit.review.bot 2011-10-23 21:35:52 -0700 All reviewed patches have been landed. Closing bug. 489162 7 darin 2011-10-23 22:05:54 -0700 (In reply to comment #4) > > It seems a little strange to do the entire load and then report it as if it was a failure once the load is done. It seems the load should instead fail as soon as we get the HTTP header before reading any more data, loosely speaking. > > Yeah, we could do that. It's easier to do at this layer because it handles the cache-hit and cache-miss cases uniformly. I'd imagine the common case is for the check to pass though. The CachedResource class could be changed call a function that says “we have the header but not the data” for both the cache-miss case and the cache-hit case. For the cache-hit case it would be called right before the call to notifyFinished. What I like about this is not just the performance but the clarity that processing of the header goes in the header function and processing of the complete data and header goes in the data function and processing of partial data goes in the partial data function. Obviously notifyFinished could use a better name too. 489165 8 abarth 2011-10-23 22:13:17 -0700 Yeah, that approach is consistent with CachedResource's design of synchronously generating the various callbacks during a cache hit in addClient. I'll keep this issue in mind when looking at Nate's patch to make XMLHttpRequest over to the CachedResourceLoader pipeline. XMLHttpRequest needs a full-featured CORS implementation, so it should be easier for ImageLoader to do this work once that's done. 505682 9 bzbarsky 2011-11-18 04:28:08 -0800 Adam, should I expect the fix for this bug to show up in this Chrome version: Google Chrome 17.0.942.0 (Official Build 110446) dev OS Mac OS X WebKit 535.8 (@100508) ? As far as I can tell that Chrome version is loading the various images linked from the list on the left of http://maps.google.it/maps/ms?msid=211095647232644570760.00047466b28e95f25449b&msa=0 that use @crossorigin but don't send CORS headers.... 506346 10 abarth 2011-11-18 23:59:57 -0800 Thanks Boris. I'll investigate. 112119 2011-10-23 10:54:59 -0700 2011-10-23 16:58:03 -0700 work in progress bug-69732-20111023105458.patch text/plain 731 abarth U3VidmVyc2lvbiBSZXZpc2lvbjogOTgxOTMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2xv YWRlci9JbWFnZUxvYWRlci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvSW1hZ2VMb2FkZXIu Y3BwCmluZGV4IGMwNDZiMjNmZTNlMTQyYzEyOGY0MzFjNDE3ZTczZWFiMDRhYzNlN2MuLjdlMzAy N2M3NzllY2FhOGFmZTgwZDVjZDFlNWE3MzNjNDE3ZjQxNWYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9X ZWJDb3JlL2xvYWRlci9JbWFnZUxvYWRlci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvbG9hZGVy L0ltYWdlTG9hZGVyLmNwcApAQCAtMjM5LDYgKzIzOSwxMiBAQCB2b2lkIEltYWdlTG9hZGVyOjpu b3RpZnlGaW5pc2hlZChDYWNoZWRSZXNvdXJjZSogcmVzb3VyY2UpCiAgICAgaWYgKG1fZmlyZWRM b2FkKQogICAgICAgICByZXR1cm47CiAKKyAgICBpZiAobV9lbGVtZW50LT5mYXN0SGFzQXR0cmli dXRlKEhUTUxOYW1lczo6Y3Jvc3NvcmlnaW5BdHRyKSAmJiAhcmVzb3VyY2UtPnBhc3Nlc0FjY2Vz c0NvbnRyb2xDaGVjayhtX2VsZW1lbnQtPmRvY3VtZW50KCktPnNlY3VyaXR5T3JpZ2luKCkpKSB7 CisgICAgICAgIHNldEltYWdlKDApOworICAgICAgICBBU1NFUlQobV9maXJlZExvYWQpOworICAg ICAgICByZXR1cm47CisgICAgfQorCiAgICAgaWYgKHJlc291cmNlLT53YXNDYW5jZWxlZCgpKSB7 CiAgICAgICAgIG1fZmlyZWRMb2FkID0gdHJ1ZTsKICAgICAgICAgcmV0dXJuOwo= 112130 2011-10-23 16:58:07 -0700 2011-10-23 21:35:47 -0700 Patch bug-69732-20111023165806.patch text/plain 4550 abarth U3VidmVyc2lvbiBSZXZpc2lvbjogOTgyMDIKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCAyNjZiNzIzZWQwZjIzNzhk ZTA4YmZhMGIyNjM1NTdhMmI0NDNhY2QwLi4wNDFjYjc5ODczMjljYjE2ZmFmNTc3NTAxNWQ0ZDEz YTkzZWZkNzhkIDEwMDc1NQotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjMgQEAKKzIwMTEtMTAtMjMgIEFkYW0g QmFydGggIDxhYmFydGhAd2Via2l0Lm9yZz4KKworICAgICAgICA8aW1nIGNyb3Nzb3JpZ2luPiBz aG91bGQgZmFpbCB0byBsb2FkIHdoZW4gQ09SUyBjaGVjayBmYWlscworICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Njk3MzIKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXaGVuIGxvYWRpbmcgYW4gaW1hZ2Ugd2l0 aCB0aGUgY3Jvc3NvcmlnaW4gYXR0cmlidXRlLCB0aGUgc3BlYyBzYXlzCisgICAgICAgIHRoYXQg d2UncmUgbm90IHN1cHBvc2VkIHRvIGxvYWQgdGhlIGltYWdlIGlmIHRoZSBDT1JTIGNoZWNrIGZh aWxzLgorICAgICAgICBUaGlzICJmYWlscyBmYXN0IiBiZWhhdmlvciBpcyBpbnRlbmRlZCB0byBo ZWxwIGRldmVsb3BlcnMgdW5kZXJzdGFuZAorICAgICAgICB3aGV0aGVyIHRoZXkndmUgY29uZmln dXJlZCBDT1JTIGNvcnJlY3RseSAoaW5zdGVhZCBvZiBvbmx5IGNhdGNoaW5nIHRoZQorICAgICAg ICBlcnJvciBsYXRlciB3aGVuIHRyeWluZyB0byByZWFkIGJhY2sgdGhlIGNhbnZhcykuCisKKyAg ICAgICAgT3VyIG5ldyBiZWhhdmlvciBtYXRjaGVzIHRoZSBzcGVjIGFuZCBGaXJlZm94LgorCisg ICAgICAgIFRlc3Q6IGh0dHAvdGVzdHMvc2VjdXJpdHkvaW1nLXdpdGgtZmFpbGVkLWNvcnMtY2hl Y2stZmFpbHMtdG8tbG9hZC5odG1sCisKKyAgICAgICAgKiBsb2FkZXIvSW1hZ2VMb2FkZXIuY3Bw OgorICAgICAgICAoV2ViQ29yZTo6SW1hZ2VMb2FkZXI6Om5vdGlmeUZpbmlzaGVkKToKKwogMjAx MS0xMC0yMiAgR2VvZmZyZXkgR2FyZW4gIDxnZ2FyZW5AYXBwbGUuY29tPgogCiAgICAgICAgIFNl cGFyYXRlZCBzdHJpbmcgbGlmZXRpbWUgYml0cyBmcm9tIGNoYXJhY3RlciBidWZmZXIgc3RhdGUg Yml0cwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvbG9hZGVyL0ltYWdlTG9hZGVyLmNwcCBi L1NvdXJjZS9XZWJDb3JlL2xvYWRlci9JbWFnZUxvYWRlci5jcHAKaW5kZXggYzA0NmIyM2ZlM2Ux NDJjMTI4ZjQzMWM0MTdlNzNlYWIwNGFjM2U3Yy4uYmYzYmVlYmNmMjBhYTA1NjhiMDZhMTkxZDE1 ZDUzODExMGNmZjM5NCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvbG9hZGVyL0ltYWdlTG9h ZGVyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvSW1hZ2VMb2FkZXIuY3BwCkBAIC0z Miw2ICszMiw3IEBACiAjaW5jbHVkZSAiSFRNTE9iamVjdEVsZW1lbnQuaCIKICNpbmNsdWRlICJI VE1MUGFyc2VySWRpb21zLmgiCiAjaW5jbHVkZSAiUmVuZGVySW1hZ2UuaCIKKyNpbmNsdWRlICJT Y3JpcHRDYWxsU3RhY2suaCIKIAogI2lmIEVOQUJMRShTVkcpCiAjaW5jbHVkZSAiUmVuZGVyU1ZH SW1hZ2UuaCIKQEAgLTIzOSw2ICsyNDAsMTYgQEAgdm9pZCBJbWFnZUxvYWRlcjo6bm90aWZ5Rmlu aXNoZWQoQ2FjaGVkUmVzb3VyY2UqIHJlc291cmNlKQogICAgIGlmIChtX2ZpcmVkTG9hZCkKICAg ICAgICAgcmV0dXJuOwogCisgICAgaWYgKG1fZWxlbWVudC0+ZmFzdEhhc0F0dHJpYnV0ZShIVE1M TmFtZXM6OmNyb3Nzb3JpZ2luQXR0cikgJiYgIXJlc291cmNlLT5wYXNzZXNBY2Nlc3NDb250cm9s Q2hlY2sobV9lbGVtZW50LT5kb2N1bWVudCgpLT5zZWN1cml0eU9yaWdpbigpKSkgeworICAgICAg ICBzZXRJbWFnZSgwKTsKKworICAgICAgICBERUZJTkVfU1RBVElDX0xPQ0FMKFN0cmluZywgY29u c29sZU1lc3NhZ2UsICgiQ3Jvc3Mtb3JpZ2luIGltYWdlIGxvYWQgZGVuaWVkIGJ5IENyb3NzLU9y aWdpbiBSZXNvdXJjZSBTaGFyaW5nIHBvbGljeS4iKSk7CisgICAgICAgIG1fZWxlbWVudC0+ZG9j dW1lbnQoKS0+YWRkTWVzc2FnZShKU01lc3NhZ2VTb3VyY2UsIExvZ01lc3NhZ2VUeXBlLCBFcnJv ck1lc3NhZ2VMZXZlbCwgY29uc29sZU1lc3NhZ2UsIDEsIFN0cmluZygpLCAwKTsKKworICAgICAg ICBBU1NFUlQobV9maXJlZExvYWQpOworICAgICAgICByZXR1cm47CisgICAgfQorCiAgICAgaWYg KHJlc291cmNlLT53YXNDYW5jZWxlZCgpKSB7CiAgICAgICAgIG1fZmlyZWRMb2FkID0gdHJ1ZTsK ICAgICAgICAgcmV0dXJuOwpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nIGIvTGF5 b3V0VGVzdHMvQ2hhbmdlTG9nCmluZGV4IGEzNWI2NTliYTIwMmVkZDk0ZWU3ZTBlYWFiZjkzZjQ4 YmVkMDA3ODQuLmJjYzM4MzEwNjgwYjUwMjQ1MGU1NGFlZjBkOTA3YTNlMWQwMzFlOWEgMTAwNjQ0 Ci0tLSBhL0xheW91dFRlc3RzL0NoYW5nZUxvZworKysgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cK QEAgLTEsMyArMSwxNiBAQAorMjAxMS0xMC0yMyAgQWRhbSBCYXJ0aCAgPGFiYXJ0aEB3ZWJraXQu b3JnPgorCisgICAgICAgIDxpbWcgY3Jvc3NvcmlnaW4+IHNob3VsZCBmYWlsIHRvIGxvYWQgd2hl biBDT1JTIGNoZWNrIGZhaWxzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD02OTczMgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIFRlc3QgdGhhdCBpbWFnZXMgbG9hZGVkIHdpdGggdGhlIGNyb3Nzb3JpZ2luIGF0 dHJpYnV0ZSBmYWlsIHRvIGxvYWQgaWYKKyAgICAgICAgdGhlIENPUlMgYWNjZXNzIGNoZWNrIGRv ZXNuJ3QgcGFzcy4KKworICAgICAgICAqIGh0dHAvdGVzdHMvc2VjdXJpdHkvaW1nLXdpdGgtZmFp bGVkLWNvcnMtY2hlY2stZmFpbHMtdG8tbG9hZC1leHBlY3RlZC50eHQ6IEFkZGVkLgorICAgICAg ICAqIGh0dHAvdGVzdHMvc2VjdXJpdHkvaW1nLXdpdGgtZmFpbGVkLWNvcnMtY2hlY2stZmFpbHMt dG8tbG9hZC5odG1sOiBBZGRlZC4KKwogMjAxMS0xMC0yMyAgR2VvZmZyZXkgR2FyZW4gIDxnZ2Fy ZW5AYXBwbGUuY29tPgogCiAgICAgICAgIE5vdCByZXZpZXdlZC4KZGlmZiAtLWdpdCBhL0xheW91 dFRlc3RzL2h0dHAvdGVzdHMvc2VjdXJpdHkvaW1nLXdpdGgtZmFpbGVkLWNvcnMtY2hlY2stZmFp bHMtdG8tbG9hZC1leHBlY3RlZC50eHQgYi9MYXlvdXRUZXN0cy9odHRwL3Rlc3RzL3NlY3VyaXR5 L2ltZy13aXRoLWZhaWxlZC1jb3JzLWNoZWNrLWZhaWxzLXRvLWxvYWQtZXhwZWN0ZWQudHh0Cm5l dyBmaWxlIG1vZGUgMTAwNjQ0CmluZGV4IDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw MDAwMDAwMDAuLjQwYzU4MDUwNmY2YTA0YTgxYzY0M2U5NDQxNmRjMmJhNzg0ODhhNzcKLS0tIC9k ZXYvbnVsbAorKysgYi9MYXlvdXRUZXN0cy9odHRwL3Rlc3RzL3NlY3VyaXR5L2ltZy13aXRoLWZh aWxlZC1jb3JzLWNoZWNrLWZhaWxzLXRvLWxvYWQtZXhwZWN0ZWQudHh0CkBAIC0wLDAgKzEsMiBA QAorQ09OU09MRSBNRVNTQUdFOiBsaW5lIDE6IENyb3NzLW9yaWdpbiBpbWFnZSBsb2FkIGRlbmll ZCBieSBDcm9zcy1PcmlnaW4gUmVzb3VyY2UgU2hhcmluZyBwb2xpY3kuCitUaGlzIHRlc3QgcGFz c2VzIGlmIHRoZSBpbWFnZSBiZWxvdyBkb2VzIG5vdCBsb2FkLiAKZGlmZiAtLWdpdCBhL0xheW91 dFRlc3RzL2h0dHAvdGVzdHMvc2VjdXJpdHkvaW1nLXdpdGgtZmFpbGVkLWNvcnMtY2hlY2stZmFp bHMtdG8tbG9hZC5odG1sIGIvTGF5b3V0VGVzdHMvaHR0cC90ZXN0cy9zZWN1cml0eS9pbWctd2l0 aC1mYWlsZWQtY29ycy1jaGVjay1mYWlscy10by1sb2FkLmh0bWwKbmV3IGZpbGUgbW9kZSAxMDA2 NDQKaW5kZXggMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMC4uOGM2N2Ex OGNiNTMyNGE0YjQ5NzczOGQ0MGFiY2ZjZDkzYzhhNzRlYwotLS0gL2Rldi9udWxsCisrKyBiL0xh eW91dFRlc3RzL2h0dHAvdGVzdHMvc2VjdXJpdHkvaW1nLXdpdGgtZmFpbGVkLWNvcnMtY2hlY2st ZmFpbHMtdG8tbG9hZC5odG1sCkBAIC0wLDAgKzEsMTYgQEAKKzxib2R5PgorVGhpcyB0ZXN0IHBh c3NlcyBpZiB0aGUgaW1hZ2UgYmVsb3cgZG9lcyBub3QgbG9hZC4KKzxzY3JpcHQ+CitpZiAod2lu ZG93LmxheW91dFRlc3RDb250cm9sbGVyKQorICAgIGxheW91dFRlc3RDb250cm9sbGVyLmR1bXBB c1RleHQoKTsKKwordmFyIGltZyA9IG5ldyBJbWFnZSgpOworCitpbWcuYWRkRXZlbnRMaXN0ZW5l cignbG9hZCcsIGZ1bmN0aW9uKGV2ZW50KSB7CisgICAgYWxlcnQoJ0ZBSUw6IFRoZSBpbWFnZSBs b2FkZWQuJyk7Cit9LCBmYWxzZSk7CisKK2ltZy5jcm9zc09yaWdpbiA9ICIiOworaW1nLnNyYyA9 ICJodHRwOi8vbG9jYWxob3N0OjgwODAvc2VjdXJpdHkvcmVzb3VyY2VzL3JlZDIwMHgxMDAucG5n IjsKK2RvY3VtZW50LmJvZHkuYXBwZW5kQ2hpbGQoaW1nKTsKKzwvc2NyaXB0Pgo=