From 3f35eadd77b34a5ab13f89772583d153bbebdc70 Mon Sep 17 00:00:00 2001 From: saturneric Date: Thu, 24 Sep 2026 22:47:57 +0200 Subject: fix(key-server): handle missing keys consistently - normalize 404 and empty responses as plain not-found results - distinguish missing keys from transport failures in publication checks - add key-server not-found helpers and unit coverage - disable batch actions for key groups and empty targets - add publish and refresh actions to the key manager menu - reformat affected C++ declarations and callbacks --- src/m_key_server_sync/KeyServerSyncModule.cpp | 183 +++++++++++++++----------- 1 file changed, 103 insertions(+), 80 deletions(-) (limited to 'src/m_key_server_sync/KeyServerSyncModule.cpp') diff --git a/src/m_key_server_sync/KeyServerSyncModule.cpp b/src/m_key_server_sync/KeyServerSyncModule.cpp index 0c4122b..aae1387 100644 --- a/src/m_key_server_sync/KeyServerSyncModule.cpp +++ b/src/m_key_server_sync/KeyServerSyncModule.cpp @@ -90,8 +90,9 @@ using ErrorCallback = std::function; void FetchKey(const KeyServerList::Route& route, const QString& handle, bool by_fingerprint, const KeyCallback& on_key, const ErrorCallback& on_error, QObject* owner = nullptr) { - QObject* context = - owner != nullptr ? owner : static_cast(QThread::currentThread()); + QObject* context = owner != nullptr + ? owner + : static_cast(QThread::currentThread()); if (route.vks) { auto* vks = new VKSInterface(route.url, owner); QObject::connect(vks, &VKSInterface::SignalKeyRetrieved, context, @@ -118,16 +119,16 @@ void FetchKey(const KeyServerList::Route& route, const QString& handle, pks, &PKSInterface::SignalKeyServerKeyLookupResult, context, [on_key, on_error](QNetworkReply::NetworkError error, const QString& error_string, const QByteArray& data) { - if (error != QNetworkReply::NoError) { - on_error(error_string, QString::fromUtf8(data)); + // A 404, or the empty 200 some HKP servers answer instead, means + // "no such key": importing an empty answer would report success + // while changing nothing. + if (error == QNetworkReply::ContentNotFoundError || + (error == QNetworkReply::NoError && data.trimmed().isEmpty())) { + on_error(KeyServerBatchLogic::NotFoundText(), {}); return; } - // An empty 200 is how some HKP servers say "no such key"; importing it - // would report success while changing nothing. - if (data.trimmed().isEmpty()) { - on_error(QCoreApplication::translate( - "GTrC", "The key server did not return a key."), - {}); + if (error != QNetworkReply::NoError) { + on_error(error_string, QString::fromUtf8(data)); return; } on_key(QString::fromUtf8(data)); @@ -186,7 +187,8 @@ class KeyServerBatch : public QObject { static constexpr int kSpacingMs = 200; KeyServerBatch(Kind kind, QList keys) - : kind_(kind), keys_(std::move(keys)), + : kind_(kind), + keys_(std::move(keys)), route_(KeyServerList::SyncRoute()) {} /// @return false when the user declined, and the batch is already gone @@ -213,8 +215,8 @@ class KeyServerBatch : public QObject { Schedule(); }, [this, key](const QString& error, const QString&) { - failures_.append(QStringLiteral("%1: %2").arg(key.fingerprint, - error)); + failures_.append( + QStringLiteral("%1: %2").arg(key.fingerprint, error)); Schedule(); }, this); @@ -224,9 +226,8 @@ class KeyServerBatch : public QObject { } void Publish(const BatchKey& key) { - const auto exported = - gf::sdk::ExportKey(GFModuleSdkContext(), - static_cast(key.channel), key.key_id, true); + const auto exported = gf::sdk::ExportKey( + GFModuleSdkContext(), static_cast(key.channel), key.key_id, true); if (exported.isEmpty()) { failures_.append(QCoreApplication::translate( "GTrC", "%1: the public key could not be exported") @@ -243,14 +244,12 @@ class KeyServerBatch : public QObject { }; if (route_.vks) { auto* vks = new VKSInterface(route_.url, this); - connect(vks, &VKSInterface::SignalKeyUploaded, this, - [ok](const QString&, const QJsonObject&, const QString&) { - ok(); - }); - connect(vks, &VKSInterface::SignalErrorOccurred, this, - [failed](const QString& error, const QString&) { - failed(error); - }); + connect( + vks, &VKSInterface::SignalKeyUploaded, this, + [ok](const QString&, const QJsonObject&, const QString&) { ok(); }); + connect( + vks, &VKSInterface::SignalErrorOccurred, this, + [failed](const QString& error, const QString&) { failed(error); }); vks->UploadKey(QString::fromUtf8(exported)); return; } @@ -281,14 +280,13 @@ class KeyServerBatch : public QObject { } Tell(severity, QCoreApplication::translate("GTrC", "Key Refresh Finished"), - KeyServerBatchLogic::RefreshSummary( - static_cast(blocks_.size()), total, failures_)); + KeyServerBatchLogic::RefreshSummary(static_cast(blocks_.size()), + total, failures_)); } else { Tell(severity, QCoreApplication::translate("GTrC", "Key Publishing Finished"), - KeyServerBatchLogic::PublishSummary(done_, total, - QUrl(route_.url).host(), - failures_)); + KeyServerBatchLogic::PublishSummary( + done_, total, QUrl(route_.url).host(), failures_)); } deleteLater(); } @@ -309,8 +307,9 @@ void StartBatch(KeyServerBatch::Kind kind, QList keys) { if (!g_batch.isNull()) { Tell(Severity::kInfo, QCoreApplication::translate("GTrC", "Key Server"), QCoreApplication::translate( - "GTrC", "A key server operation is already running. Try again " - "when it has finished.")); + "GTrC", + "A key server operation is already running. Try again " + "when it has finished.")); return; } auto* batch = new KeyServerBatch(kind, std::move(keys)); @@ -369,12 +368,13 @@ auto UploadKeyToServer(int channel, const QString& key_id) -> int { const auto exported = gf::sdk::ExportKey(GFModuleSdkContext(), channel, key_id, true); if (exported.isEmpty()) { - Tell(Severity::kError, QCoreApplication::translate("GTrC", "Key Upload Failed"), - QCoreApplication::translate( - "GTrC", - "Failed to export the public key before uploading.\n" - "Key: %1") - .arg(key_id)); + Tell(Severity::kError, + QCoreApplication::translate("GTrC", "Key Upload Failed"), + QCoreApplication::translate( + "GTrC", + "Failed to export the public key before uploading.\n" + "Key: %1") + .arg(key_id)); return -1; } @@ -391,27 +391,29 @@ auto UploadKeyToServer(int channel, const QString& key_id) -> int { pks, &PKSInterface::SignalKeyServerKeyUploadResult, QThread::currentThread(), [server, key_id](QNetworkReply::NetworkError error, - const QString& error_string) { + const QString& error_string) { if (error != QNetworkReply::NoError) { - Tell(Severity::kError, QCoreApplication::translate("GTrC", "Key Upload Failed"), - QCoreApplication::translate( - "GTrC", - "Failed to upload public key to the server.\n" - "Fingerprint: %1\n" - "Error: %2") - .arg(key_id, error_string)); + Tell(Severity::kError, + QCoreApplication::translate("GTrC", "Key Upload Failed"), + QCoreApplication::translate( + "GTrC", + "Failed to upload public key to the server.\n" + "Fingerprint: %1\n" + "Error: %2") + .arg(key_id, error_string)); return; } // No verification mail follows an HKP upload, so do not promise one. - Tell(Severity::kInfo, QCoreApplication::translate("GTrC", - "Public Key Upload Successful"), - QCoreApplication::translate( - "GTrC", - "The public key was uploaded to the key server %2 over " - "HKP.\n" - "Fingerprint: %1") - .arg(key_id, QUrl(server).host())); + Tell(Severity::kInfo, + QCoreApplication::translate("GTrC", + "Public Key Upload Successful"), + QCoreApplication::translate( + "GTrC", + "The public key was uploaded to the key server %2 over " + "HKP.\n" + "Fingerprint: %1") + .arg(key_id, QUrl(server).host())); }); QObject::connect(pks, &PKSInterface::SignalKeyServerKeyUploadResult, pks, &PKSInterface::deleteLater); @@ -424,7 +426,7 @@ auto UploadKeyToServer(int channel, const QString& key_id) -> int { QObject::connect( vks, &VKSInterface::SignalKeyUploaded, QThread::currentThread(), [server](const QString& fpr, const QJsonObject& status, - const QString& token) { + const QString& token) { // Handle successful response QString status_message = QCoreApplication::translate( "GTrC", "The following email addresses have status:\n"); @@ -445,7 +447,9 @@ auto UploadKeyToServer(int channel, const QString& key_id) -> int { const auto host = QUrl(server).host(); // Notify user of successful upload and status details - Tell(Severity::kInfo, QCoreApplication::translate("GTrC", "Public Key Upload Successful"), + Tell( + Severity::kInfo, + QCoreApplication::translate("GTrC", "Public Key Upload Successful"), QCoreApplication::translate( "GTrC", "The public key was successfully uploaded to the " @@ -460,13 +464,14 @@ auto UploadKeyToServer(int channel, const QString& key_id) -> int { QObject::connect( vks, &VKSInterface::SignalErrorOccurred, QThread::currentThread(), [key_id](const QString& error, const QString& data) { - Tell(Severity::kError, QCoreApplication::translate("GTrC", "Key Upload Failed"), - QCoreApplication::translate( - "GTrC", - "Failed to upload public key to the server.\n" - "Fingerprint: %1\n" - "Error: %2") - .arg(key_id, error)); + Tell(Severity::kError, + QCoreApplication::translate("GTrC", "Key Upload Failed"), + QCoreApplication::translate( + "GTrC", + "Failed to upload public key to the server.\n" + "Fingerprint: %1\n" + "Error: %2") + .arg(key_id, error)); }); QObject::connect(vks, &VKSInterface::SignalKeyUploaded, vks, @@ -492,13 +497,14 @@ auto UpdateKeyFromKeyServer(int channel, const QString& fpr) -> int { Q_UNUSED(data); // Name the server: it is the user's choice now, and a failure they // cannot attribute to a host is one they cannot fix. - Tell(Severity::kError, QCoreApplication::translate("GTrC", "Key Update Failed"), - QCoreApplication::translate( - "GTrC", - "Failed to retrieve public key from %3.\n" - "Key ID: %1\n" - "Error: %2") - .arg(fpr, error, host)); + Tell(Severity::kError, + QCoreApplication::translate("GTrC", "Key Update Failed"), + QCoreApplication::translate( + "GTrC", + "Failed to retrieve public key from %3.\n" + "Key ID: %1\n" + "Error: %2") + .arg(fpr, error, host)); }); return 0; } @@ -536,18 +542,24 @@ auto KeysOf(const KeyArgs& a) -> QList { struct PublishKey { static constexpr gf::cmd::Meta kMeta{ - GF_MODULE_ID ".publish_key", GC_TR("Publish Public Key to Key Server"), + GF_MODULE_ID ".publish_key", + GC_TR("Publish Public Key to Key Server"), GC_TR("Upload the public key to the key server used for syncing"), - GC_TR("Key Server Operations"), 0, gf::cmd::kNeedsGuiThread}; + GC_TR("Key Server Operations"), + 0, + gf::cmd::kNeedsGuiThread}; using Args = KeyArgs; using Result = gf::cmd::Unit; }; struct RefreshKey { static constexpr gf::cmd::Meta kMeta{ - GF_MODULE_ID ".refresh_key", GC_TR("Refresh Public Key From Key Server"), + GF_MODULE_ID ".refresh_key", + GC_TR("Refresh Public Key From Key Server"), GC_TR("Import the latest copy of the public key from the key server"), - GC_TR("Key Server Operations"), 0, gf::cmd::kNeedsGuiThread}; + GC_TR("Key Server Operations"), + 0, + gf::cmd::kNeedsGuiThread}; using Args = KeyArgs; using Result = gf::cmd::Unit; }; @@ -557,15 +569,20 @@ struct CheckPublication { GF_MODULE_ID ".check_publication", GC_TR("Check Publication Status"), GC_TR("Ask the key server whether it has this public key"), - GC_TR("Key Server Operations"), 0, gf::cmd::kNeedsGuiThread}; + GC_TR("Key Server Operations"), + 0, + gf::cmd::kNeedsGuiThread}; using Args = KeyArgs; using Result = gf::cmd::Unit; }; struct SearchKey { static constexpr gf::cmd::Meta kMeta{ - GF_MODULE_ID ".search_key", GC_TR("Key Server"), - GC_TR("Import public keys from a trusted key server."), "", 0, + GF_MODULE_ID ".search_key", + GC_TR("Key Server"), + GC_TR("Import public keys from a trusted key server."), + "", + 0, gf::cmd::kNeedsGuiThread}; struct Args { std::optional fingerprint; @@ -620,8 +637,7 @@ auto DoCheckPublication(const gf::cmd::CommandContext& /*ctx*/, const auto fpr = keys.front().fingerprint; const auto route = KeyServerList::SyncRoute(); const auto host = QUrl(route.url).host(); - const auto title = - QCoreApplication::translate("GTrC", "Publication Status"); + const auto title = QCoreApplication::translate("GTrC", "Publication Status"); FetchKey( route, fpr, true, [title, host](const QString&) { @@ -631,9 +647,16 @@ auto DoCheckPublication(const gf::cmd::CommandContext& /*ctx*/, .arg(host)); }, [title, host](const QString& error, const QString&) { - Tell(Severity::kInfo, title, + if (KeyServerBatchLogic::IsNotFound(error)) { + Tell(Severity::kInfo, title, + QCoreApplication::translate( + "GTrC", "The public key is not published on %1.") + .arg(host)); + return; + } + Tell(Severity::kWarning, title, QCoreApplication::translate( - "GTrC", "%1 did not return this public key.\n%2") + "GTrC", "Could not ask %1 about this key.\n\n%2") .arg(host, error)); }); return gf::cmd::Outcome::Success({}); -- cgit