93850 2012-08-13 08:15:46 -0700 FrameLoader::receivedMainResourceError doesn't handle GET cancellations well. 2012-08-13 12:30:34 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 93777 1 mkwst mkwst abarth japhet jochen webkit.review.bot oldest_to_newest 693669 0 mkwst 2012-08-13 08:15:46 -0700 If a form submission via GET is cancelled, FrameLoader::receivedMainResourceError fails to clear m_submittedFormURL, as the form's action doesn't match the requested URL. 693674 1 158005 mkwst 2012-08-13 08:23:56 -0700 Created attachment 158005 Patch 693676 2 mkwst 2012-08-13 08:24:17 -0700 WDYT, Jochen? 693683 3 jochen 2012-08-13 08:39:13 -0700 I'd defer to Adam or Nate on this 693687 4 mkwst 2012-08-13 08:43:42 -0700 Cool. Ideas for a test would be helpful. :) 693809 5 abarth 2012-08-13 10:43:28 -0700 Makes sense to me, but I'd like to hear from Nate before landing. 693814 6 mkwst 2012-08-13 10:49:19 -0700 (In reply to comment #5) > Makes sense to me, but I'd like to hear from Nate before landing. For context, http://trac.webkit.org/browser/trunk/Source/WebCore/loader/FrameLoader.cpp#L2571 is the line where it breaks down now. The URLs don't match, as the latter has the whole query string. 693837 7 158005 japhet 2012-08-13 11:04:03 -0700 Comment on attachment 158005 Patch LGTM, but no test? 693864 8 mkwst 2012-08-13 11:23:00 -0700 (In reply to comment #7) > (From update of attachment 158005 [details]) > LGTM, but no test? Yes, it should have a test before landing it. I talked with Jochen about it, and neither of us could quickly come up with a test that would trigger the conditions. He noted there was a method on testRunner to make the next request fail, but didn't think it would work in the context of a form submit. I'd like to have a test here, I'd appreciate your input as to how it might be structured. 693874 9 abarth 2012-08-13 11:33:46 -0700 Presumably we'll get a test as part of Bug 93777? 693877 10 mkwst 2012-08-13 11:36:26 -0700 (In reply to comment #9) > Presumably we'll get a test as part of Bug 93777? Sure. You'll get two, in fact! But that's somewhat of a cop out, isn't it? "Dude, I totally have a test in the next patch. I swear!" :) 693901 11 158005 abarth 2012-08-13 11:52:17 -0700 Comment on attachment 158005 Patch Yeah, but given that the next patch is already written and the tests are there, I don't think there's that much risk that you won't come through. :) 693908 12 158005 mkwst 2012-08-13 11:56:59 -0700 Comment on attachment 158005 Patch In that case, CQ? 693950 13 158005 webkit.review.bot 2012-08-13 12:30:30 -0700 Comment on attachment 158005 Patch Clearing flags on attachment: 158005 Committed r125436: <http://trac.webkit.org/changeset/125436> 693951 14 webkit.review.bot 2012-08-13 12:30:34 -0700 All reviewed patches have been landed. Closing bug. 158005 2012-08-13 08:23:56 -0700 2012-08-13 12:30:30 -0700 Patch bug-93850-20120813172324.patch text/plain 2339 mkwst U3VidmVyc2lvbiBSZXZpc2lvbjogMTI1NDA3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNWVhMTBkY2I4OWMxNTg5 NTBkYTQwOTA0ZDE0NTgwYWE1NThkMGVhYy4uYTBjZmI4YzU4ZmZjYjQzNDA3NGY3Y2UzMjExZDY5 MTQ5NTIyYzk3YSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI3IEBACisyMDEyLTA4LTEzICBNaWtl IFdlc3QgIDxta3dzdEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgRnJhbWVMb2FkZXI6OnJlY2Vp dmVkTWFpblJlc291cmNlRXJyb3IgZG9lc24ndCBoYW5kbGUgR0VUIGNhbmNlbGxhdGlvbnMgd2Vs bC4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTkzODUw CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgJ0ZyYW1l TG9hZGVyOjpjYW5jZWwnIHNob3VsZCByZXNldCB0aGUgbG9hZGVyJ3Mgc3RhdGUsIGFuZCBpdCBt b3N0bHkKKyAgICAgICAgZG9lcy4gSW4gdGhlIGNhc2Ugb2YgYSBHRVQgZm9ybSBzdWJtaXNzaW9u LCBob3dldmVyLCB0aGUgc3VibWl0dGVkIGZvcm0KKyAgICAgICAgVVJMIHRoYXQncyBzdG9yZWQg ZG9lc24ndCBtYXRjaCB0aGUgVVJMIHRoYXQncyByZXF1ZXN0ZWQgKHF1ZXJ5IHN0cmluZykuCisg ICAgICAgIFRoaXMgbGVhZHMgdG8gdGhlIGxvYWRlciBnZXR0aW5nIGEgYml0IGNvbmZ1c2VkIGFi b3V0IHdoYXQncyBnb2luZyBvbiwKKyAgICAgICAgYW5kIG5vdCBjbGVhcmluZyB0aGUgc3RvcmVk IGZvcm0gc3VibWlzc2lvbiBVUkwuCisKKyAgICAgICAgVGhpcyBwYXRjaCBtb3ZlcyB0byBzdG9y aW5nIHRoZSBmb3JtIHN1Ym1pc3Npb24ncyBhY3R1YWwgcmVxdWVzdCBVUkwKKyAgICAgICAgYXMg b3Bwb3NlZCB0byB0aGUgZm9ybSBhY3Rpb24gVVJMLgorCisgICAgICAgIE5vbmUgb2YgdGhlIGN1 cnJlbnQgdGVzdHMgc2hvdWxkIGJyZWFrLCBhbmQgd2hlbiBpc3N1ZSA5Mzc3NyBsYW5kcywgaXQK KyAgICAgICAgd29uJ3QgYmUgYnJva2VuIGVpdGhlci4KKworICAgICAgICAqIGxvYWRlci9GcmFt ZUxvYWRlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpGcmFtZUxvYWRlcjo6c3VibWl0Rm9ybSk6 CisgICAgICAgICAgICBTd2l0Y2hlZCB0byB1c2luZyAnRm9ybVN1Ym1pc3Npb246OnJlcXVlc3RV UkwnLCBhcyB0aGF0IG1hdGNoZXMgdGhlCisgICAgICAgICAgICBVUkwgdGhhdCdzIGFjdHVhbGx5 IHJlcXVlc3RlZCBkdXJpbmcgYSBmb3JtIHN1Ym1pc3Npb24gdmlhIEdFVC4KKwogMjAxMi0wOC0x MyAgTWFyaW8gU2FuY2hleiBQcmFkYSAgPG1zYW5jaGV6QGlnYWxpYS5jb20+CiAKICAgICAgICAg W0dUS10gSW1wbGVtZW50YXRpb24gb2YgYXRrX2VkaXRhYmxlX3RleHRfaW5zZXJ0X3RleHQgaWdu b3JlcyAnbGVuZ3RoJyBwYXJhbWV0ZXIKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2xvYWRl ci9GcmFtZUxvYWRlci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvRnJhbWVMb2FkZXIuY3Bw CmluZGV4IDBkY2IzYjI0ZjliZWI4YmJiY2QyZjE5MDk2NmE2YWMzZWFlOTVkNGEuLjE4MmUzZTc4 OWU0MTE0MWNmZjdhYmM5OGQ4NDUwODRjNmExZDAzOTUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL2xvYWRlci9GcmFtZUxvYWRlci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvbG9hZGVyL0Zy YW1lTG9hZGVyLmNwcApAQCAtMzM0LDkgKzMzNCw5IEBAIHZvaWQgRnJhbWVMb2FkZXI6OnN1Ym1p dEZvcm0oUGFzc1JlZlB0cjxGb3JtU3VibWlzc2lvbj4gc3VibWlzc2lvbikKICAgICAvLyBuZWVk ZWQgYW55IG1vcmUgbm93IHRoYXQgd2UgcmVzZXQgbV9zdWJtaXR0ZWRGb3JtVVJMIG9uIGVhY2gg bW91c2Ugb3Iga2V5IGRvd24gZXZlbnQuCiAKICAgICBpZiAobV9mcmFtZS0+dHJlZSgpLT5pc0Rl c2NlbmRhbnRPZih0YXJnZXRGcmFtZSkpIHsKLSAgICAgICAgaWYgKG1fc3VibWl0dGVkRm9ybVVS TCA9PSBzdWJtaXNzaW9uLT5hY3Rpb24oKSkKKyAgICAgICAgaWYgKG1fc3VibWl0dGVkRm9ybVVS TCA9PSBzdWJtaXNzaW9uLT5yZXF1ZXN0VVJMKCkpCiAgICAgICAgICAgICByZXR1cm47Ci0gICAg ICAgIG1fc3VibWl0dGVkRm9ybVVSTCA9IHN1Ym1pc3Npb24tPmFjdGlvbigpOworICAgICAgICBt X3N1Ym1pdHRlZEZvcm1VUkwgPSBzdWJtaXNzaW9uLT5yZXF1ZXN0VVJMKCk7CiAgICAgfQogCiAg ICAgc3VibWlzc2lvbi0+ZGF0YSgpLT5nZW5lcmF0ZUZpbGVzKG1fZnJhbWUtPmRvY3VtZW50KCkp Owo=