From 0411d98192ede84b115618ece876579fd79252f8 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Wed, 22 Feb 2023 08:27:55 +0100 Subject: [PATCH] QCryptographicHash: auto-calculate MaxHashLength Add Algorithm::NumAlgorithms and use it to iterate over all statically-available algorithms, querying their hashLengthInternal(). This avoids having to statically_assert(<= MaxHashLength) everywhere, and auto-adjusts the buffer size in SHA1_ONLY builds. Yes, the extra case labels for NumAlgorithms are a nuisance, but at least the compiler will remind us when we forget, unlike a missing static_cast(<= MaxHashLength) that might easily be forgotten. Adjust the test (which iterates over the QMetaEnum for QCryptographicHash::Algorithm, so finds NumAlgorithms and tries to pass it to the hash() function which responds with a Q_UNREACHABLE(). Only test hashLength() == 0 for that enum value. Pick-to: 6.5 Change-Id: I70155d2460464f0b2094e136eb6bea185effc9d5 Reviewed-by: Thiago Macieira --- src/corelib/tools/qcryptographichash.cpp | 36 +++++++++++++------ src/corelib/tools/qcryptographichash.h | 1 + .../tst_qcryptographichash.cpp | 11 ++++-- 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/corelib/tools/qcryptographichash.cpp b/src/corelib/tools/qcryptographichash.cpp index 6a76c27520..4a8a8c1c20 100644 --- a/src/corelib/tools/qcryptographichash.cpp +++ b/src/corelib/tools/qcryptographichash.cpp @@ -143,15 +143,11 @@ public: { return QByteArrayView{m_data.data(), size()}; } }; -static constexpr qsizetype MaxHashLength = 64; - static constexpr int hashLengthInternal(QCryptographicHash::Algorithm method) noexcept { switch (method) { #define CASE(Enum, Size) \ case QCryptographicHash:: Enum : \ - /* if this triggers, then increase MaxHashLength accordingly */ \ - static_assert(MaxHashLength >= qsizetype(Size) ); \ return Size \ /*end*/ CASE(Sha1, 20); @@ -172,35 +168,42 @@ static constexpr int hashLengthInternal(QCryptographicHash::Algorithm method) no CASE(Blake2s_128, 128 / 8); case QCryptographicHash::Blake2b_160: case QCryptographicHash::Blake2s_160: - static_assert(160 / 8 <= MaxHashLength); return 160 / 8; case QCryptographicHash::RealSha3_224: case QCryptographicHash::Keccak_224: case QCryptographicHash::Blake2s_224: - static_assert(224 / 8 <= MaxHashLength); return 224 / 8; case QCryptographicHash::RealSha3_256: case QCryptographicHash::Keccak_256: case QCryptographicHash::Blake2b_256: case QCryptographicHash::Blake2s_256: - static_assert(256 / 8 <= MaxHashLength); return 256 / 8; case QCryptographicHash::RealSha3_384: case QCryptographicHash::Keccak_384: case QCryptographicHash::Blake2b_384: - static_assert(384 / 8 <= MaxHashLength); return 384 / 8; case QCryptographicHash::RealSha3_512: case QCryptographicHash::Keccak_512: case QCryptographicHash::Blake2b_512: - static_assert(512 / 8 <= MaxHashLength); return 512 / 8; #endif #undef CASE + case QCryptographicHash::NumAlgorithms: ; + // fall through + // Q_UNREACHABLE() would be BiC here, as hashLength(~~invalid~~) worked in 6.4 } return 0; } +static constexpr int maxHashLength() +{ + int result = 0; + using A = QCryptographicHash::Algorithm; + for (int i = 0; i < A::NumAlgorithms; ++i) + result = std::max(result, hashLengthInternal(A(i))); + return result; +} + #ifdef USING_OPENSSL30 static constexpr const char * methodToName(QCryptographicHash::Algorithm method) noexcept { @@ -298,7 +301,7 @@ public: #endif // protects result in finalize() QBasicMutex finalizeMutex; - QSmallByteArray result; + QSmallByteArray result; const QCryptographicHash::Algorithm method; }; @@ -399,6 +402,7 @@ void QCryptographicHashPrivate::sha3Finish(int bitCount, Sha3Variant sha3Variant \omitvalue RealSha3_256 \omitvalue RealSha3_384 \omitvalue RealSha3_512 + \omitvalue NumAlgorithms */ /*! @@ -578,6 +582,8 @@ void QCryptographicHashPrivate::reset() noexcept blake2s_init(&blake2sContext, hashLengthInternal(method)); break; #endif + case QCryptographicHash::NumAlgorithms: + Q_UNREACHABLE(); } #endif // !QT_CONFIG(opensslv30) } @@ -687,6 +693,8 @@ void QCryptographicHashPrivate::addData(QByteArrayView bytes) noexcept blake2s_update(&blake2sContext, reinterpret_cast(data), length); break; #endif + case QCryptographicHash::NumAlgorithms: + Q_UNREACHABLE(); } #endif // !QT_CONFIG(opensslv30) } @@ -878,6 +886,8 @@ void QCryptographicHashPrivate::finalizeUnchecked() noexcept break; } #endif + case QCryptographicHash::NumAlgorithms: + Q_UNREACHABLE(); } #endif // !QT_CONFIG(opensslv30) } @@ -981,6 +991,12 @@ static int qt_hash_block_size(QCryptographicHash::Algorithm method) case QCryptographicHash::Blake2s_256: return BLAKE2S_BLOCKBYTES; #endif // QT_CRYPTOGRAPHICHASH_ONLY_SHA1 + case QCryptographicHash::NumAlgorithms: +#if !defined(Q_GCC_ONLY) || Q_CC_GCC >= 900 + // GCC 8 has trouble with Q_UNREACHABLE() in constexpr functions + Q_UNREACHABLE(); +#endif + break; } return 0; } diff --git a/src/corelib/tools/qcryptographichash.h b/src/corelib/tools/qcryptographichash.h index 9bf473dd4b..294453adce 100644 --- a/src/corelib/tools/qcryptographichash.h +++ b/src/corelib/tools/qcryptographichash.h @@ -60,6 +60,7 @@ public: Blake2s_224, Blake2s_256, #endif + NumAlgorithms }; Q_ENUM(Algorithm) diff --git a/tests/auto/corelib/tools/qcryptographichash/tst_qcryptographichash.cpp b/tests/auto/corelib/tools/qcryptographichash/tst_qcryptographichash.cpp index 00f7afc876..4ac176fea1 100644 --- a/tests/auto/corelib/tools/qcryptographichash/tst_qcryptographichash.cpp +++ b/tests/auto/corelib/tools/qcryptographichash/tst_qcryptographichash.cpp @@ -401,8 +401,15 @@ void tst_QCryptographicHash::hashLength() { QFETCH(const QCryptographicHash::Algorithm, algorithm); - QByteArray output = QCryptographicHash::hash("test", algorithm); - QCOMPARE(QCryptographicHash::hashLength(algorithm), output.size()); + qsizetype expectedSize; + if (algorithm == QCryptographicHash::NumAlgorithms) { + // It's UB to call ::hash() with NumAlgorithms, but hashLength() is + // fine and returns 0 for invalid values: + expectedSize = 0; + } else { + expectedSize = QCryptographicHash::hash("test", algorithm).size(); + } + QCOMPARE(QCryptographicHash::hashLength(algorithm), expectedSize); } void tst_QCryptographicHash::move()