123461 2013-10-29 13:23:03 -0700 CryptoAlgorithmDescriptionBuilder should support producing nested algorithms 2013-10-31 09:26:46 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 122679 1 ap ap darin sam oldest_to_newest 944563 0 ap 2013-10-29 13:23:03 -0700 Many WebCrypto algorithms have nested algorithms, like hash in HMAC. We need to expose that as CryptoKey.algorithm. 944564 1 215415 ap 2013-10-29 13:25:49 -0700 Created attachment 215415 proposed patch 944629 2 215415 darin 2013-10-29 14:57:38 -0700 Comment on attachment 215415 proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=215415&action=review > Source/WebCore/bindings/js/JSCryptoAlgorithmBuilder.cpp:49 > + return std::unique_ptr<CryptoAlgorithmDescriptionBuilder>(new JSCryptoAlgorithmBuilder(m_exec)); We should use make_unique here. > Source/WebCore/bindings/js/JSCryptoAlgorithmBuilder.cpp:70 > + const JSCryptoAlgorithmBuilder* jsBuilder = static_cast<const JSCryptoAlgorithmBuilder*>(nestedBuilder); What guarantees this is a good cast? > Source/WebCore/bindings/js/JSCryptoAlgorithmBuilder.h:51 > + virtual void add(const char*, const CryptoAlgorithmDescriptionBuilder*) OVERRIDE; Why is this a pointer, not a reference? 944639 3 ap 2013-10-29 15:11:45 -0700 > What guarantees this is a good cast? Not much. The code that clones a builder and code that adds it back will be all in one function, so it's very unlikely that a foreign builder will be passed to add(). And if a mistake like that was made, the code would just crash, so it would be discovered even by most cursory testing. Is there a better pattern for this kind of thing? Here is how a call site looks like: void CryptoKeyHMAC::buildAlgorithmDescription(CryptoAlgorithmDescriptionBuilder& builder) const { CryptoKey::buildAlgorithmDescription(builder); std::unique_ptr<CryptoAlgorithmDescriptionBuilder> hashDescriptionBuilder(builder.createEmptyClone()); hashDescriptionBuilder->add("name", CryptoAlgorithmRegistry::shared().nameForIdentifier(m_hash)); builder.add("hash", hashDescriptionBuilder.get()); // will add a star when add() takes a reference as you suggested builder.add("length", m_key.size()); } 945544 4 ap 2013-10-31 09:26:46 -0700 Committed <http://trac.webkit.org/r158361>. 215415 2013-10-29 13:25:49 -0700 2013-10-29 14:57:38 -0700 proposed patch AlgorithmDescriptionBuilder.txt text/plain 4418 ap SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE1ODIxMykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBACisyMDEzLTEwLTI5ICBBbGV4ZXkg UHJvc2t1cnlha292ICA8YXBAYXBwbGUuY29tPgorCisgICAgICAgIENyeXB0b0FsZ29yaXRobURl c2NyaXB0aW9uQnVpbGRlciBzaG91bGQgc3VwcG9ydCBwcm9kdWNpbmcgbmVzdGVkIGFsZ29yaXRo bXMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEyMzQ2 MQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRvIGFk ZCBhIG5lc3RlZCBhbGdvcml0aG0sIGNsb25lIGEgYnVpbGRlciB3aXRoIGNyZWF0ZUVtcHR5Q2xv bmUoKSwgZmlsbCBpdCwKKyAgICAgICAgYW5kIGFkZCBpdCB1c2luZyBhZGQoKS4KKworICAgICAg ICAqIGJpbmRpbmdzL2pzL0pTQ3J5cHRvQWxnb3JpdGhtQnVpbGRlci5oOgorICAgICAgICAqIGNy eXB0by9DcnlwdG9BbGdvcml0aG1EZXNjcmlwdGlvbkJ1aWxkZXIuaDoKKyAgICAgICAgKiBiaW5k aW5ncy9qcy9KU0NyeXB0b0FsZ29yaXRobUJ1aWxkZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6 SlNDcnlwdG9BbGdvcml0aG1CdWlsZGVyOjpjcmVhdGVFbXB0eUNsb25lKToKKyAgICAgICAgKFdl YkNvcmU6OkpTQ3J5cHRvQWxnb3JpdGhtQnVpbGRlcjo6YWRkKTogS2VlcCBWTSBpbiBhIGxvY2Fs IHZhcmlhYmxlIGZvciBtYXJnaW5hbGx5CisgICAgICAgIGJldHRlciBwZXJmb3JtYW5jZS4KKwog MjAxMy0xMC0yOSAgQW5kcmVhcyBLbGluZyAgPGFrbGluZ0BhcHBsZS5jb20+CiAKICAgICAgICAg UmVuZGVyT2JqZWN0OjpvdXRsaW5lU3R5bGVGb3JSZXBhaW50KCkgc2hvdWxkIHJldHVybiBhIHJl ZmVyZW5jZS4KSW5kZXg6IFNvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL2pzL0pTQ3J5cHRvQWxnb3Jp dGhtQnVpbGRlci5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvYmluZGluZ3MvanMv SlNDcnlwdG9BbGdvcml0aG1CdWlsZGVyLmNwcAkocmV2aXNpb24gMTU4MjA4KQorKysgU291cmNl L1dlYkNvcmUvYmluZGluZ3MvanMvSlNDcnlwdG9BbGdvcml0aG1CdWlsZGVyLmNwcAkod29ya2lu ZyBjb3B5KQpAQCAtNDQsMTYgKzQ0LDMxIEBAIEpTQ3J5cHRvQWxnb3JpdGhtQnVpbGRlcjo6fkpT Q3J5cHRvQWxnb3IKIHsKIH0KIAotdm9pZCBKU0NyeXB0b0FsZ29yaXRobUJ1aWxkZXI6OmFkZChj b25zdCBjaGFyKiBrZXksIHVuc2lnbmVkIGxvbmcgdmFsdWUpCitzdGQ6OnVuaXF1ZV9wdHI8Q3J5 cHRvQWxnb3JpdGhtRGVzY3JpcHRpb25CdWlsZGVyPiBKU0NyeXB0b0FsZ29yaXRobUJ1aWxkZXI6 OmNyZWF0ZUVtcHR5Q2xvbmUoKSBjb25zdAogewotICAgIElkZW50aWZpZXIgaWRlbnRpZmllciht X2V4ZWMsIGtleSk7Ci0gICAgbV9kaWN0aW9uYXJ5LT5wdXREaXJlY3QobV9leGVjLT52bSgpLCBp ZGVudGlmaWVyLCBqc051bWJlcih2YWx1ZSkpOworICAgIHJldHVybiBzdGQ6OnVuaXF1ZV9wdHI8 Q3J5cHRvQWxnb3JpdGhtRGVzY3JpcHRpb25CdWlsZGVyPihuZXcgSlNDcnlwdG9BbGdvcml0aG1C dWlsZGVyKG1fZXhlYykpOworfQorCit2b2lkIEpTQ3J5cHRvQWxnb3JpdGhtQnVpbGRlcjo6YWRk KGNvbnN0IGNoYXIqIGtleSwgdW5zaWduZWQgdmFsdWUpCit7CisgICAgVk0mIHZtID0gbV9leGVj LT52bSgpOworICAgIElkZW50aWZpZXIgaWRlbnRpZmllcigmdm0sIGtleSk7CisgICAgbV9kaWN0 aW9uYXJ5LT5wdXREaXJlY3Qodm0sIGlkZW50aWZpZXIsIGpzTnVtYmVyKHZhbHVlKSk7CiB9CiAK IHZvaWQgSlNDcnlwdG9BbGdvcml0aG1CdWlsZGVyOjphZGQoY29uc3QgY2hhcioga2V5LCBjb25z dCBTdHJpbmcmIHZhbHVlKQogewotICAgIElkZW50aWZpZXIgaWRlbnRpZmllcihtX2V4ZWMsIGtl eSk7Ci0gICAgbV9kaWN0aW9uYXJ5LT5wdXREaXJlY3QobV9leGVjLT52bSgpLCBpZGVudGlmaWVy LCBqc1N0cmluZyhtX2V4ZWMsIHZhbHVlKSk7CisgICAgVk0mIHZtID0gbV9leGVjLT52bSgpOwor ICAgIElkZW50aWZpZXIgaWRlbnRpZmllcigmdm0sIGtleSk7CisgICAgbV9kaWN0aW9uYXJ5LT5w dXREaXJlY3Qodm0sIGlkZW50aWZpZXIsIGpzU3RyaW5nKG1fZXhlYywgdmFsdWUpKTsKK30KKwor dm9pZCBKU0NyeXB0b0FsZ29yaXRobUJ1aWxkZXI6OmFkZChjb25zdCBjaGFyKiBrZXksIGNvbnN0 IENyeXB0b0FsZ29yaXRobURlc2NyaXB0aW9uQnVpbGRlciogbmVzdGVkQnVpbGRlcikKK3sKKyAg ICBWTSYgdm0gPSBtX2V4ZWMtPnZtKCk7CisgICAgSWRlbnRpZmllciBpZGVudGlmaWVyKCZ2bSwg a2V5KTsKKyAgICBjb25zdCBKU0NyeXB0b0FsZ29yaXRobUJ1aWxkZXIqIGpzQnVpbGRlciA9IHN0 YXRpY19jYXN0PGNvbnN0IEpTQ3J5cHRvQWxnb3JpdGhtQnVpbGRlcio+KG5lc3RlZEJ1aWxkZXIp OworICAgIG1fZGljdGlvbmFyeS0+cHV0RGlyZWN0KHZtLCBpZGVudGlmaWVyLCBqc0J1aWxkZXIt PnJlc3VsdCgpKTsKIH0KIAogfSAvLyBuYW1lc3BhY2UgV2ViQ29yZQpJbmRleDogU291cmNlL1dl YkNvcmUvYmluZGluZ3MvanMvSlNDcnlwdG9BbGdvcml0aG1CdWlsZGVyLmgKPT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQot LS0gU291cmNlL1dlYkNvcmUvYmluZGluZ3MvanMvSlNDcnlwdG9BbGdvcml0aG1CdWlsZGVyLmgJ KHJldmlzaW9uIDE1ODIwOCkKKysrIFNvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL2pzL0pTQ3J5cHRv QWxnb3JpdGhtQnVpbGRlci5oCSh3b3JraW5nIGNvcHkpCkBAIC00MiwxMCArNDIsMTMgQEAgcHVi bGljOgogICAgIEpTQ3J5cHRvQWxnb3JpdGhtQnVpbGRlcihKU0M6OkV4ZWNTdGF0ZSopOwogICAg IHZpcnR1YWwgfkpTQ3J5cHRvQWxnb3JpdGhtQnVpbGRlcigpOwogCi0gICAgSlNDOjpKU09iamVj dCogcmVzdWx0KCkgeyByZXR1cm4gbV9kaWN0aW9uYXJ5OyB9CisgICAgSlNDOjpKU09iamVjdCog cmVzdWx0KCkgY29uc3QgeyByZXR1cm4gbV9kaWN0aW9uYXJ5OyB9CiAKLSAgICB2aXJ0dWFsIHZv aWQgYWRkKGNvbnN0IGNoYXIqLCB1bnNpZ25lZCBsb25nKSBPVkVSUklERTsKKyAgICB2aXJ0dWFs IHN0ZDo6dW5pcXVlX3B0cjxDcnlwdG9BbGdvcml0aG1EZXNjcmlwdGlvbkJ1aWxkZXI+IGNyZWF0 ZUVtcHR5Q2xvbmUoKSBjb25zdCBPVkVSUklERTsKKworICAgIHZpcnR1YWwgdm9pZCBhZGQoY29u c3QgY2hhciosIHVuc2lnbmVkKSBPVkVSUklERTsKICAgICB2aXJ0dWFsIHZvaWQgYWRkKGNvbnN0 IGNoYXIqLCBjb25zdCBTdHJpbmcmKSBPVkVSUklERTsKKyAgICB2aXJ0dWFsIHZvaWQgYWRkKGNv bnN0IGNoYXIqLCBjb25zdCBDcnlwdG9BbGdvcml0aG1EZXNjcmlwdGlvbkJ1aWxkZXIqKSBPVkVS UklERTsKIAogcHJpdmF0ZToKICAgICBKU0M6OkV4ZWNTdGF0ZSogbV9leGVjOwpJbmRleDogU291 cmNlL1dlYkNvcmUvY3J5cHRvL0NyeXB0b0FsZ29yaXRobURlc2NyaXB0aW9uQnVpbGRlci5oCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2NyeXB0by9DcnlwdG9BbGdvcml0aG1EZXNjcmlw dGlvbkJ1aWxkZXIuaAkocmV2aXNpb24gMTU4MjA4KQorKysgU291cmNlL1dlYkNvcmUvY3J5cHRv L0NyeXB0b0FsZ29yaXRobURlc2NyaXB0aW9uQnVpbGRlci5oCSh3b3JraW5nIGNvcHkpCkBAIC0z OSw4ICszOSwxMSBAQCBwdWJsaWM6CiAgICAgQ3J5cHRvQWxnb3JpdGhtRGVzY3JpcHRpb25CdWls ZGVyKCk7CiAgICAgdmlydHVhbCB+Q3J5cHRvQWxnb3JpdGhtRGVzY3JpcHRpb25CdWlsZGVyKCk7 CiAKLSAgICB2aXJ0dWFsIHZvaWQgYWRkKGNvbnN0IGNoYXIqLCB1bnNpZ25lZCBsb25nKSA9IDA7 CisgICAgdmlydHVhbCBzdGQ6OnVuaXF1ZV9wdHI8Q3J5cHRvQWxnb3JpdGhtRGVzY3JpcHRpb25C dWlsZGVyPiBjcmVhdGVFbXB0eUNsb25lKCkgY29uc3QgPSAwOworCisgICAgdmlydHVhbCB2b2lk IGFkZChjb25zdCBjaGFyKiwgdW5zaWduZWQpID0gMDsKICAgICB2aXJ0dWFsIHZvaWQgYWRkKGNv bnN0IGNoYXIqLCBjb25zdCBTdHJpbmcmKSA9IDA7CisgICAgdmlydHVhbCB2b2lkIGFkZChjb25z dCBjaGFyKiwgY29uc3QgQ3J5cHRvQWxnb3JpdGhtRGVzY3JpcHRpb25CdWlsZGVyKikgPSAwOwog fTsKIAogfSAvLyBuYW1lc3BhY2UgV2ViQ29yZQo=