129193 2014-02-21 20:39:19 -0800 CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify contains seemingly accidental unreachable code 2014-02-21 22:02:33 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 joepeck ap ap commit-queue joepeck sam oldest_to_newest 983495 0 joepeck 2014-02-21 20:39:19 -0800 Compiling CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp with -Wunreachable-code catches: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:94:24: error: will never be executed [-Werror,-Wunreachable-code] It looks like a legit warning: void CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify(const CryptoAlgorithmRsaSsaParams& parameters, const CryptoKeyRSA& key, const CryptoOperationData& signature, const CryptoOperationData& data, BoolCallback callback, VoidCallback failureCallback, ExceptionCode& ec) { ... CCCryptorStatus status = CCRSACryptorVerify(key.platformKey(), ccPKCS1Padding, digestData.data(), digestData.size(), digestAlgorithm, 0, signature.first, signature.second); if (!status) callback(true); else if (status == kCCNotVerified || kCCDecodeError) // <rdar://problem/15464982> CCRSACryptorVerify returns kCCDecodeError instead of kCCNotVerified sometimes callback(false); else failureCallback(); } --- The "|| kCCDecodeError" part looks suspicious. Perhaps that should be comparing with status, otherwise it looks like it will always be true, and the "else failureCallback()" will never be reached. 983496 1 224943 ap 2014-02-21 20:51:01 -0800 Created attachment 224943 proposed fix 983502 2 224943 joepeck 2014-02-21 21:20:03 -0800 Comment on attachment 224943 proposed fix r=me! Should we write a test? 983505 3 224943 ap 2014-02-21 21:30:14 -0800 Comment on attachment 224943 proposed fix > Should we write a test? I don't think that there is any way to get to the failureCallback() branch in practice. 983509 4 224943 commit-queue 2014-02-21 22:02:26 -0800 Comment on attachment 224943 proposed fix Clearing flags on attachment: 224943 Committed r164528: <http://trac.webkit.org/changeset/164528> 983510 5 commit-queue 2014-02-21 22:02:33 -0800 All reviewed patches have been landed. Closing bug. 224943 2014-02-21 20:51:01 -0800 2014-02-21 22:02:23 -0800 proposed fix ErrorTypo.txt text/plain 1730 ap SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE2NDUyNikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBACisyMDE0LTAyLTIxICBBbGV4ZXkg UHJvc2t1cnlha292ICA8YXBAYXBwbGUuY29tPgorCisgICAgICAgIENyeXB0b0FsZ29yaXRobVJT QVNTQV9QS0NTMV92MV81OjpwbGF0Zm9ybVZlcmlmeSBjb250YWlucyBzZWVtaW5nbHkgYWNjaWRl bnRhbCB1bnJlYWNoYWJsZSBjb2RlCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0xMjkxOTMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICAqIGNyeXB0by9tYWMvQ3J5cHRvQWxnb3JpdGhtUlNBU1NBX1BLQ1MxX3Yx XzVNYWMuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Q3J5cHRvQWxnb3JpdGhtUlNBU1NBX1BLQ1Mx X3YxXzU6OnBsYXRmb3JtVmVyaWZ5KTogRml4IHRoZSBhY2NpZGVudGFsIGNvZGUuCisKIDIwMTQt MDItMjEgIFJ5b3N1a2UgTml3YSAgPHJuaXdhQHdlYmtpdC5vcmc+CiAKICAgICAgICAgSW5uZXIg dGV4dCBlbGVtZW50IHNob3VsZCBub3QgdXNlIC13ZWJraXQtdXNlci1tb2RpZnkKSW5kZXg6IFNv dXJjZS9XZWJDb3JlL2NyeXB0by9tYWMvQ3J5cHRvQWxnb3JpdGhtUlNBU1NBX1BLQ1MxX3YxXzVN YWMuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2NyeXB0by9tYWMvQ3J5cHRvQWxn b3JpdGhtUlNBU1NBX1BLQ1MxX3YxXzVNYWMuY3BwCShyZXZpc2lvbiAxNjQ0NDYpCisrKyBTb3Vy Y2UvV2ViQ29yZS9jcnlwdG8vbWFjL0NyeXB0b0FsZ29yaXRobVJTQVNTQV9QS0NTMV92MV81TWFj LmNwcAkod29ya2luZyBjb3B5KQpAQCAtODgsNyArODgsNyBAQCB2b2lkIENyeXB0b0FsZ29yaXRo bVJTQVNTQV9QS0NTMV92MV81OjpwCiAgICAgQ0NDcnlwdG9yU3RhdHVzIHN0YXR1cyA9IENDUlNB Q3J5cHRvclZlcmlmeShrZXkucGxhdGZvcm1LZXkoKSwgY2NQS0NTMVBhZGRpbmcsIGRpZ2VzdERh dGEuZGF0YSgpLCBkaWdlc3REYXRhLnNpemUoKSwgZGlnZXN0QWxnb3JpdGhtLCAwLCBzaWduYXR1 cmUuZmlyc3QsIHNpZ25hdHVyZS5zZWNvbmQpOwogICAgIGlmICghc3RhdHVzKQogICAgICAgICBj YWxsYmFjayh0cnVlKTsKLSAgICBlbHNlIGlmIChzdGF0dXMgPT0ga0NDTm90VmVyaWZpZWQgfHwg a0NDRGVjb2RlRXJyb3IpIC8vIDxyZGFyOi8vcHJvYmxlbS8xNTQ2NDk4Mj4gQ0NSU0FDcnlwdG9y VmVyaWZ5IHJldHVybnMga0NDRGVjb2RlRXJyb3IgaW5zdGVhZCBvZiBrQ0NOb3RWZXJpZmllZCBz b21ldGltZXMKKyAgICBlbHNlIGlmIChzdGF0dXMgPT0ga0NDTm90VmVyaWZpZWQgfHwgc3RhdHVz ID09IGtDQ0RlY29kZUVycm9yKSAvLyA8cmRhcjovL3Byb2JsZW0vMTU0NjQ5ODI+IENDUlNBQ3J5 cHRvclZlcmlmeSByZXR1cm5zIGtDQ0RlY29kZUVycm9yIGluc3RlYWQgb2Yga0NDTm90VmVyaWZp ZWQgc29tZXRpbWVzCiAgICAgICAgIGNhbGxiYWNrKGZhbHNlKTsKICAgICBlbHNlCiAgICAgICAg IGZhaWx1cmVDYWxsYmFjaygpOwo=