38428 2010-05-02 03:24:41 -0700 HTTP code 500 (and other non-4xx codes) wrongfully treated as success for subresources 2010-08-20 22:06:46 -0700 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 tom ap ap beidson commit-queue koivisto playmobil oldest_to_newest 219809 0 tom 2010-05-02 03:24:41 -0700 The following code produces an alert in Firefox, but none in Safari/Chrome: <html> <head> <script src="http://example.com/NOTFOUND.js" type="text/javascript" onerror="alert('error')"></script> </head> <body> </body> </html> 219812 1 tom 2010-05-02 03:36:20 -0700 Sorry, my first example is actually working correctly in both browsers (although I thought I tested it, maybe a cache issue). Here's another test case that doesn't work however: test.php: <?php header('HTTP/1.1 500 Internal Server Error'); ?> alert('executing script'); onerror.html: <html> <head> <script onerror="alert('error');" src="test.php" type="text/javascript"></script> <head> <body> </body> </html> Firefox displays an "error" alert here, whereas Safari/Chrome try to execute the script (i.e. the "executing script" alert box shows up). As 500 is an error code, I believe Firefox' behavior is correct. 220181 2 ap 2010-05-03 10:48:12 -0700 Yes, we wrongfully only treat 4xx as error for subresources: if (resource->response().httpStatusCode() / 100 == 4) { // Treat a 4xx response like a network error for all resources but images (which will ignore the error and continue to load for // legacy compatibility). resource->httpStatusCodeError(); return; } 220185 3 beidson 2010-05-03 11:01:07 -0700 (In reply to comment #2) > Yes, we wrongfully only treat 4xx as error for subresources: > > if (resource->response().httpStatusCode() / 100 == 4) { > // Treat a 4xx response like a network error for all resources but > images (which will ignore the error and continue to load for > // legacy compatibility). > resource->httpStatusCodeError(); > return; > } Funny thing is we do a `>= 400` check in many other places. Resource.js: if (this.statusCode >= 400) InspectorController.cpp: if (response.httpStatusCode() >= 400) { DocumentLoader.cpp: return m_substituteData.isValid() || m_response.httpStatusCode() >= 400; HistoryController.cpp: if (!unreachableURL.isEmpty() || !docLoader || docLoader->response().httpStatusCode() >= 400) NetscapePlugInStreamLoader.cpp: if (response.httpStatusCode() < 100 || response.httpStatusCode() >= 400) I wonder if there's any particular reason the cache doesn't do that, or if it's just an ancient bug. 220212 4 ap 2010-05-03 11:30:27 -0700 ApplicationCache does "response.httpStatusCode() / 100 != 2", which I thought was the formally correct thing to do. Of course, the networking layer insulates us from seeing things like 101 or 304 in the loader too often. 267498 5 65013 ap 2010-08-20 17:50:08 -0700 Created attachment 65013 proposed patch Let's try and see what breaks? 267519 6 65013 beidson 2010-08-20 19:58:39 -0700 Comment on attachment 65013 proposed patch Yes. Let's see. :) 267533 7 65013 commit-queue 2010-08-20 22:06:40 -0700 Comment on attachment 65013 proposed patch Clearing flags on attachment: 65013 Committed r65774: <http://trac.webkit.org/changeset/65774> 267534 8 commit-queue 2010-08-20 22:06:46 -0700 All reviewed patches have been landed. Closing bug. 65013 2010-08-20 17:50:08 -0700 2010-08-20 22:06:40 -0700 proposed patch 500.txt text/plain 3450 ap SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA2NTc3MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTAtMDgtMjAgIEFsZXhleSBQcm9za3VyeWFrb3YgIDxhcEBhcHBs ZS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTM4NDI4CisgICAgICAgIEhU VFAgY29kZSA1MDAgKGFuZCBvdGhlciBub24tNHh4IGNvZGVzKSB3cm9uZ2Z1bGx5IHRyZWF0ZWQg YXMgc3VjY2VzcyBmb3Igc3VicmVzb3VyY2VzCisKKyAgICAgICAgVGVzdDogaHR0cC90ZXN0cy9t aXNjL3NjcmlwdC01MDAuaHRtbAorCisgICAgICAgICogbG9hZGVyL2xvYWRlci5jcHA6IChXZWJD b3JlOjpMb2FkZXI6Okhvc3Q6OmRpZFJlY2VpdmVEYXRhKTogVHJlYXQgYWxsIEhUVFAgY29kZXMg Pj0gNDAwIGFzCisgICAgICAgIGVycm9yIG9uZXMsIHNpbmNlIHRoZXkgYXJlLgorCiAyMDEwLTA4 LTIwICBUb255IEdlbnRpbGNvcmUgIDx0b255Z0BjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmV2 aWV3ZWQgYnkgQWRhbSBCYXJ0aC4KSW5kZXg6IFdlYkNvcmUvbG9hZGVyL2xvYWRlci5jcHAKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gV2ViQ29yZS9sb2FkZXIvbG9hZGVyLmNwcAkocmV2aXNpb24gNjU3MzcpCisr KyBXZWJDb3JlL2xvYWRlci9sb2FkZXIuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC01NTEsOSArNTUx LDcgQEAgdm9pZCBMb2FkZXI6Okhvc3Q6OmRpZFJlY2VpdmVEYXRhKFN1YnJlcwogICAgIGlmIChy ZXNvdXJjZS0+ZXJyb3JPY2N1cnJlZCgpKQogICAgICAgICByZXR1cm47CiAgICAgICAgIAotICAg IGlmIChyZXNvdXJjZS0+cmVzcG9uc2UoKS5odHRwU3RhdHVzQ29kZSgpIC8gMTAwID09IDQpIHsK LSAgICAgICAgLy8gVHJlYXQgYSA0eHggcmVzcG9uc2UgbGlrZSBhIG5ldHdvcmsgZXJyb3IgZm9y IGFsbCByZXNvdXJjZXMgYnV0IGltYWdlcyAod2hpY2ggd2lsbCBpZ25vcmUgdGhlIGVycm9yIGFu ZCBjb250aW51ZSB0byBsb2FkIGZvciAKLSAgICAgICAgLy8gbGVnYWN5IGNvbXBhdGliaWxpdHkp LgorICAgIGlmIChyZXNvdXJjZS0+cmVzcG9uc2UoKS5odHRwU3RhdHVzQ29kZSgpID49IDQwMCkg ewogICAgICAgICByZXNvdXJjZS0+aHR0cFN0YXR1c0NvZGVFcnJvcigpOwogICAgICAgICByZXR1 cm47CiAgICAgfQpJbmRleDogTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExh eW91dFRlc3RzL0NoYW5nZUxvZwkocmV2aXNpb24gNjU3NzApCisrKyBMYXlvdXRUZXN0cy9DaGFu Z2VMb2cJKHdvcmtpbmcgY29weSkKQEAgLTEsMyArMSwxNCBAQAorMjAxMC0wOC0yMCAgQWxleGV5 IFByb3NrdXJ5YWtvdiAgPGFwQGFwcGxlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9Mzg0MjgKKyAgICAgICAgSFRUUCBjb2RlIDUwMCAoYW5kIG90aGVyIG5vbi00eHggY29k ZXMpIHdyb25nZnVsbHkgdHJlYXRlZCBhcyBzdWNjZXNzIGZvciBzdWJyZXNvdXJjZXMKKworICAg ICAgICAqIGh0dHAvdGVzdHMvbWlzYy9yZXNvdXJjZXMvc2NyaXB0LTUwMC5waHA6IEFkZGVkLgor ICAgICAgICAqIGh0dHAvdGVzdHMvbWlzYy9zY3JpcHQtNTAwLWV4cGVjdGVkLnR4dDogQWRkZWQu CisgICAgICAgICogaHR0cC90ZXN0cy9taXNjL3NjcmlwdC01MDAuaHRtbDogQWRkZWQuCisKIDIw MTAtMDgtMjAgIFRvbnkgR2VudGlsY29yZSAgPHRvbnlnQGNocm9taXVtLm9yZz4KIAogICAgICAg ICBSZXZpZXdlZCBieSBBZGFtIEJhcnRoLgpJbmRleDogTGF5b3V0VGVzdHMvaHR0cC90ZXN0cy9t aXNjL3NjcmlwdC01MDAtZXhwZWN0ZWQudHh0Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3RzL2h0 dHAvdGVzdHMvbWlzYy9zY3JpcHQtNTAwLWV4cGVjdGVkLnR4dAkocmV2aXNpb24gMCkKKysrIExh eW91dFRlc3RzL2h0dHAvdGVzdHMvbWlzYy9zY3JpcHQtNTAwLWV4cGVjdGVkLnR4dAkocmV2aXNp b24gMCkKQEAgLTAsMCArMSwyIEBACitUZXN0IHRoYXQgb25lcnJvciBmaXJlcyBmb3IgYSBzY3Jp cHQgd2l0aCBIVFRQIHN0YXR1cyA1MDAuCitQQVNTCkluZGV4OiBMYXlvdXRUZXN0cy9odHRwL3Rl c3RzL21pc2Mvc2NyaXB0LTUwMC5odG1sCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3RzL2h0dHAv dGVzdHMvbWlzYy9zY3JpcHQtNTAwLmh0bWwJKHJldmlzaW9uIDApCisrKyBMYXlvdXRUZXN0cy9o dHRwL3Rlc3RzL21pc2Mvc2NyaXB0LTUwMC5odG1sCShyZXZpc2lvbiAwKQpAQCAtMCwwICsxLDEx IEBACis8aHRtbD4KKzxib2R5PgorPGRpdj5UZXN0IHRoYXQgb25lcnJvciBmaXJlcyBmb3IgYSBz Y3JpcHQgd2l0aCBIVFRQIHN0YXR1cyA1MDAuPC9kaXY+Cis8ZGl2IGlkPXJlc3VsdD5UZXN0aW5n Li4uPC9kaXY+Cis8c2NyaXB0PgoraWYgKHdpbmRvdy5sYXlvdXRUZXN0Q29udHJvbGxlcikKKyAg ICBsYXlvdXRUZXN0Q29udHJvbGxlci5kdW1wQXNUZXh0KCk7Cis8L3NjcmlwdD4KKzxzY3JpcHQg b25lcnJvcj0iZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoJ3Jlc3VsdCcpLmlubmVySFRNTCA9ICdQ QVNTJzsiIHNyYz0icmVzb3VyY2VzL3NjcmlwdC01MDAucGhwIiB0eXBlPSJ0ZXh0L2phdmFzY3Jp cHQiPjwvc2NyaXB0PgorPC9ib2R5PgorPC9odG1sPgpJbmRleDogTGF5b3V0VGVzdHMvaHR0cC90 ZXN0cy9taXNjL3Jlc291cmNlcy9zY3JpcHQtNTAwLnBocAo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRU ZXN0cy9odHRwL3Rlc3RzL21pc2MvcmVzb3VyY2VzL3NjcmlwdC01MDAucGhwCShyZXZpc2lvbiAw KQorKysgTGF5b3V0VGVzdHMvaHR0cC90ZXN0cy9taXNjL3Jlc291cmNlcy9zY3JpcHQtNTAwLnBo cAkocmV2aXNpb24gMCkKQEAgLTAsMCArMSw0IEBACis8P3BocAoraGVhZGVyKCdIVFRQLzEuMSA1 MDAgSW50ZXJuYWwgU2VydmVyIEVycm9yJyk7Cis/PgorZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQo J3Jlc3VsdCcpLmlubmVySFRNTCA9ICdGQUlMJzsK