From 2a2fc4d16d17e10ee91bbd5ca16769a33b19c742 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Wed, 22 Feb 2023 15:50:31 +0100 Subject: [PATCH] QMessageAuthenticationCode: remove lazy initialization of messageHash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This just complicates the code, for the small benefit of avoiding a messageHash seeding from an empty key that then has to be reset. This lazy initialization is in the way of using QCH's SmallByteArray for the key, which this author thinks is the more important optimization, because it will allow passing keys by QByteArrayView, removing the impedance mismatch between QMAC and QCH. Since the QMAC API doesn't distinguish between the absence of a key, and the presence of a null (ie. empty) key, we can't not call initMessageHash() when the key is empty, so we should suggest to pass the actual key to the constructor as often as possible, and use setKey() only to change the key afterwards. [ChangeLog][QtCore][QMessageAuthenticationCode] No longer delays processing of the key to the first setData() or result() call. While passing a default-constructed key to the constructor and then calling setKey() continues to work, for optimal performance, we suggest to pass the actual key as a constructor argument and call setKey() only to change the key. Pick-to: 6.5 Change-Id: If0a078f37a16f8306f77d2b2bd5dacf23ce5c3e2 Reviewed-by: Qt CI Bot Reviewed-by: Thiago Macieira Reviewed-by: Mårten Nordheim --- .../qmessageauthenticationcode/main.cpp | 4 +- src/corelib/tools/qcryptographichash.cpp | 43 +++++++++++++------ 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/corelib/doc/snippets/qmessageauthenticationcode/main.cpp b/src/corelib/doc/snippets/qmessageauthenticationcode/main.cpp index 90ea384319c..530f93cdd56 100644 --- a/src/corelib/doc/snippets/qmessageauthenticationcode/main.cpp +++ b/src/corelib/doc/snippets/qmessageauthenticationcode/main.cpp @@ -1,3 +1,4 @@ +// Copyright (C) 2023 The Qt Company Ltd. // Copyright (C) 2016 Ruslan Nigmatullin // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR BSD-3-Clause @@ -13,8 +14,7 @@ int main(int argc, char *argv[]) //! [0] //! [1] - QMessageAuthenticationCode code(QCryptographicHash::Sha1); - code.setKey(key); + QMessageAuthenticationCode code(QCryptographicHash::Sha1, key); code.addData(message); code.result().toHex(); // returns "de7c9b85b8b78aa6bc8a7a36f70a90701c9db4d9" //! [1] diff --git a/src/corelib/tools/qcryptographichash.cpp b/src/corelib/tools/qcryptographichash.cpp index 0e57154c573..95498df3d97 100644 --- a/src/corelib/tools/qcryptographichash.cpp +++ b/src/corelib/tools/qcryptographichash.cpp @@ -983,7 +983,7 @@ class QMessageAuthenticationCodePrivate { public: QMessageAuthenticationCodePrivate(QCryptographicHash::Algorithm m) - : messageHash(m), method(m), messageHashInited(false) + : messageHash(m), method(m) { } @@ -992,7 +992,6 @@ public: QBasicMutex finalizeMutex; QCryptographicHash messageHash; const QCryptographicHash::Algorithm method; - bool messageHashInited; void initMessageHash(); void finalize(); @@ -1005,10 +1004,6 @@ public: void QMessageAuthenticationCodePrivate::initMessageHash() { - if (messageHashInited) - return; - messageHashInited = true; - const int blockSize = qt_hash_block_size(method); if (key.size() > blockSize) { @@ -1068,6 +1063,7 @@ QMessageAuthenticationCode::QMessageAuthenticationCode(QCryptographicHash::Algor : d(new QMessageAuthenticationCodePrivate(method)) { d->key = key; + d->initMessageHash(); } /*! @@ -1085,16 +1081,43 @@ void QMessageAuthenticationCode::reset() { d->result.clear(); d->messageHash.reset(); - d->messageHashInited = false; + d->initMessageHash(); } /*! Sets secret \a key. Calling this method automatically resets the object state. + + For optimal performance, call this method only to \e change the active key, + not to set an \e initial key, as in + + \code + QMessageAuthenticationCode mac(method); + mac.setKey(key); // does extra work + use(mac); + \endcode + + Perfer to pass initial keys as the constructor argument: + + \code + QMessageAuthenticationCode mac(method, key); // OK, optimal + use(mac); + \endcode + + You can use std::optional to delay construction of a + QMessageAuthenticationCode until you know the key: + + \code + std::optional mac; + ~~~ + key = ~~~; + mac.emplace(method, key); + use(*mac); + \endcode */ void QMessageAuthenticationCode::setKey(const QByteArray &key) { - reset(); d->key = key; + reset(); } /*! @@ -1102,7 +1125,6 @@ void QMessageAuthenticationCode::setKey(const QByteArray &key) */ void QMessageAuthenticationCode::addData(const char *data, qsizetype length) { - d->initMessageHash(); d->messageHash.addData({data, length}); } @@ -1111,7 +1133,6 @@ void QMessageAuthenticationCode::addData(const char *data, qsizetype length) */ void QMessageAuthenticationCode::addData(const QByteArray &data) { - d->initMessageHash(); d->messageHash.addData(data); } @@ -1123,7 +1144,6 @@ void QMessageAuthenticationCode::addData(const QByteArray &data) */ bool QMessageAuthenticationCode::addData(QIODevice *device) { - d->initMessageHash(); return d->messageHash.addData(device); } @@ -1143,7 +1163,6 @@ void QMessageAuthenticationCodePrivate::finalize() const auto lock = qt_scoped_lock(finalizeMutex); if (!result.isEmpty()) return; - initMessageHash(); finalizeUnchecked(); }