Fetch public keys asynchronously and remove the waiting forms - #8
Fetch public keys asynchronously and remove the waiting forms#8jwrosewell wants to merge 9 commits into
Conversation
PublicKeyFetch.publicKeyPem and PublicKeyFetch.verify returned only once the creator had answered, holding whichever thread asked, which on a request thread or an event loop is a stall of up to fifteen seconds for each key not yet held. Both now return a CompletableFuture at once and the waiting forms are gone, with no deprecated twin left behind. The request is made by a new PublicKeyTransport interface, and the new HttpUrlConnectionTransport is the one used where a caller names none. It runs the JDK's blocking HttpURLConnection on an Executor, either one the caller gives or a shared pool of daemon threads bounded at twice the processors available, because Java 8, which the library still targets, has no non-blocking HTTP client of its own. On Java 11 and later a transport over java.net.http.HttpClient.sendAsync can be supplied instead. The redirect refusal and the date parameter are kept exactly as they were. The cache now holds the future of each fetch rather than the key, so two requests for the same key made while the first is in flight share one request, and a fetch that fails is dropped so the next request asks again. The tests join the futures, and five are added, covering the shared in-flight request, a failure not being held, the request running on the executor given, an executor that refuses, and a missing transport. The README example and the class list are updated to the new shape.
|
This branch breaks the 51Degrees Java consumer, and the cause is one line.
Checked by compiling, not by readingRunning the pipeline module's exact compiler settings on JDK 21.0.11 over this branch's The same command over The fix that worksReplacing that one line with the URI form and adding URLConnection opened = URI.create(url).toURL().openConnection();
One behaviour difference to decide rather than absorb silently. Adding the file to the consumer's exclude list also works, and I checked that too, but it would leave the transport uncompiled in the consumer, which is worse than not having the deprecation. What is not affectedThe rest of this change lands cleanly on the consumers, which I checked rather than assumed.
NotesWritten with AI assistance from a session working on the 51Did packages, and needs human review. Nothing here has been changed. |
The URL(String) constructor is deprecated from Java 20, and pipeline-java compiles this source into its 51Did module with -Xlint:all and -Werror, excluding only Endpoints and PublicKeyFetch, so the new transport would have failed that build on the Java 21 leg of its matrix. URI then toURL replaces it. URI.toURL has been present since 1.0, so the Java 8 floor is unaffected. A url that will not parse now raises URISyntaxException rather than MalformedURLException, so the catch takes both and a bad url is still reported as the key being unavailable with the same status as before.
|
Fixed in 010b133. The new transport used the Reproduced before fixing, compiling this branch's The same command passes after the change. On the behaviour point, I used
|
|
Verification run book for this change and the four alongside it: SWAN-community/owid-dotnet#17 It gives the commands and expected results for every port, the traps that waste time (Rust threads its tests where the other six do not, a restored file can leave a build stale and passing against an old binary, Python needs the owid submodule on PYTHONPATH with a Windows separator), and two deliberate breaks that must make specific tests fail, so the tests are shown to be worth something rather than assumed to be. |
The cache was keyed by the whole key url, and the url carries the date of the identifier being verified in minutes. A creator's key changes on the order of a week, so two identifiers signed a minute apart never shared an entry and a hundred identifiers over a hundred minutes made a hundred requests for one key. The cache only ever served a repeat verification of one identifier. Keys are now held by end point, which is the key url without its date, and each key carries the span of minutes the creator has confirmed it for. A key is in force from the start of its period until the next key starts, so a key the creator answers with at two minutes was in force at every minute between them. An identifier dated inside a confirmed span is verified without a request. One dated outside every span is asked about, and the answer widens the span when the same key comes back or adds a key when it does not. The span is never widened across a minute the creator has answered with another key for. A date later than now is held against now, because that is how a creator reads it. Held against the future minute, the key would still be served for that minute after the creator had rotated. A request without a date is read as now for the same reason. The bound of 1024 keys and the empty and refill are unchanged, as is the sharing of one request between callers arriving together and the refusal to hold a failure. clearCache now also forgets the requests under way, as the Python and Rust ports do. The same change is made to every port that caches, so the ports behave the same way. Tests cover a minute between two confirmed minutes being served without a request, a hundred identifiers inside one confirmed period making none, a key never being served outside its span across a rotation, a future date being read as now, and the bound against a creator that answers every minute with a different key. The bound test and the shared request test were checked by breaking the code and watching them fail. The main sources compile clean with -Xlint:all,-try,-options -Werror.
…g them The span cache read a date later than now as now and held the answer against now. A creator whose clock runs ahead of this one's signs identifiers dated in this process's future, and just after a rotation the cache would have served such an identifier the old key from a span confirmed up to now. It would have read as not matching until this clock caught up, for as long as the two clocks differ. The cache keyed by url did not have this fault, because it asked the creator for the exact minute. A minute within fifteen minutes of now, or later, is now neither served from the cache nor held in it. The creator may have read such a minute as its present rather than as the minute named, so its answer says nothing certain about the minute. A request with no date is treated the same way. Live identifiers therefore cost one request per minute per creator, which is what they always cost, and every identifier older than the allowance is served from the spans. The same allowance is applied on every port that caches. The test that read a future date as now is replaced by one that shows a recent minute asked about twice, a future minute and an undated request asked about, and a minute beyond the allowance held after its first request.
The public key end point now answers with a JSON object carrying the key as publicKeySPKI together with validFrom and validTo, the UTC moments the key came into force and the next key starts. validFrom is null for a creator with one key and no schedule, and validTo is null for the last key in a schedule. The PEM alone as text is not a valid answer, and a client that receives it reports the key as one it cannot read. The answer is checked before it is sent, by the same code a client checks it with. The key must be a public key this library can read, a key valid to a moment must be valid from an earlier one, and the key must have been in force at the moment asked about. A creator whose store or schedule fails that check answers with a server error rather than a bad answer, so a fault on the server side shows up in the server's own tests and never reaches a client. The client holds the key for the whole span the creator stated, so an identifier dated anywhere in it is verified without a request whatever the clock drift, and live identifiers cost one request per key rather than one per minute. A creator that states no span has its key held against the minutes it confirms, with the fifteen minute clock drift allowance kept out of that cache as before. A signature that does not verify under the key selected for the identifier's minute, where that minute is within the drift allowance of an edge of the key's span, is checked against the neighbouring key before it is reported as not matching, because a creator's signing machines may not agree with its schedule to the minute. Where the key tried was never in force at the identifier's minute the neighbours are not tried. Callers verifying the same identifier at the same moment make one request between them, and a test with many threads proves it. The stand in creator in the tests answers with what this library's own server side code builds, so every client test runs against the response a creator built on this library sends, and the loop between the two halves is closed.
…d the stated span The neighbouring key is asked for by the minute just beyond the edge of the span the creator stated, rather than by a minute a fixed distance from the identifier, so a key in force for less than the drift allowance is still the one tried. The span a caller sees is the one the creator stated. A key stated with a start and no end has no later edge, whatever the cache holds it for, so a live identifier dated just after a rotation is checked against the key before it. A creator that stated no span has one key and no neighbour to try. Where the creator's own statement puts the identifier's date outside the span of the key it answered with and nothing verifies, the key is reported as unavailable rather than the signature as not matching, because a key that was not in force proves nothing about the identifier. The request asks for the JSON form by name, and the README and the end point javadoc describe the answer as the JSON object the specification requires rather than the PEM alone.
The specification no longer has a creator end point. It repeated the key the public-key end point serves and added fields no verifier read. The end point helpers serve the public-key end point alone, the JSON writing that only the creator answer used goes with it, and the tests that exercised both end points exercise the one that remains.
The public-key answer carries the key as publicKey and the encoding it is in as format. The one format served is spki, which a request without the parameter receives, and any other value is answered 400 rather than in an encoding the caller did not ask for. The answer is checked for its format before its key and its span, so a creator never sends an encoding it does not itself read. The client asks for spki by name, reads publicKey, and refuses an answer that states another format as a key it cannot read. The stand in creator in the tests honours the format the request asks for, so the client is exercised against a creator that refuses the way the specification requires.
A signature covers the OWID's own bytes without the signature field and nothing else. Signing and verifying over other OWIDs is gone from Creator, from every verification surface and from the tests, along with the chained interop fixtures, because nothing in production signs that way and an undocumented signing input is a liability for anyone implementing from the specification.
1 participant
What changed
The creator's public key is now fetched asynchronously and only asynchronously. Every network method on
PublicKeyFetchreturns aCompletableFutureand the waiting forms are removed outright. This is one of a set of changes making every OWID port and every 51Did client asynchronous where the network is involved, so the libraries are consistent across languages.The build stays at Java 8, which has no non-blocking HTTP client in the standard library, so the transport is an interface:
with
HttpUrlConnectionTransportas the default. That default runs the existingHttpURLConnectioncode, redirect refusal and timeouts unchanged, on anExecutorthe caller may supply, with a bounded pool of daemon threads by default. Its Javadoc says plainly that this is blocking I/O on a background thread and that on Java 11 and later a transport overjava.net.http.HttpClient.sendAsynccan be supplied instead. The redirect and exact URL rules are written into the interface's Javadoc so a replacement transport keeps them.Verification with a key already in hand is unchanged and stays synchronous, as do
publicKeyUrlandclearCache, which make no request.Removed
public static String publicKeyPem(Owid owid, String scheme) throws OwidExceptionpublic static OwidVerificationResult verify(Owid owid, String scheme, List<Owid> others)verifyAtUrlandpublicKeyPemAtUrlthat only tests usedNew
public static CompletableFuture<String> publicKeyPem(Owid owid, String scheme)and an overload taking aPublicKeyTransportpublic static CompletableFuture<OwidVerificationResult> verify(Owid owid, String scheme, List<Owid> others)and an overload taking aPublicKeyTransportPublicKeyTransportandHttpUrlConnectionTransportas aboveA
publicKeyPemfuture fails withPublicKeyFetchException(status, domain and code carried) orOwidException. Averifyfuture never fails, every route maps to a status as before.Kept exactly
The redirect refusal from #7 and the date parameter on the key URL. The cache now holds futures, so concurrent requests for one URL share a fetch and a failed fetch is evicted so the next request retries.
Verification
mvn teston JDK 21 with the compiler release at 8 for main and test sources:Tests run: 134, Failures: 0, Errors: 0, Skipped: 0,BUILD SUCCESS(129 before). New tests: two requests in flight for one key make one request, a failing fetch is not held, the request runs on the executor given, a request the executor refuses is key unavailable, a missing transport is refused. The Java 8, 11 and 17 runtimes are exercised by CI's matrix on this pull request; the repository has no coverage tool, so coverage was checked by inspection, every previous test is kept in async form.Cache keyed on the key's span, added 7 September 2026
The cache was keyed by the whole key url, and the url carries the identifier's date in minutes, so two identifiers signed a minute apart never shared an entry and a hundred identifiers over a hundred minutes made a hundred requests for one key. Keys are now held by creator end point, which is the key url without its date, each against the span of minutes the creator has confirmed it for. A key is in force from the start of its period until the next key starts, so a key the creator answers with at two minutes was in force at every minute between them, and an identifier dated inside a confirmed span is verified without a request. The span is never widened past a minute the creator has answered for, nor across a minute the creator answered with another key for. A date later than now is held against now, because the creator reads it that way, and a request with no date is read as now.
The bound of 1024 keys and the empty and refill are unchanged, as are the shared request between callers arriving together and the refusal to hold a failure.
clearCachenow also forgets the requests under way, as the Python and Rust ports do. The same change is on every port that caches. Addresses #9.mvn test139 of 139,BUILD SUCCESS. Five tests added. Every main source compiles clean on JDK 17 with-Xlint:all,-try,-options -Werror. Delete theheldKeys >= MAXIMUM_CACHED_KEYSblock inholdandtheCacheIsBoundedfails withheld 1025 of at most 1024. ReplaceIN_FLIGHT.get(url)withnullandtwoRequestsInFlightForOneKeyMakeOneRequestfails with expected 1 but was 2. Both checked.Clock drift, added later the same day
The first version of this section read a date later than now as now and held the answer against now. A creator whose clock runs ahead signs identifiers dated in this process's future, and just after a rotation such an identifier would have been served the old key from a span confirmed up to now, reading as not matching for as long as the two clocks differ. A minute within fifteen minutes of now, or later, is now neither served from the cache nor held in it, and a request with no date is treated the same way. Live identifiers therefore cost one request per minute per creator, which is what they always cost, and every identifier older than the allowance is served from the spans. The test that read a future date as now is replaced by one that shows a recent minute asked about twice, a future minute and an undated request asked about, and a minute beyond the allowance held after its first request.
mvn test139 of 139 and the lint compile clean after the change.The answer carries the span, added 7 September 2026
The public key end point now answers with a JSON object carrying the key as
publicKeySPKItogether withvalidFromandvalidTo, the UTC moments the key came into force and the next key starts.validFromis null for a creator with one key and no schedule, andvalidTois null for the last key in a schedule. The PEM alone as text is not a valid answer, and a client that receives it reports the key as one it cannot read. The answer is checked before it is sent, by the same code a client checks it with, so a store or schedule that would produce an answer a client refuses is a server error at the creator.Endpoints.publicKeyResponseAtandEndpoints.publicKeyAnswerbuild the answer from aPublicKeySchedule, andPublicKeyResponsereads, writes and checks the body with the JDK alone.The client holds the key for the whole span the creator stated, so an identifier dated anywhere in it is verified without a request whatever the clock drift, and live identifiers cost one request per key rather than one per minute. A creator that states no span has its key held against the minutes it confirms, with the fifteen minute clock drift allowance kept out of that cache. A signature that does not verify under the key selected, where the identifier is dated within the drift allowance of an edge of the key's span, is checked against the neighbouring key before it is reported as not matching, because a creator's signing machines may not agree with its schedule to the minute.
The stand in creator in the tests answers with what this port's own server side code builds, so every client test runs against the response a creator built on this port sends. A test with thirty two threads verifying one OWID at the same moment shows one request between them.
mvn test144 of 144 and every main source compiles clean on JDK 17 with-Xlint:all,-try,-options -Werror.Brought into line with the specification as merged, 7 September 2026
Four commits, each on its own. The neighbouring key is asked for by the minute just beyond the edge of the span the creator stated, the span the neighbour check works from is the one the creator stated so a key with a start and no end has no later edge, a creator that stated no span has no neighbour to try, and where the creator's own statement puts the identifier's date outside the key it answered with and nothing verifies the key is reported as unavailable rather than the signature as not matching. The creator end point is gone, because the specification no longer has one. The answer carries the key as
publicKeyand its encoding asformat, the one format isspki, a request without it receivesspki, and any other value is answered 400. A signature covers the OWID's own bytes and nothing else, so signing and verifying over other OWIDs is gone with the chained interop fixtures, because nothing in production signs that way.The key request also asks for JSON by name in its Accept header, which it did not before.
mvn test146 of 146, and every main source compiles clean on JDK 17 with-Xlint:all,-try,-options -Werror.Notes
Written with AI assistance and reviewed before merging. The 51Degrees fork and the
pipeline.didpackage follow this merge.