From a211d6607c1bd1a1bf0a399ba7e639647f5c4a50 Mon Sep 17 00:00:00 2001 From: Benjamin Marwell Date: Wed, 27 May 2026 16:35:37 +0200 Subject: [PATCH 1/5] feat: implement HKP op=index support (#137) - Add KeyIndexResult and UidIndexEntry as top-level records in application-api so the web layer only depends on primary ports - Add package-info.java (@NullMarked) to application-api root package - Extend KeyRepository secondary port with findManyBySearch() - Add keyserver-application-api dependency to application-port-repository - Add searchForIndex() to KeyRepositoryService and implement in PersistentKeyRepositoryService - Implement findManyBySearch() in JpaKeyRepository; refactor private search helpers (queryEntitiesByEmail, queryEntitiesByUidSubstring) to eliminate duplicated JPQL strings; both single- and multi-result paths share one query definition - Add HkpIndexRenderer (ApplicationScoped): renderMachineReadable() produces HKP info:/pub:/uid: format; renderHtml() a simple table - Wire op=index routing in LookupEndpoint (options=mr -> text/plain, else HTML; 404 when no results) - Add LookupEndpointIndexTest (7 tests): 404 on empty, info: header, pub: line format, UID percent-encoding, revoked/expired flags, HTML Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../application/api/KeyIndexResult.java | 25 +++ .../application/api/KeyRepositoryService.java | 11 + .../application/api/UidIndexEntry.java | 19 ++ .../application/api/package-info.java | 15 ++ .../core/PersistentKeyRepositoryService.java | 7 + .../VerifyUidCommandHandlerTest.java | 6 + .../application-port-repository/pom.xml | 8 + .../port/repository/KeyRepository.java | 17 +- .../repository/JpaKeyRepository.java | 117 +++++++++- .../keyserver/web/pks/HkpIndexRenderer.java | 125 +++++++++++ .../keyserver/web/pks/LookupEndpoint.java | 42 +++- .../web/pks/LookupEndpointIndexTest.java | 200 ++++++++++++++++++ 12 files changed, 577 insertions(+), 15 deletions(-) create mode 100644 application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/KeyIndexResult.java create mode 100644 application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/UidIndexEntry.java create mode 100644 application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/package-info.java create mode 100644 web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java create mode 100644 web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java diff --git a/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/KeyIndexResult.java b/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/KeyIndexResult.java new file mode 100644 index 0000000..fd822ba --- /dev/null +++ b/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/KeyIndexResult.java @@ -0,0 +1,25 @@ +/* + * Copyright (C) 2023-2024 The java-keyserver project team. + * + * SPDX-License-Identifier: EUPL-1.2 OR Apache-2.0 + */ +package io.github.bmarwell.keyserver.application.api; + +import java.time.OffsetDateTime; +import java.util.List; +import java.util.Optional; + +/// Full index entry for one key, used in `op=index` responses. +/// +/// Contains the key metadata fields needed for the HKP machine-readable +/// format (`pub:` line) plus all verified UIDs (`uid:` lines). +/// Only verified UIDs appear in {@code verifiedUids} — unverified UIDs +/// are never exposed. +public record KeyIndexResult( + String fingerprint, + int algorithm, + Optional bitStrength, + OffsetDateTime creationTime, + Optional expirationTime, + boolean revoked, + List verifiedUids) {} diff --git a/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/KeyRepositoryService.java b/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/KeyRepositoryService.java index aca4939..79181da 100644 --- a/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/KeyRepositoryService.java +++ b/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/KeyRepositoryService.java @@ -8,6 +8,7 @@ import io.github.bmarwell.keyserver.common.ids.KeyId; import io.github.bmarwell.keyserver.common.ids.PgpPublicKey; import io.github.bmarwell.keyserver.common.ids.RepositoryName; +import java.util.List; import java.util.Optional; /** @@ -30,4 +31,14 @@ public interface KeyRepositoryService { /// @param search HKP search string /// @param exactMatch if true, only exact fingerprint or email matches are returned Optional getArmoredKeyBySearch(String search, boolean exactMatch); + + /// Searches for all matching keys and returns their index metadata for `op=index` responses. + /// + /// Uses the same search routing as {@link #getArmoredKeyBySearch} but returns all matching + /// keys (not just the first) along with algorithm, timestamp, and verified-UID metadata + /// needed to render the HKP machine-readable index format. + /// + /// @param search HKP search string + /// @param exactMatch if true, only exact fingerprint or email matches are returned + List searchForIndex(String search, boolean exactMatch); } diff --git a/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/UidIndexEntry.java b/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/UidIndexEntry.java new file mode 100644 index 0000000..770e62a --- /dev/null +++ b/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/UidIndexEntry.java @@ -0,0 +1,19 @@ +/* + * Copyright (C) 2023-2024 The java-keyserver project team. + * + * SPDX-License-Identifier: EUPL-1.2 OR Apache-2.0 + */ +package io.github.bmarwell.keyserver.application.api; + +import java.time.OffsetDateTime; +import java.util.Optional; + +/// Metadata for a single verified UID, used in `op=index` responses. +/// +/// All timestamps are `Optional` because UID packets do not always carry +/// creation/expiration data (older keys often omit them). +public record UidIndexEntry( + String uidRaw, + Optional creationTime, + Optional expirationTime, + boolean revoked) {} diff --git a/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/package-info.java b/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/package-info.java new file mode 100644 index 0000000..b9540c4 --- /dev/null +++ b/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/package-info.java @@ -0,0 +1,15 @@ +/* + * Copyright (C) 2023-2024 The java-keyserver project team. + * + * SPDX-License-Identifier: EUPL-1.2 OR Apache-2.0 + */ + +/// Primary application port: service interfaces and shared query result types. +/// +/// All types in this package are non-null by default (jspecify `@NullMarked`). +/// Fields and parameters that may be `null` are explicitly annotated with +/// {@link org.jspecify.annotations.Nullable}. +@NullMarked +package io.github.bmarwell.keyserver.application.api; + +import org.jspecify.annotations.NullMarked; diff --git a/application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/PersistentKeyRepositoryService.java b/application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/PersistentKeyRepositoryService.java index f64fe2c..dbe22f4 100644 --- a/application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/PersistentKeyRepositoryService.java +++ b/application/application-core/src/main/java/io/github/bmarwell/keyserver/application/core/PersistentKeyRepositoryService.java @@ -5,6 +5,7 @@ */ package io.github.bmarwell.keyserver.application.core; +import io.github.bmarwell.keyserver.application.api.KeyIndexResult; import io.github.bmarwell.keyserver.application.api.KeyRepositoryService; import io.github.bmarwell.keyserver.application.port.repository.KeyRepository; import io.github.bmarwell.keyserver.common.ids.KeyId; @@ -14,6 +15,7 @@ import jakarta.enterprise.inject.Default; import jakarta.inject.Inject; import java.io.Serializable; +import java.util.List; import java.util.Optional; @Default @@ -38,6 +40,11 @@ public Optional getArmoredKeyBySearch(String search, boolean exactMatch) return keyRepository.findBySearch(search, exactMatch).map(KeyRepository.KeySearchResult::armoredKey); } + @Override + public List searchForIndex(String search, boolean exactMatch) { + return this.keyRepository.findManyBySearch(search, exactMatch); + } + // CDI-friendly setter for unit testing public void setKeyRepository(KeyRepository keyRepository) { this.keyRepository = keyRepository; diff --git a/application/application-core/src/test/java/io/github/bmarwell/keyserver/application/core/cmdhandler/VerifyUidCommandHandlerTest.java b/application/application-core/src/test/java/io/github/bmarwell/keyserver/application/core/cmdhandler/VerifyUidCommandHandlerTest.java index 11e5e4e..c22cf40 100644 --- a/application/application-core/src/test/java/io/github/bmarwell/keyserver/application/core/cmdhandler/VerifyUidCommandHandlerTest.java +++ b/application/application-core/src/test/java/io/github/bmarwell/keyserver/application/core/cmdhandler/VerifyUidCommandHandlerTest.java @@ -63,6 +63,12 @@ public void publishVerifiedUid(String fingerprint, String uidRaw, String uidEmai public Optional findBySearch(String search, boolean exactMatch) { return Optional.empty(); } + + @Override + public List findManyBySearch( + String search, boolean exactMatch) { + return List.of(); + } } /** diff --git a/application/application-ports/application-port-repository/pom.xml b/application/application-ports/application-port-repository/pom.xml index 375b5bd..f59b5e3 100644 --- a/application/application-ports/application-port-repository/pom.xml +++ b/application/application-ports/application-port-repository/pom.xml @@ -13,4 +13,12 @@ 0.1.0-SNAPSHOT Java Keyserver :: application :: port :: repository + + + io.github.bmarwell.keyserver + keyserver-application-api + 0.1.0-SNAPSHOT + + + diff --git a/application/application-ports/application-port-repository/src/main/java/io/github/bmarwell/keyserver/application/port/repository/KeyRepository.java b/application/application-ports/application-port-repository/src/main/java/io/github/bmarwell/keyserver/application/port/repository/KeyRepository.java index a25c9cf..5bf4757 100644 --- a/application/application-ports/application-port-repository/src/main/java/io/github/bmarwell/keyserver/application/port/repository/KeyRepository.java +++ b/application/application-ports/application-port-repository/src/main/java/io/github/bmarwell/keyserver/application/port/repository/KeyRepository.java @@ -5,6 +5,8 @@ */ package io.github.bmarwell.keyserver.application.port.repository; +import io.github.bmarwell.keyserver.application.api.KeyIndexResult; +import java.util.List; import java.util.Optional; /// Secondary (outbound) port for the published key store. @@ -17,7 +19,7 @@ public interface KeyRepository { /// Result type returned by key search queries. /// - /// Contains the minimal data required by the HKP `op=get` and `op=index` responses. + /// Contains the minimal data required by the HKP `op=get` response. /// The `armoredKey` field holds only the verified UIDs (privacy-preserving, per /// {@link #publishVerifiedUid} contract). record KeySearchResult(String fingerprint, String armoredKey) {} @@ -52,4 +54,17 @@ record KeySearchResult(String fingerprint, String armoredKey) {} /// @param search HKP search string /// @param exactMatch when true, only exact fingerprint/email matches are returned Optional findBySearch(String search, boolean exactMatch); + + /// Searches for keys by fingerprint, key ID, email address, or UID substring and returns + /// full index metadata for all matching keys. + /// + /// Uses the same search routing as {@link #findBySearch} but returns all matching keys + /// (not limited to the first) and includes per-key algorithm, timestamp, and verified-UID + /// metadata needed to render HKP `op=index` responses. + /// + /// Only verified UIDs are included in each {@link KeyIndexResult#verifiedUids()} list. + /// + /// @param search HKP search string + /// @param exactMatch when true, only exact fingerprint/email matches are returned + List findManyBySearch(String search, boolean exactMatch); } diff --git a/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java b/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java index 32e7473..87d70f6 100644 --- a/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java +++ b/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java @@ -5,6 +5,8 @@ */ package io.github.bmarwell.keyserver.repository; +import io.github.bmarwell.keyserver.application.api.KeyIndexResult; +import io.github.bmarwell.keyserver.application.api.UidIndexEntry; import io.github.bmarwell.keyserver.application.port.repository.KeyRepository; import io.github.bmarwell.keyserver.application.port.repository.KeyRepository.KeySearchResult; import io.github.bmarwell.keyserver.repository.entity.KeyEntity; @@ -151,6 +153,30 @@ public Optional findBySearch(String search, boolean exactMatch) return findByUidSubstring(normalized); } + @Override + @Transactional + public List findManyBySearch(String search, boolean exactMatch) { + if (search == null || search.isBlank()) { + return List.of(); + } + + String normalized = search.strip(); + + String hexCandidate = normalized.startsWith("0x") ? normalized.substring(2) : normalized; + if ((normalized.startsWith("0x") || isHexString(hexCandidate)) && isValidKeyIdLength(hexCandidate.length())) { + return findManyByKeyIdOrFingerprint(hexCandidate); + } + + if (normalized.contains("@")) { + return findManyByEmail(normalized.toLowerCase(Locale.ROOT), exactMatch); + } + + if (exactMatch) { + return List.of(); + } + return findManyByUidSubstring(normalized); + } + private Optional findByKeyIdOrFingerprint(String hexValue) { // Normalise to uppercase: fingerprint (and the DB-generated keyid_long column) are stored // as uppercase. Avoiding LOWER() on the column allows the DB to use the keys_keyid_long @@ -159,12 +185,10 @@ private Optional findByKeyIdOrFingerprint(String hexValue) { int len = upper.length(); if (len == 40 || len == 64) { - // Full fingerprint — primary key lookup (always indexed). return toResult(getEntityManager().find(KeyEntity.class, upper)); } if (len == 16) { - // Long key ID: exact match on the generated keyid_long column (uses keys_keyid_long index). List results = getEntityManager() .createQuery("SELECT k FROM KeyEntity k WHERE k.keyidLong = :keyId", KeyEntity.class) .setParameter("keyId", upper) @@ -174,8 +198,7 @@ private Optional findByKeyIdOrFingerprint(String hexValue) { } // Short key ID (8 chars): reverse it and use the rfingerprint text_pattern_ops index for a - // prefix scan. reverse(fingerprint) starts with reverse(shortKeyId), so this matches the - // last 8 chars of the fingerprint without a leading-wildcard LIKE on the forward column. + // prefix scan. String reversedShortId = new StringBuilder(upper).reverse().toString(); List results = getEntityManager() .createQuery("SELECT k FROM KeyEntity k WHERE k.rfingerprint LIKE :prefix", KeyEntity.class) @@ -185,31 +208,85 @@ private Optional findByKeyIdOrFingerprint(String hexValue) { return results.isEmpty() ? Optional.empty() : toResult(results.get(0)); } + private List findManyByKeyIdOrFingerprint(String hexValue) { + String upper = hexValue.toUpperCase(Locale.ROOT); + int len = upper.length(); + + if (len == 40 || len == 64) { + // Full fingerprint — primary key lookup (always indexed). + KeyEntity entity = getEntityManager().find(KeyEntity.class, upper); + return toIndexResults(entity == null ? List.of() : List.of(entity)); + } + + if (len == 16) { + // Long key ID: exact match on the generated keyid_long column (uses keys_keyid_long index). + List results = getEntityManager() + .createQuery("SELECT k FROM KeyEntity k WHERE k.keyidLong = :keyId", KeyEntity.class) + .setParameter("keyId", upper) + .getResultList(); + return toIndexResults(results); + } + + // Short key ID (8 chars): reverse it and use the rfingerprint text_pattern_ops index for a + // prefix scan. reverse(fingerprint) starts with reverse(shortKeyId), so this matches the + // last 8 chars of the fingerprint without a leading-wildcard LIKE on the forward column. + String reversedShortId = new StringBuilder(upper).reverse().toString(); + List results = getEntityManager() + .createQuery("SELECT k FROM KeyEntity k WHERE k.rfingerprint LIKE :prefix", KeyEntity.class) + .setParameter("prefix", reversedShortId + "%") + .getResultList(); + return toIndexResults(results); + } + private Optional findByEmail(String email, boolean exactMatch) { + List results = queryEntitiesByEmail(email, exactMatch, 1); + return results.isEmpty() ? Optional.empty() : toResult(results.get(0)); + } + + private List findManyByEmail(String email, boolean exactMatch) { + return toIndexResults(queryEntitiesByEmail(email, exactMatch, Integer.MAX_VALUE)); + } + + /// Returns KeyEntity rows for a given email filter. + /// + /// Keeps the JPQL in one place so both the single-result and multi-result paths + /// stay in sync when the schema changes. + private List queryEntitiesByEmail(String email, boolean exactMatch, int maxResults) { String jpql = exactMatch ? "SELECT DISTINCT k FROM KeyEntity k JOIN k.uids u" + " WHERE LOWER(u.uidEmail) = :email AND u.verified = true" : "SELECT DISTINCT k FROM KeyEntity k JOIN k.uids u" + " WHERE LOWER(u.uidEmail) LIKE :email AND u.verified = true"; String param = exactMatch ? email : "%" + email + "%"; - List results = getEntityManager() + return getEntityManager() .createQuery(jpql, KeyEntity.class) .setParameter("email", param) - .setMaxResults(1) + .setMaxResults(maxResults) .getResultList(); - return results.isEmpty() ? Optional.empty() : toResult(results.get(0)); } private Optional findByUidSubstring(String term) { - List results = getEntityManager() + List results = queryEntitiesByUidSubstring(term, 1); + return results.isEmpty() ? Optional.empty() : toResult(results.get(0)); + } + + private List findManyByUidSubstring(String term) { + return toIndexResults(queryEntitiesByUidSubstring(term, Integer.MAX_VALUE)); + } + + /// Returns KeyEntity rows whose raw UID text contains the given term (case-insensitive). + /// + /// Keeps the JPQL in one place so both the single-result and multi-result paths + /// stay in sync when the schema changes. + private List queryEntitiesByUidSubstring(String term, int maxResults) { + return getEntityManager() .createQuery( "SELECT DISTINCT k FROM KeyEntity k JOIN k.uids u" + " WHERE LOWER(u.uidRaw) LIKE :term AND u.verified = true", KeyEntity.class) .setParameter("term", "%" + term.toLowerCase(Locale.ROOT) + "%") - .setMaxResults(1) + .setMaxResults(maxResults) .getResultList(); - return results.isEmpty() ? Optional.empty() : toResult(results.get(0)); } private static Optional toResult(KeyEntity key) { @@ -219,6 +296,26 @@ private static Optional toResult(KeyEntity key) { return Optional.of(new KeySearchResult(key.getFingerprint(), key.getArmoredKey())); } + private static List toIndexResults(List keys) { + return keys.stream().map(JpaKeyRepository::toIndexResult).toList(); + } + + private static KeyIndexResult toIndexResult(KeyEntity key) { + List uids = key.getUids().stream() + .filter(UidEntity::isVerified) + .map(u -> new UidIndexEntry(u.getUidRaw(), u.getCreationTime(), u.getExpirationTime(), u.isRevoked())) + .toList(); + + return new KeyIndexResult( + key.getFingerprint(), + key.getAlgorithm(), + key.getBitStrength(), + key.getCreationTime(), + key.getExpirationTime(), + key.isRevoked(), + uids); + } + private static boolean isHexString(String s) { if (s.isEmpty()) { return false; diff --git a/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java b/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java new file mode 100644 index 0000000..dabaec0 --- /dev/null +++ b/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java @@ -0,0 +1,125 @@ +/* + * Copyright (C) 2023-2024 The java-keyserver project team. + * + * SPDX-License-Identifier: EUPL-1.2 OR Apache-2.0 + */ +package io.github.bmarwell.keyserver.web.pks; + +import io.github.bmarwell.keyserver.application.api.KeyIndexResult; +import io.github.bmarwell.keyserver.application.api.UidIndexEntry; +import jakarta.enterprise.context.ApplicationScoped; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; +import java.time.Instant; +import java.time.OffsetDateTime; +import java.util.List; +import java.util.Optional; + +/// Renders HKP `op=index` responses in machine-readable and HTML formats. +/// +/// Machine-readable format (`options=mr`) follows the HKP draft specification: +/// each key block starts with a `pub:` line containing fingerprint, algorithm, key length, +/// creation timestamp, expiration timestamp, and flags. Each verified UID follows as a +/// `uid:` line. The response begins with an `info:` header line. +/// +/// The HTML format is a simple table — sufficient for browser inspection but not required +/// for GnuPG interoperability. +@ApplicationScoped +public class HkpIndexRenderer { + + /// Renders the machine-readable HKP index format (`options=mr`). + /// + /// Format per the HKP draft: + /// 
+    /// info:1:<count>
+    /// pub:<fingerprint>:<algo>:<keylen>:<ctime>:<exptime>:<flags>
+    /// uid:<pct-encoded-uid>:<ctime>:<exptime>:<flags>
+    ///
+ /// + /// Flags: `r` if revoked, `e` if expired (expiration is in the past), or empty. + /// Key length is empty for ECC keys (bit strength is null), numeric for RSA/DSA. + public String renderMachineReadable(List results) { + Instant now = Instant.now(); + StringBuilder sb = new StringBuilder(); + sb.append("info:1:").append(results.size()).append('\n'); + for (KeyIndexResult key : results) { + sb.append("pub:"); + sb.append(key.fingerprint()).append(':'); + sb.append(key.algorithm()).append(':'); + key.bitStrength().ifPresent(bs -> sb.append(bs)); + sb.append(':'); + sb.append(toEpochSeconds(key.creationTime())).append(':'); + sb.append(key.expirationTime().map(HkpIndexRenderer::toEpochSeconds).orElse("")) + .append(':'); + sb.append(computeFlags(key.revoked(), key.expirationTime(), now)); + sb.append('\n'); + for (UidIndexEntry uid : key.verifiedUids()) { + sb.append("uid:"); + sb.append(URLEncoder.encode(uid.uidRaw(), StandardCharsets.UTF_8)) + .append(':'); + sb.append(uid.creationTime() + .map(HkpIndexRenderer::toEpochSeconds) + .orElse("")) + .append(':'); + sb.append(uid.expirationTime() + .map(HkpIndexRenderer::toEpochSeconds) + .orElse("")) + .append(':'); + sb.append(computeFlags(uid.revoked(), uid.expirationTime(), now)); + sb.append('\n'); + } + } + return sb.toString(); + } + + /// Renders a simple HTML table for browser display. + /// + /// The HTML is intentionally minimal — it is not required for GnuPG interoperability + /// and is provided as a human-readable fallback only. + public String renderHtml(List results) { + Instant now = Instant.now(); + StringBuilder sb = new StringBuilder(); + sb.append("

Search results: ").append(results.size()).append(" key(s)

\n"); + sb.append("" + + "\n"); + for (KeyIndexResult key : results) { + sb.append(""); + sb.append(""); + sb.append(""); + sb.append(""); + sb.append(""); + sb.append(""); + sb.append(""); + sb.append("\n"); + } + sb.append("
FingerprintAlgorithmCreatedExpiresFlagsUIDs
").append(htmlEscape(key.fingerprint())).append("").append(key.algorithm()).append("").append(key.creationTime()).append("") + .append(key.expirationTime().map(Object::toString).orElse("")) + .append("") + .append(computeFlags(key.revoked(), key.expirationTime(), now)) + .append(""); + for (UidIndexEntry uid : key.verifiedUids()) { + sb.append(htmlEscape(uid.uidRaw())).append("
"); + } + sb.append("
"); + return sb.toString(); + } + + private static String toEpochSeconds(OffsetDateTime dt) { + return String.valueOf(dt.toEpochSecond()); + } + + private static String computeFlags(boolean revoked, Optional expiration, Instant now) { + StringBuilder flags = new StringBuilder(); + if (revoked) { + flags.append('r'); + } + if (expiration.isPresent() && expiration.get().toInstant().isBefore(now)) { + flags.append('e'); + } + return flags.toString(); + } + + private static String htmlEscape(String s) { + return s.replace("&", "&").replace("<", "<").replace(">", ">").replace("\"", """); + } +} diff --git a/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/LookupEndpoint.java b/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/LookupEndpoint.java index f9d83ed..61e9c54 100644 --- a/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/LookupEndpoint.java +++ b/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/LookupEndpoint.java @@ -5,19 +5,21 @@ */ package io.github.bmarwell.keyserver.web.pks; +import io.github.bmarwell.keyserver.application.api.KeyIndexResult; import io.github.bmarwell.keyserver.application.api.KeyRepositoryService; import jakarta.inject.Inject; import jakarta.ws.rs.GET; import jakarta.ws.rs.Path; import jakarta.ws.rs.QueryParam; import jakarta.ws.rs.core.Response; +import java.util.List; import java.util.Optional; import org.eclipse.microprofile.openapi.annotations.tags.Tag; /// HKP `/pks/lookup` endpoint. /// -/// Handles `op=get` (return ASCII-armored key block) and stubs `op=index`/`op=vindex` -/// (machine-readable listing — future work). +/// Handles `op=get` (return ASCII-armored key block) and `op=index` +/// (machine-readable or HTML key listing). /// /// Only keys with at least one verified UID are returned. Search by fingerprint /// (`0x`), long/short key ID, email address, or UID substring. @@ -28,9 +30,15 @@ public class LookupEndpoint { @Inject KeyRepositoryService keyRepositoryService; + @Inject + HkpIndexRenderer hkpIndexRenderer; + @GET public Response doLookup( - @QueryParam("op") String op, @QueryParam("search") String search, @QueryParam("exact") String exact) { + @QueryParam("op") String op, + @QueryParam("search") String search, + @QueryParam("exact") String exact, + @QueryParam("options") String options) { // Both null/blank op and search are invalid — HKP requires both to be present. // Report exactly which parameter(s) are missing so the client can self-correct. @@ -50,7 +58,12 @@ public Response doLookup( return handleGet(search, exactMatch); } - // op=index and op=vindex are future work + if ("index".equalsIgnoreCase(op)) { + boolean machineReadable = options != null && options.contains("mr"); + return handleIndex(search, exactMatch, machineReadable); + } + + // op=vindex and unknown ops return Response.status(Response.Status.NOT_IMPLEMENTED) .entity("op=" + op + " is not yet implemented") .type("text/plain") @@ -73,8 +86,29 @@ private Response handleGet(String search, boolean exactMatch) { .build(); } + private Response handleIndex(String search, boolean exactMatch, boolean machineReadable) { + List results = this.keyRepositoryService.searchForIndex(search, exactMatch); + if (results.isEmpty()) { + return Response.status(Response.Status.NOT_FOUND) + .entity("No keys found for the provided search term") + .type("text/plain") + .build(); + } + if (machineReadable) { + String body = this.hkpIndexRenderer.renderMachineReadable(results); + return Response.ok(body).type("text/plain; charset=utf-8").build(); + } + String body = this.hkpIndexRenderer.renderHtml(results); + return Response.ok(body).type("text/html; charset=utf-8").build(); + } + // CDI-friendly setter for unit testing public void setKeyRepositoryService(KeyRepositoryService keyRepositoryService) { this.keyRepositoryService = keyRepositoryService; } + + // CDI-friendly setter for unit testing + public void setHkpIndexRenderer(HkpIndexRenderer hkpIndexRenderer) { + this.hkpIndexRenderer = hkpIndexRenderer; + } } diff --git a/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java b/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java new file mode 100644 index 0000000..9867077 --- /dev/null +++ b/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java @@ -0,0 +1,200 @@ +/* + * Copyright (C) 2023-2024 The java-keyserver project team. + * + * SPDX-License-Identifier: EUPL-1.2 OR Apache-2.0 + */ +package io.github.bmarwell.keyserver.web.pks; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.github.bmarwell.keyserver.application.api.KeyIndexResult; +import io.github.bmarwell.keyserver.application.api.KeyRepositoryService; +import io.github.bmarwell.keyserver.application.api.UidIndexEntry; +import jakarta.ws.rs.core.Response; +import java.time.OffsetDateTime; +import java.time.ZoneOffset; +import java.util.List; +import java.util.Optional; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class LookupEndpointIndexTest { + + private static final String FINGERPRINT = "A1B2C3D4E5F60708091011121314151617181920A1B2"; + private static final OffsetDateTime CREATION = OffsetDateTime.of(2024, 1, 15, 12, 0, 0, 0, ZoneOffset.UTC); + private static final String UID_RAW = "Alice "; + + private LookupEndpoint endpoint; + private FakeKeyRepositoryService fakeService; + + @BeforeEach + void setUp() { + this.fakeService = new FakeKeyRepositoryService(); + this.endpoint = new LookupEndpoint(); + this.endpoint.setKeyRepositoryService(this.fakeService); + this.endpoint.setHkpIndexRenderer(new HkpIndexRenderer()); + } + + @Test + void returnsNotFoundWhenNoKeyMatchesIndexSearch() { + // given — the repository contains no keys matching the search + this.fakeService.indexResults = List.of(); + + // when — the client sends op=index with options=mr + Response response = this.endpoint.doLookup("index", "alice@example.com", null, "mr"); + + // then — a 404 is returned so GnuPG knows no key was found + assertThat(response.getStatus()) + .as("empty index results must yield 404 so HKP clients know no key was found") + .isEqualTo(404); + } + + @Test + void rendersMachineReadableInfoLineWithKeyCount() { + // given — one key with one verified UID in the repository + UidIndexEntry uid = new UidIndexEntry(UID_RAW, Optional.of(CREATION), Optional.empty(), false); + KeyIndexResult key = + new KeyIndexResult(FINGERPRINT, 22, Optional.empty(), CREATION, Optional.empty(), false, List.of(uid)); + this.fakeService.indexResults = List.of(key); + + // when — the client sends op=index&options=mr (machine-readable) + Response response = this.endpoint.doLookup("index", "alice@example.com", null, "mr"); + String body = (String) response.getEntity(); + + // then — the info header must contain the key count so GnuPG can allocate result structures + assertThat(response.getStatus()).isEqualTo(200); + assertThat(body) + .as("machine-readable response must start with info:1: per HKP draft spec") + .startsWith("info:1:1\n"); + } + + @Test + void rendersMachineReadablePubLine() { + // given — one RSA-2048 key (algorithm 1, bitStrength 2048) with a known creation time + UidIndexEntry uid = new UidIndexEntry(UID_RAW, Optional.of(CREATION), Optional.empty(), false); + KeyIndexResult key = + new KeyIndexResult(FINGERPRINT, 1, Optional.of(2048), CREATION, Optional.empty(), false, List.of(uid)); + this.fakeService.indexResults = List.of(key); + + // when — client requests machine-readable index + Response response = this.endpoint.doLookup("index", "alice@example.com", null, "mr"); + String body = (String) response.getEntity(); + + // then — pub: line must carry fingerprint, algorithm, keylen, epoch seconds, expiry (empty), and flags + // The format is required by GnuPG for trust lookups: missing fields break key import + assertThat(body) + .as("pub line must contain fingerprint and algorithm for GnuPG to import the key") + .contains("pub:" + FINGERPRINT + ":1:2048:" + CREATION.toEpochSecond() + "::\n"); + } + + @Test + void rendersMachineReadableUidLinePercentEncoded() { + // given — a UID string containing characters that need percent-encoding ('<', '>', ' ') + UidIndexEntry uid = new UidIndexEntry(UID_RAW, Optional.of(CREATION), Optional.empty(), false); + KeyIndexResult key = + new KeyIndexResult(FINGERPRINT, 22, Optional.empty(), CREATION, Optional.empty(), false, List.of(uid)); + this.fakeService.indexResults = List.of(key); + + // when — client requests machine-readable index + Response response = this.endpoint.doLookup("index", "alice@example.com", null, "mr"); + String body = (String) response.getEntity(); + + // then — UID must be percent-encoded so clients can safely parse the colon-delimited format + assertThat(body) + .as("UID must be percent-encoded because colons and special chars would break the field delimiter") + .contains("uid:Alice+%3Calice%40example.com%3E:"); + } + + @Test + void setsFlagRForRevokedKey() { + // given — a revoked key without expiration + UidIndexEntry uid = new UidIndexEntry(UID_RAW, Optional.of(CREATION), Optional.empty(), false); + KeyIndexResult key = + new KeyIndexResult(FINGERPRINT, 22, Optional.empty(), CREATION, Optional.empty(), true, List.of(uid)); + this.fakeService.indexResults = List.of(key); + + // when — client requests machine-readable index + Response response = this.endpoint.doLookup("index", "alice@example.com", null, "mr"); + String body = (String) response.getEntity(); + + // then — the pub: flags field must contain 'r' so HKP clients display it as revoked + // format: pub:::::: + assertThat(body) + .as("revoked key must have 'r' in flags so HKP clients display it as revoked") + .contains("pub:" + FINGERPRINT + ":22::" + CREATION.toEpochSecond() + "::r\n"); + } + + @Test + void setsFlagEForExpiredKey() { + // given — a key whose expiration time is clearly in the past (year 2000) + OffsetDateTime pastExpiry = OffsetDateTime.of(2000, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC); + UidIndexEntry uid = new UidIndexEntry(UID_RAW, Optional.of(CREATION), Optional.of(pastExpiry), false); + KeyIndexResult key = new KeyIndexResult( + FINGERPRINT, 22, Optional.empty(), CREATION, Optional.of(pastExpiry), false, List.of(uid)); + this.fakeService.indexResults = List.of(key); + + // when — client requests machine-readable index + Response response = this.endpoint.doLookup("index", "alice@example.com", null, "mr"); + String body = (String) response.getEntity(); + + // then — the pub: flags field must contain 'e' so GnuPG knows to mark the key as expired + assertThat(body) + .as("expired key must have 'e' in flags so GnuPG does not attempt to use it for encryption") + .contains(":e\n"); + } + + @Test + void rendersHtmlWhenOptionsMrAbsent() { + // given — one key in the repository, no options=mr in the request + UidIndexEntry uid = new UidIndexEntry(UID_RAW, Optional.of(CREATION), Optional.empty(), false); + KeyIndexResult key = + new KeyIndexResult(FINGERPRINT, 22, Optional.empty(), CREATION, Optional.empty(), false, List.of(uid)); + this.fakeService.indexResults = List.of(key); + + // when — client sends op=index without options=mr (browser request) + Response response = this.endpoint.doLookup("index", "alice@example.com", null, null); + String body = (String) response.getEntity(); + + // then — response must be HTML so it can be displayed in a browser for human inspection + assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.getHeaderString("Content-Type")) + .as("browser requests (no options=mr) must receive HTML for human readability") + .startsWith("text/html"); + assertThat(body) + .as("HTML body must contain the fingerprint so users can identify the key") + .contains(FINGERPRINT); + } + + // --------------------------------------------------------------------------- + // Test double — avoids CDI/Mockito overhead; keeps the test focused on the + // endpoint's routing and response-building logic only + // --------------------------------------------------------------------------- + + private static final class FakeKeyRepositoryService implements KeyRepositoryService { + + List indexResults = List.of(); + + @Override + public Optional getArmoredKeyBySearch(String search, boolean exactMatch) { + return Optional.empty(); + } + + @Override + public List searchForIndex(String search, boolean exactMatch) { + return this.indexResults; + } + + @Override + public void getKeyByRepoAndKeyId( + io.github.bmarwell.keyserver.common.ids.RepositoryName repoName, + io.github.bmarwell.keyserver.common.ids.KeyId keyId) { + throw new UnsupportedOperationException("not needed in this test"); + } + + @Override + public Optional getKeyByKeyId( + io.github.bmarwell.keyserver.common.ids.KeyId keyId) { + return Optional.empty(); + } + } +} From 25dd46290fed35cfb7701f2116b67f0cc2abdd92 Mon Sep 17 00:00:00 2001 From: Benjamin Marwell Date: Wed, 27 May 2026 19:54:51 +0200 Subject: [PATCH 2/5] fix: address review comments on op=index implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Cap multi-result index queries at 5_000 rows (INDEX_RESULT_LIMIT constant) to prevent unbounded memory use on broad search terms; applies to both email and UID-substring paths - Replace '+' with '%20' in UID percent-encoding: URLEncoder uses form-encoding (space→'+') but '+' is a legal literal in UID strings and would be mis-decoded by strict RFC 3986 clients - Strengthen expired-key test: assert the full pub: line including the expiration epoch and ':e' flag position, not just ':e\n', so the test targets the key-level flag specifically rather than the uid: line Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../repository/JpaKeyRepository.java | 20 +++++++++++++++---- .../keyserver/web/pks/HkpIndexRenderer.java | 7 ++++++- .../web/pks/LookupEndpointIndexTest.java | 18 +++++++++++------ 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java b/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java index 87d70f6..23d1960 100644 --- a/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java +++ b/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java @@ -52,6 +52,14 @@ @ApplicationScoped public class JpaKeyRepository extends BaseRepository implements KeyRepository { + /// Maximum number of keys returned by multi-result index queries. + /// + /// A broad search term (e.g. a common email domain or UID substring) could otherwise + /// load an unbounded number of key rows — together with their UID collections — into a + /// single JPA transaction. 5 000 matches what popular HKP clients are expected to handle + /// and prevents memory exhaustion under adversarial inputs. + private static final int INDEX_RESULT_LIMIT = 5_000; + @Override @Transactional public void publishVerifiedUid(String fingerprint, String uidRaw, String uidEmail, String armoredKey) { @@ -244,13 +252,15 @@ private Optional findByEmail(String email, boolean exactMatch) } private List findManyByEmail(String email, boolean exactMatch) { - return toIndexResults(queryEntitiesByEmail(email, exactMatch, Integer.MAX_VALUE)); + return toIndexResults(queryEntitiesByEmail(email, exactMatch, INDEX_RESULT_LIMIT)); } /// Returns KeyEntity rows for a given email filter. /// /// Keeps the JPQL in one place so both the single-result and multi-result paths - /// stay in sync when the schema changes. + /// stay in sync when the schema changes. Callers that want all results should pass + /// {@link #INDEX_RESULT_LIMIT} rather than {@link Integer#MAX_VALUE} to prevent + /// unbounded memory use. private List queryEntitiesByEmail(String email, boolean exactMatch, int maxResults) { String jpql = exactMatch ? "SELECT DISTINCT k FROM KeyEntity k JOIN k.uids u" @@ -271,13 +281,15 @@ private Optional findByUidSubstring(String term) { } private List findManyByUidSubstring(String term) { - return toIndexResults(queryEntitiesByUidSubstring(term, Integer.MAX_VALUE)); + return toIndexResults(queryEntitiesByUidSubstring(term, INDEX_RESULT_LIMIT)); } /// Returns KeyEntity rows whose raw UID text contains the given term (case-insensitive). /// /// Keeps the JPQL in one place so both the single-result and multi-result paths - /// stay in sync when the schema changes. + /// stay in sync when the schema changes. Callers that want all results should pass + /// {@link #INDEX_RESULT_LIMIT} rather than {@link Integer#MAX_VALUE} to prevent + /// unbounded memory use. private List queryEntitiesByUidSubstring(String term, int maxResults) { return getEntityManager() .createQuery( diff --git a/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java b/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java index dabaec0..8690ef4 100644 --- a/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java +++ b/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java @@ -55,7 +55,12 @@ public String renderMachineReadable(List results) { sb.append('\n'); for (UidIndexEntry uid : key.verifiedUids()) { sb.append("uid:"); - sb.append(URLEncoder.encode(uid.uidRaw(), StandardCharsets.UTF_8)) + // Use %20 for spaces (RFC 3986 percent-encoding) rather than +. + // URLEncoder uses application/x-www-form-urlencoded which encodes spaces as '+', + // but '+' is a legal literal character in UID strings and would be mis-decoded + // by strict HKP clients expecting RFC 3986. + sb.append(URLEncoder.encode(uid.uidRaw(), StandardCharsets.UTF_8) + .replace("+", "%20")) .append(':'); sb.append(uid.creationTime() .map(HkpIndexRenderer::toEpochSeconds) diff --git a/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java b/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java index 9867077..5139854 100644 --- a/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java +++ b/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java @@ -99,10 +99,12 @@ void rendersMachineReadableUidLinePercentEncoded() { Response response = this.endpoint.doLookup("index", "alice@example.com", null, "mr"); String body = (String) response.getEntity(); - // then — UID must be percent-encoded so clients can safely parse the colon-delimited format + // then — UID must be percent-encoded using %20 for spaces (RFC 3986), not + (form-encoding) + // '+' is a legal literal character in UID strings and would be mis-decoded by strict clients assertThat(body) - .as("UID must be percent-encoded because colons and special chars would break the field delimiter") - .contains("uid:Alice+%3Calice%40example.com%3E:"); + .as("UID must use %20 for spaces, not '+', because '+' is legal in UID strings and" + + " would be mis-decoded by strict RFC 3986 percent-encoding clients") + .contains("uid:Alice%20%3Calice%40example.com%3E:"); } @Test @@ -137,10 +139,14 @@ void setsFlagEForExpiredKey() { Response response = this.endpoint.doLookup("index", "alice@example.com", null, "mr"); String body = (String) response.getEntity(); - // then — the pub: flags field must contain 'e' so GnuPG knows to mark the key as expired + // then — the pub: flags field (not just the uid: flags) must contain 'e' + // format: pub:::::: + // Asserting the full pub: line ensures we test the key-level flag specifically, + // not just the uid: line which would also show 'e' for the same expiry assertThat(body) - .as("expired key must have 'e' in flags so GnuPG does not attempt to use it for encryption") - .contains(":e\n"); + .as("expired key must have 'e' in flags on the pub: line so GnuPG knows not to use it for encryption") + .contains("pub:" + FINGERPRINT + ":22::" + CREATION.toEpochSecond() + ":" + pastExpiry.toEpochSecond() + + ":e\n"); } @Test From ded7efae97e6c08d9a60ed3d3b1188f063d167d5 Mon Sep 17 00:00:00 2001 From: Benjamin Marwell Date: Wed, 27 May 2026 20:33:12 +0200 Subject: [PATCH 3/5] fix(index): add 'd' flag support and fix N+1 query on op=index - Add DB migration V010: 'disabled' BOOLEAN column on keys table (default FALSE) - Add 'disabled' field to KeyEntity with getter/setter - Add 'disabled' component to KeyIndexResult record - HkpIndexRenderer.computeFlags now emits 'd' between 'r' and 'e' per HKP spec - Key-level pub: line passes key.disabled(); uid: lines always pass false - Fix N+1 query for multi-result index paths (findManyByEmail, findManyByUidSubstring): - Two-query pattern: first query selects fingerprints only with setMaxResults() so the SQL LIMIT applies cleanly on a scalar query without pagination/JOIN FETCH conflict - Second query does JOIN FETCH k.uids WHERE fingerprint IN :fps for correct eager loading - Fix findManyByKeyIdOrFingerprint: use JOIN FETCH k.uids with DISTINCT on all paths (fingerprint, long key ID, short key ID) to avoid N+1 per key - Add JPA provider read-only hints (Hibernate, EclipseLink, Apache OpenJPA) on all multi-result index queries to skip dirty tracking and lock acquisition - Add test setsFlagDForDisabledKey() in LookupEndpointIndexTest - Update all existing KeyIndexResult constructions for new 'disabled' component Closes #137 review comments (N+1 query, 'd' flag) Related: #153 (search result ranking, filed separately) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../application/api/KeyIndexResult.java | 5 + .../repository/JpaKeyRepository.java | 143 +++++++++++++++--- .../repository/entity/KeyEntity.java | 15 ++ .../V010__add-disabled-flag-to-keys.sql | 5 + .../keyserver/web/pks/HkpIndexRenderer.java | 18 ++- .../web/pks/LookupEndpointIndexTest.java | 41 +++-- 6 files changed, 189 insertions(+), 38 deletions(-) create mode 100644 repository/src/main/resources/io/github/bmarwell/keyserver/repository/migrations/V010__add-disabled-flag-to-keys.sql diff --git a/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/KeyIndexResult.java b/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/KeyIndexResult.java index fd822ba..8e751ec 100644 --- a/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/KeyIndexResult.java +++ b/application/application-api/src/main/java/io/github/bmarwell/keyserver/application/api/KeyIndexResult.java @@ -15,6 +15,10 @@ /// format (`pub:` line) plus all verified UIDs (`uid:` lines). /// Only verified UIDs appear in {@code verifiedUids} — unverified UIDs /// are never exposed. +/// +/// The {@code disabled} flag is a keyserver-administrative flag (HKP `d`). +/// It is independent of the OpenPGP {@code revoked} flag: an operator may +/// disable a key on this server without the key owner having revoked it. public record KeyIndexResult( String fingerprint, int algorithm, @@ -22,4 +26,5 @@ public record KeyIndexResult( OffsetDateTime creationTime, Optional expirationTime, boolean revoked, + boolean disabled, List verifiedUids) {} diff --git a/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java b/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java index 23d1960..3046b8b 100644 --- a/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java +++ b/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java @@ -14,6 +14,7 @@ import jakarta.enterprise.context.ApplicationScoped; import jakarta.enterprise.inject.Default; import jakarta.persistence.LockModeType; +import jakarta.persistence.TypedQuery; import jakarta.transaction.Transactional; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -60,6 +61,24 @@ public class JpaKeyRepository extends BaseRepository implements KeyRepository { /// and prevents memory exhaustion under adversarial inputs. private static final int INDEX_RESULT_LIMIT = 5_000; + // ------------------------------------------------------------------------- + // JPA provider read-only query hints. + // + // Index queries map results directly to immutable DTOs; the entities are + // never written back. Telling each provider to skip dirty tracking and lock + // acquisition reduces heap pressure and eliminates unnecessary lock rows. + // Unknown hints are silently ignored by every compliant JPA provider. + // ------------------------------------------------------------------------- + + /// Hibernate: mark loaded entities as read-only so dirty checking is skipped. + private static final String HINT_HIBERNATE_READ_ONLY = "org.hibernate.readOnly"; + + /// EclipseLink: detach objects immediately after loading (read-only session). + private static final String HINT_ECLIPSELINK_READ_ONLY = "eclipselink.read-only"; + + /// Apache OpenJPA: acquire no read lock, reducing contention on index queries. + private static final String HINT_OPENJPA_READ_LOCK_MODE = "openjpa.FetchPlan.ReadLockMode"; + @Override @Transactional public void publishVerifiedUid(String fingerprint, String uidRaw, String uidEmail, String armoredKey) { @@ -221,16 +240,24 @@ private List findManyByKeyIdOrFingerprint(String hexValue) { int len = upper.length(); if (len == 40 || len == 64) { - // Full fingerprint — primary key lookup (always indexed). - KeyEntity entity = getEntityManager().find(KeyEntity.class, upper); - return toIndexResults(entity == null ? List.of() : List.of(entity)); + // Full fingerprint — switch from em.find() to JPQL so JOIN FETCH eagerly loads UIDs. + List results = applyReadOnlyHints(getEntityManager() + .createQuery( + "SELECT DISTINCT k FROM KeyEntity k JOIN FETCH k.uids" + + " WHERE k.fingerprint = :fp", + KeyEntity.class) + .setParameter("fp", upper)) + .getResultList(); + return toIndexResults(results); } if (len == 16) { - // Long key ID: exact match on the generated keyid_long column (uses keys_keyid_long index). - List results = getEntityManager() - .createQuery("SELECT k FROM KeyEntity k WHERE k.keyidLong = :keyId", KeyEntity.class) - .setParameter("keyId", upper) + List results = applyReadOnlyHints(getEntityManager() + .createQuery( + "SELECT DISTINCT k FROM KeyEntity k JOIN FETCH k.uids" + + " WHERE k.keyidLong = :keyId", + KeyEntity.class) + .setParameter("keyId", upper)) .getResultList(); return toIndexResults(results); } @@ -239,9 +266,12 @@ private List findManyByKeyIdOrFingerprint(String hexValue) { // prefix scan. reverse(fingerprint) starts with reverse(shortKeyId), so this matches the // last 8 chars of the fingerprint without a leading-wildcard LIKE on the forward column. String reversedShortId = new StringBuilder(upper).reverse().toString(); - List results = getEntityManager() - .createQuery("SELECT k FROM KeyEntity k WHERE k.rfingerprint LIKE :prefix", KeyEntity.class) - .setParameter("prefix", reversedShortId + "%") + List results = applyReadOnlyHints(getEntityManager() + .createQuery( + "SELECT DISTINCT k FROM KeyEntity k JOIN FETCH k.uids" + + " WHERE k.rfingerprint LIKE :prefix", + KeyEntity.class) + .setParameter("prefix", reversedShortId + "%")) .getResultList(); return toIndexResults(results); } @@ -252,15 +282,14 @@ private Optional findByEmail(String email, boolean exactMatch) } private List findManyByEmail(String email, boolean exactMatch) { - return toIndexResults(queryEntitiesByEmail(email, exactMatch, INDEX_RESULT_LIMIT)); + List fingerprints = queryFingerprintsByEmail(email, exactMatch); + return fingerprints.isEmpty() ? List.of() : fetchIndexResultsByFingerprints(fingerprints); } - /// Returns KeyEntity rows for a given email filter. + /// Returns at most one KeyEntity row for a given email filter (single-result path). /// - /// Keeps the JPQL in one place so both the single-result and multi-result paths - /// stay in sync when the schema changes. Callers that want all results should pass - /// {@link #INDEX_RESULT_LIMIT} rather than {@link Integer#MAX_VALUE} to prevent - /// unbounded memory use. + /// Keeps the JPQL in one place; callers must always pass {@code maxResults = 1}. + /// The multi-result path uses {@link #queryFingerprintsByEmail} instead. private List queryEntitiesByEmail(String email, boolean exactMatch, int maxResults) { String jpql = exactMatch ? "SELECT DISTINCT k FROM KeyEntity k JOIN k.uids u" @@ -275,21 +304,40 @@ private List queryEntitiesByEmail(String email, boolean exactMatch, i .getResultList(); } + /// First query of the two-query pattern for email index searches. + /// + /// Selects fingerprints only (no collection fetch) so that {@link #INDEX_RESULT_LIMIT} + /// is applied correctly at the SQL level. The caller then passes the fingerprint list + /// to {@link #fetchIndexResultsByFingerprints} which loads the full entity graph + /// via {@code JOIN FETCH} without any conflicting pagination. + private List queryFingerprintsByEmail(String email, boolean exactMatch) { + String jpql = exactMatch + ? "SELECT DISTINCT k.fingerprint FROM KeyEntity k JOIN k.uids u" + + " WHERE LOWER(u.uidEmail) = :email AND u.verified = true" + : "SELECT DISTINCT k.fingerprint FROM KeyEntity k JOIN k.uids u" + + " WHERE LOWER(u.uidEmail) LIKE :email AND u.verified = true"; + String param = exactMatch ? email : "%" + email + "%"; + return applyReadOnlyHints(getEntityManager() + .createQuery(jpql, String.class) + .setParameter("email", param) + .setMaxResults(INDEX_RESULT_LIMIT)) + .getResultList(); + } + private Optional findByUidSubstring(String term) { List results = queryEntitiesByUidSubstring(term, 1); return results.isEmpty() ? Optional.empty() : toResult(results.get(0)); } private List findManyByUidSubstring(String term) { - return toIndexResults(queryEntitiesByUidSubstring(term, INDEX_RESULT_LIMIT)); + List fingerprints = queryFingerprintsByUidSubstring(term); + return fingerprints.isEmpty() ? List.of() : fetchIndexResultsByFingerprints(fingerprints); } - /// Returns KeyEntity rows whose raw UID text contains the given term (case-insensitive). + /// Returns at most one KeyEntity row whose raw UID text contains the given term (single-result path). /// - /// Keeps the JPQL in one place so both the single-result and multi-result paths - /// stay in sync when the schema changes. Callers that want all results should pass - /// {@link #INDEX_RESULT_LIMIT} rather than {@link Integer#MAX_VALUE} to prevent - /// unbounded memory use. + /// Callers must always pass {@code maxResults = 1}. + /// The multi-result path uses {@link #queryFingerprintsByUidSubstring} instead. private List queryEntitiesByUidSubstring(String term, int maxResults) { return getEntityManager() .createQuery( @@ -301,6 +349,35 @@ private List queryEntitiesByUidSubstring(String term, int maxResults) .getResultList(); } + /// First query of the two-query pattern for UID substring index searches. + /// + /// Selects fingerprints only so {@link #INDEX_RESULT_LIMIT} is applied at the SQL level. + private List queryFingerprintsByUidSubstring(String term) { + return applyReadOnlyHints(getEntityManager() + .createQuery( + "SELECT DISTINCT k.fingerprint FROM KeyEntity k JOIN k.uids u" + + " WHERE LOWER(u.uidRaw) LIKE :term AND u.verified = true", + String.class) + .setParameter("term", "%" + term.toLowerCase(Locale.ROOT) + "%") + .setMaxResults(INDEX_RESULT_LIMIT)) + .getResultList(); + } + + /// Second query of the two-query pattern: loads full key entities with their UIDs eagerly. + /// + /// Because the fingerprint list was already bounded by {@link #INDEX_RESULT_LIMIT} in the + /// first query, this {@code JOIN FETCH} query has no conflicting pagination and the JPA + /// provider can apply the join at the SQL level without in-memory row reduction. + private List fetchIndexResultsByFingerprints(List fingerprints) { + List entities = applyReadOnlyHints(getEntityManager() + .createQuery( + "SELECT DISTINCT k FROM KeyEntity k JOIN FETCH k.uids" + " WHERE k.fingerprint IN :fps", + KeyEntity.class) + .setParameter("fps", fingerprints)) + .getResultList(); + return toIndexResults(entities); + } + private static Optional toResult(KeyEntity key) { if (key == null) { return Optional.empty(); @@ -325,9 +402,31 @@ private static KeyIndexResult toIndexResult(KeyEntity key) { key.getCreationTime(), key.getExpirationTime(), key.isRevoked(), + key.isDisabled(), uids); } + /// Applies read-only query hints for the three major JPA providers. + /// + /// Index queries map results directly to immutable {@link KeyIndexResult} DTOs — + /// the loaded entities are never modified. Signalling read-only intent allows each + /// provider to skip dirty tracking and lock acquisition: + ///
    + ///
  • Hibernate ({@code org.hibernate.readOnly = true}): skips snapshot creation and + /// dirty checking at flush time.
    • + ///
  • EclipseLink ({@code eclipselink.read-only = true}): detaches objects immediately, + /// preventing them from entering the identity map.
    • + ///
  • Apache OpenJPA ({@code openjpa.FetchPlan.ReadLockMode = "NONE"}): suppresses + /// implicit read-lock acquisition on each loaded row.
    • + ///
+ /// Unknown hints are silently ignored by all compliant JPA providers, so this method + /// is safe to call regardless of which provider is active at runtime. + private static TypedQuery applyReadOnlyHints(TypedQuery query) { + return query.setHint(HINT_HIBERNATE_READ_ONLY, Boolean.TRUE) + .setHint(HINT_ECLIPSELINK_READ_ONLY, Boolean.TRUE) + .setHint(HINT_OPENJPA_READ_LOCK_MODE, "NONE"); + } + private static boolean isHexString(String s) { if (s.isEmpty()) { return false; diff --git a/repository/src/main/java/io/github/bmarwell/keyserver/repository/entity/KeyEntity.java b/repository/src/main/java/io/github/bmarwell/keyserver/repository/entity/KeyEntity.java index a289e6f..551a9e5 100644 --- a/repository/src/main/java/io/github/bmarwell/keyserver/repository/entity/KeyEntity.java +++ b/repository/src/main/java/io/github/bmarwell/keyserver/repository/entity/KeyEntity.java @@ -59,6 +59,12 @@ public class KeyEntity { @Column(name = "revoked", nullable = false) private boolean revoked; + /// Keyserver-administrative flag: true if a keyserver operator has explicitly + /// disabled this key. Unlike {@code revoked}, this flag is not set by the key + /// owner and is not reflected in the OpenPGP key material. + @Column(name = "disabled", nullable = false) + private boolean disabled; + /// Full ASCII-armored public key block returned verbatim on `op=get`. @Column(name = "armored_key", nullable = false) private String armoredKey; @@ -143,6 +149,15 @@ public void setRevoked(boolean revoked) { this.mtime = OffsetDateTime.now(); } + public boolean isDisabled() { + return disabled; + } + + public void setDisabled(boolean disabled) { + this.disabled = disabled; + this.mtime = OffsetDateTime.now(); + } + public String getArmoredKey() { return armoredKey; } diff --git a/repository/src/main/resources/io/github/bmarwell/keyserver/repository/migrations/V010__add-disabled-flag-to-keys.sql b/repository/src/main/resources/io/github/bmarwell/keyserver/repository/migrations/V010__add-disabled-flag-to-keys.sql new file mode 100644 index 0000000..1a67e9b --- /dev/null +++ b/repository/src/main/resources/io/github/bmarwell/keyserver/repository/migrations/V010__add-disabled-flag-to-keys.sql @@ -0,0 +1,5 @@ +-- Add keyserver-level 'disabled' flag to the keys table. +-- A disabled key is hidden from op=index and op=get results but is not revoked +-- in the OpenPGP sense; the flag is set by keyserver administrators only. +-- Defaults to FALSE so all existing keys remain visible after the migration. +ALTER TABLE keys ADD COLUMN disabled BOOLEAN NOT NULL DEFAULT FALSE; diff --git a/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java b/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java index 8690ef4..e39e682 100644 --- a/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java +++ b/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java @@ -36,8 +36,11 @@ public class HkpIndexRenderer { /// uid:<pct-encoded-uid>:<ctime>:<exptime>:<flags> /// /// - /// Flags: `r` if revoked, `e` if expired (expiration is in the past), or empty. + /// Flags (in spec order): `r` if revoked, `d` if disabled on this keyserver, + /// `e` if expired (expiration is in the past), or empty. /// Key length is empty for ECC keys (bit strength is null), numeric for RSA/DSA. + /// The `d` flag is a keyserver-administrative concept; it does not appear on + /// `uid:` lines since UIDs are not disabled independently. public String renderMachineReadable(List results) { Instant now = Instant.now(); StringBuilder sb = new StringBuilder(); @@ -51,7 +54,7 @@ public String renderMachineReadable(List results) { sb.append(toEpochSeconds(key.creationTime())).append(':'); sb.append(key.expirationTime().map(HkpIndexRenderer::toEpochSeconds).orElse("")) .append(':'); - sb.append(computeFlags(key.revoked(), key.expirationTime(), now)); + sb.append(computeFlags(key.revoked(), key.disabled(), key.expirationTime(), now)); sb.append('\n'); for (UidIndexEntry uid : key.verifiedUids()) { sb.append("uid:"); @@ -70,7 +73,8 @@ public String renderMachineReadable(List results) { .map(HkpIndexRenderer::toEpochSeconds) .orElse("")) .append(':'); - sb.append(computeFlags(uid.revoked(), uid.expirationTime(), now)); + // UIDs are not disabled independently; pass false for the 'd' flag. + sb.append(computeFlags(uid.revoked(), false, uid.expirationTime(), now)); sb.append('\n'); } } @@ -96,7 +100,7 @@ public String renderHtml(List results) { .append(key.expirationTime().map(Object::toString).orElse("")) .append(""); sb.append("") - .append(computeFlags(key.revoked(), key.expirationTime(), now)) + .append(computeFlags(key.revoked(), key.disabled(), key.expirationTime(), now)) .append(""); sb.append(""); for (UidIndexEntry uid : key.verifiedUids()) { @@ -113,11 +117,15 @@ private static String toEpochSeconds(OffsetDateTime dt) { return String.valueOf(dt.toEpochSecond()); } - private static String computeFlags(boolean revoked, Optional expiration, Instant now) { + private static String computeFlags( + boolean revoked, boolean disabled, Optional expiration, Instant now) { StringBuilder flags = new StringBuilder(); if (revoked) { flags.append('r'); } + if (disabled) { + flags.append('d'); + } if (expiration.isPresent() && expiration.get().toInstant().isBefore(now)) { flags.append('e'); } diff --git a/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java b/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java index 5139854..cfc29e0 100644 --- a/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java +++ b/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java @@ -53,8 +53,8 @@ void returnsNotFoundWhenNoKeyMatchesIndexSearch() { void rendersMachineReadableInfoLineWithKeyCount() { // given — one key with one verified UID in the repository UidIndexEntry uid = new UidIndexEntry(UID_RAW, Optional.of(CREATION), Optional.empty(), false); - KeyIndexResult key = - new KeyIndexResult(FINGERPRINT, 22, Optional.empty(), CREATION, Optional.empty(), false, List.of(uid)); + KeyIndexResult key = new KeyIndexResult( + FINGERPRINT, 22, Optional.empty(), CREATION, Optional.empty(), false, false, List.of(uid)); this.fakeService.indexResults = List.of(key); // when — the client sends op=index&options=mr (machine-readable) @@ -72,8 +72,8 @@ void rendersMachineReadableInfoLineWithKeyCount() { void rendersMachineReadablePubLine() { // given — one RSA-2048 key (algorithm 1, bitStrength 2048) with a known creation time UidIndexEntry uid = new UidIndexEntry(UID_RAW, Optional.of(CREATION), Optional.empty(), false); - KeyIndexResult key = - new KeyIndexResult(FINGERPRINT, 1, Optional.of(2048), CREATION, Optional.empty(), false, List.of(uid)); + KeyIndexResult key = new KeyIndexResult( + FINGERPRINT, 1, Optional.of(2048), CREATION, Optional.empty(), false, false, List.of(uid)); this.fakeService.indexResults = List.of(key); // when — client requests machine-readable index @@ -91,8 +91,8 @@ void rendersMachineReadablePubLine() { void rendersMachineReadableUidLinePercentEncoded() { // given — a UID string containing characters that need percent-encoding ('<', '>', ' ') UidIndexEntry uid = new UidIndexEntry(UID_RAW, Optional.of(CREATION), Optional.empty(), false); - KeyIndexResult key = - new KeyIndexResult(FINGERPRINT, 22, Optional.empty(), CREATION, Optional.empty(), false, List.of(uid)); + KeyIndexResult key = new KeyIndexResult( + FINGERPRINT, 22, Optional.empty(), CREATION, Optional.empty(), false, false, List.of(uid)); this.fakeService.indexResults = List.of(key); // when — client requests machine-readable index @@ -111,8 +111,8 @@ void rendersMachineReadableUidLinePercentEncoded() { void setsFlagRForRevokedKey() { // given — a revoked key without expiration UidIndexEntry uid = new UidIndexEntry(UID_RAW, Optional.of(CREATION), Optional.empty(), false); - KeyIndexResult key = - new KeyIndexResult(FINGERPRINT, 22, Optional.empty(), CREATION, Optional.empty(), true, List.of(uid)); + KeyIndexResult key = new KeyIndexResult( + FINGERPRINT, 22, Optional.empty(), CREATION, Optional.empty(), true, false, List.of(uid)); this.fakeService.indexResults = List.of(key); // when — client requests machine-readable index @@ -132,7 +132,7 @@ void setsFlagEForExpiredKey() { OffsetDateTime pastExpiry = OffsetDateTime.of(2000, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC); UidIndexEntry uid = new UidIndexEntry(UID_RAW, Optional.of(CREATION), Optional.of(pastExpiry), false); KeyIndexResult key = new KeyIndexResult( - FINGERPRINT, 22, Optional.empty(), CREATION, Optional.of(pastExpiry), false, List.of(uid)); + FINGERPRINT, 22, Optional.empty(), CREATION, Optional.of(pastExpiry), false, false, List.of(uid)); this.fakeService.indexResults = List.of(key); // when — client requests machine-readable index @@ -153,8 +153,8 @@ void setsFlagEForExpiredKey() { void rendersHtmlWhenOptionsMrAbsent() { // given — one key in the repository, no options=mr in the request UidIndexEntry uid = new UidIndexEntry(UID_RAW, Optional.of(CREATION), Optional.empty(), false); - KeyIndexResult key = - new KeyIndexResult(FINGERPRINT, 22, Optional.empty(), CREATION, Optional.empty(), false, List.of(uid)); + KeyIndexResult key = new KeyIndexResult( + FINGERPRINT, 22, Optional.empty(), CREATION, Optional.empty(), false, false, List.of(uid)); this.fakeService.indexResults = List.of(key); // when — client sends op=index without options=mr (browser request) @@ -171,6 +171,25 @@ void rendersHtmlWhenOptionsMrAbsent() { .contains(FINGERPRINT); } + @Test + void setsFlagDForDisabledKey() { + // given — a key that has been disabled on this keyserver (not OpenPGP-revoked) + UidIndexEntry uid = new UidIndexEntry(UID_RAW, Optional.of(CREATION), Optional.empty(), false); + KeyIndexResult key = new KeyIndexResult( + FINGERPRINT, 22, Optional.empty(), CREATION, Optional.empty(), false, true, List.of(uid)); + this.fakeService.indexResults = List.of(key); + + // when — client requests machine-readable index + Response response = this.endpoint.doLookup("index", "alice@example.com", null, "mr"); + String body = (String) response.getEntity(); + + // then — the pub: flags field must contain 'd' (not 'r') since the key is disabled, not revoked + // format: pub:::::: + assertThat(body) + .as("keyserver-disabled key must have 'd' in pub: flags; 'r' is reserved for OpenPGP revocation") + .contains("pub:" + FINGERPRINT + ":22::" + CREATION.toEpochSecond() + "::d\n"); + } + // --------------------------------------------------------------------------- // Test double — avoids CDI/Mockito overhead; keeps the test focused on the // endpoint's routing and response-building logic only From dcb9343790592c9fe8b21d6124278d16688f49cd Mon Sep 17 00:00:00 2001 From: Benjamin Marwell Date: Wed, 27 May 2026 20:45:24 +0200 Subject: [PATCH 4/5] fix(index): align review comments - migration note, unverified key filter, HTML label - Fix V010 migration comment: disabled keys are shown with 'd' flag in op=index, not hidden from results (HKP spec behaviour) - Filter keys with no verified UIDs from all findManyBy* result paths: toIndexResults() now drops KeyIndexResult entries with empty verifiedUids, making fingerprint/keyid lookups consistent with email/UID-substring paths which already gate matching on u.verified = true - HTML renderer: rename 'Algorithm' column to 'Algorithm (OpenPGP code)' so users understand the integer value (e.g. 22 = EdDSA, 1 = RSA) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../bmarwell/keyserver/repository/JpaKeyRepository.java | 5 ++++- .../migrations/V010__add-disabled-flag-to-keys.sql | 2 +- .../github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java b/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java index 3046b8b..ba9bf5a 100644 --- a/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java +++ b/repository/src/main/java/io/github/bmarwell/keyserver/repository/JpaKeyRepository.java @@ -386,7 +386,10 @@ private static Optional toResult(KeyEntity key) { } private static List toIndexResults(List keys) { - return keys.stream().map(JpaKeyRepository::toIndexResult).toList(); + return keys.stream() + .map(JpaKeyRepository::toIndexResult) + .filter(r -> !r.verifiedUids().isEmpty()) + .toList(); } private static KeyIndexResult toIndexResult(KeyEntity key) { diff --git a/repository/src/main/resources/io/github/bmarwell/keyserver/repository/migrations/V010__add-disabled-flag-to-keys.sql b/repository/src/main/resources/io/github/bmarwell/keyserver/repository/migrations/V010__add-disabled-flag-to-keys.sql index 1a67e9b..4246c04 100644 --- a/repository/src/main/resources/io/github/bmarwell/keyserver/repository/migrations/V010__add-disabled-flag-to-keys.sql +++ b/repository/src/main/resources/io/github/bmarwell/keyserver/repository/migrations/V010__add-disabled-flag-to-keys.sql @@ -1,5 +1,5 @@ -- Add keyserver-level 'disabled' flag to the keys table. --- A disabled key is hidden from op=index and op=get results but is not revoked +-- A disabled key is shown in op=index results with the 'd' flag but is not revoked -- in the OpenPGP sense; the flag is set by keyserver administrators only. -- Defaults to FALSE so all existing keys remain visible after the migration. ALTER TABLE keys ADD COLUMN disabled BOOLEAN NOT NULL DEFAULT FALSE; diff --git a/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java b/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java index e39e682..cb2ff13 100644 --- a/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java +++ b/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java @@ -89,7 +89,7 @@ public String renderHtml(List results) { Instant now = Instant.now(); StringBuilder sb = new StringBuilder(); sb.append("

Search results: ").append(results.size()).append(" key(s)

\n"); - sb.append("" + sb.append("
FingerprintAlgorithm
" + "\n"); for (KeyIndexResult key : results) { sb.append(""); From 85716c37da24e2b9a22517bfac4a4d4e87a0cff4 Mon Sep 17 00:00:00 2001 From: Benjamin Marwell Date: Wed, 27 May 2026 21:08:51 +0200 Subject: [PATCH 5/5] fix(index): add Optional-in-record exception, fix options parsing, add HTML DOCTYPE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - AGENTS.adoc: document exception to 'no Optional in fields' rule — immutable transient DTO records (never persisted, never serialized) may use Optional components when optionality is intrinsic to the domain; KeyIndexResult and UidIndexEntry are the canonical examples - LookupEndpoint: fix options=mr detection — split on ',' and compare tokens case-insensitively via Arrays.stream().anyMatch() instead of String.contains(), preventing false matches on tokens like 'nomr' or 'mrtg' - HkpIndexRenderer.renderHtml: add DOCTYPE and so browsers render in standards mode; fixes potential mis-rendering of UTF-8 UIDs via 'save as' workflows and legacy intermediaries - LookupEndpointIndexTest: add assertions for DOCTYPE/charset in HTML test and new recognisesMrTokenAmongCommaDelimitedOptions test (nm,mr -> machine-readable) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/AGENTS.adoc | 11 +++++++++ .../keyserver/web/pks/HkpIndexRenderer.java | 4 +++- .../keyserver/web/pks/LookupEndpoint.java | 4 +++- .../web/pks/LookupEndpointIndexTest.java | 23 +++++++++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/.github/AGENTS.adoc b/.github/AGENTS.adoc index 9afdea9..1f5bad8 100644 --- a/.github/AGENTS.adoc +++ b/.github/AGENTS.adoc @@ -160,6 +160,17 @@ Every mutating command is wrapped in a BTX lifecycle: * Use `@Nullable` sparingly and only at boundaries that genuinely allow absence. * Prefer `Optional` instead of `@Nullable` for return types. * Never use `Optional` in fields, and avoid `Optional` parameters unless there is no cleaner API shape. ++ +[NOTE] +==== +*Exception — immutable DTO record components:* `Optional` is acceptable as a record _component_ when all of the +following hold: (a) the record is an immutable value-type DTO that is never persisted or serialized to an external +format, (b) the optionality is an intrinsic part of the domain concept (e.g. `expirationTime` that may genuinely be +absent), and (c) there is no natural sentinel or null-object alternative. +`KeyIndexResult` and `UidIndexEntry` satisfy these criteria — they are transient application-layer DTOs consumed and +discarded within a single request, never mapped by JPA or written to JSON/XML. +JPA entity fields and any class with `@XmlRootElement`, `@JsonSerialize`, or similar must still not use `Optional`. +==== * Avoid mixing docs, refactors, and behavior changes unless the task needs it. * After each Java or `pom.xml` change, run `./mvnw spotless:apply`. diff --git a/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java b/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java index cb2ff13..f0b8879 100644 --- a/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java +++ b/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/HkpIndexRenderer.java @@ -88,7 +88,9 @@ public String renderMachineReadable(List results) { public String renderHtml(List results) { Instant now = Instant.now(); StringBuilder sb = new StringBuilder(); - sb.append("

Search results: ").append(results.size()).append(" key(s)

\n"); + sb.append("\n"); + sb.append("Key search results"); + sb.append("

Search results: ").append(results.size()).append(" key(s)

\n"); sb.append("
FingerprintAlgorithm (OpenPGP code)CreatedExpiresFlagsUIDs
" + "\n"); for (KeyIndexResult key : results) { diff --git a/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/LookupEndpoint.java b/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/LookupEndpoint.java index 61e9c54..976dbc3 100644 --- a/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/LookupEndpoint.java +++ b/web/openpgp-keyserver-protocol/src/main/java/io/github/bmarwell/keyserver/web/pks/LookupEndpoint.java @@ -12,6 +12,7 @@ import jakarta.ws.rs.Path; import jakarta.ws.rs.QueryParam; import jakarta.ws.rs.core.Response; +import java.util.Arrays; import java.util.List; import java.util.Optional; import org.eclipse.microprofile.openapi.annotations.tags.Tag; @@ -59,7 +60,8 @@ public Response doLookup( } if ("index".equalsIgnoreCase(op)) { - boolean machineReadable = options != null && options.contains("mr"); + boolean machineReadable = options != null + && Arrays.stream(options.split(",")).map(String::strip).anyMatch("mr"::equalsIgnoreCase); return handleIndex(search, exactMatch, machineReadable); } diff --git a/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java b/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java index cfc29e0..3b846f8 100644 --- a/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java +++ b/web/openpgp-keyserver-protocol/src/test/java/io/github/bmarwell/keyserver/web/pks/LookupEndpointIndexTest.java @@ -169,6 +169,29 @@ void rendersHtmlWhenOptionsMrAbsent() { assertThat(body) .as("HTML body must contain the fingerprint so users can identify the key") .contains(FINGERPRINT); + assertThat(body) + .as("HTML must include DOCTYPE and charset meta so browsers render it in standards mode with UTF-8") + .startsWith(""); + assertThat(body).contains(""); + } + + @Test + void recognisesMrTokenAmongCommaDelimitedOptions() { + // given — options contains 'mr' alongside another token + UidIndexEntry uid = new UidIndexEntry(UID_RAW, Optional.of(CREATION), Optional.empty(), false); + KeyIndexResult key = new KeyIndexResult( + FINGERPRINT, 22, Optional.empty(), CREATION, Optional.empty(), false, false, List.of(uid)); + this.fakeService.indexResults = List.of(key); + + // when — client sends options=nm,mr (comma-separated list) + Response response = this.endpoint.doLookup("index", "alice@example.com", null, "nm,mr"); + String body = (String) response.getEntity(); + + // then — machine-readable format must be returned because 'mr' is in the options list + assertThat(response.getStatus()).isEqualTo(200); + assertThat(body) + .as("'mr' token in a comma-separated options list must trigger machine-readable output") + .startsWith("info:1:"); } @Test
FingerprintAlgorithm (OpenPGP code)CreatedExpiresFlagsUIDs