feat(core): return result objects instead of by-reference output parameters - #708
Open
Spomky wants to merge 1 commit into
Open
feat(core): return result objects instead of by-reference output parameters#708Spomky wants to merge 1 commit into
Spomky wants to merge 1 commit into
Conversation
…meters The verifiers, the decrypters, the loaders and the serializer managers used to write their secondary results - the key that verified a signature, the index of the recipient that could be decrypted, the name of the serializer - into variables of the caller. Those methods cannot be called with named arguments, cannot be composed and are hard to type for static analysers. Readonly result objects carry those values instead, and the methods that populate the references are implemented on top of the new ones: - JWSVerifier::verify() returns a VerificationResult - JWEDecrypter::decrypt() returns a DecryptionResult, and leaves the JWE it is given untouched - JWSLoader::loadAndVerify(), JWELoader::loadAndDecrypt() and NestedTokenLoader::loadAndVerify() return a LoadingResult - JWSSerializerManager::unserializeToken() and JWESerializerManager::unserializeToken() return an UnserializationResult The new methods accept a JWK as well as a JWKSet, and the callable that observes the keys discarded along the way is now a declared argument. They are declared on the service interfaces, so that a decorator can be plugged into them the same way as into the methods they replace. The old methods keep their exact behaviour and trigger a deprecation. The two methods whose output parameter is optional - the serializer managers' unserialize() and NestedTokenLoader::load() - only deprecate that argument. The algorithm interfaces cannot be changed without breaking every third-party implementation, so 4.3.0 only documents the signature they will have in 5.0.0 and ships the objects they will return: EncryptedContent and WrappedKey.
Spomky
force-pushed
the
feature/result-objects
branch
from
August 29, 2026 16:59
eab5cae to
341c3be
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #685. Rebased on
4.3.xafter #709 (service interfaces) and #707 (exception hierarchy).What changes
Readonly result objects replace the by-reference output parameters listed in the issue. The new methods sit next to the old ones, and the old ones are implemented on top of them, so nothing breaks.
JWSVerifier::verifyWithKeySet($jws, $jwkset, 0, null, $jwk)verify($jws, $keys, 0)→VerificationResultJWEDecrypter::decryptUsingKey/KeySet($jwe, …)decrypt($jwe, $keys, 0)→DecryptionResultJWSLoader::loadAndVerifyWithKey/KeySet($token, …, $signature)loadAndVerify($token, $keys)→LoadingResultJWELoader::loadAndDecryptWithKey/KeySet($token, …, $recipient)loadAndDecrypt($token, $keys)→LoadingResultJWSSerializerManager::unserialize($input, $name)unserializeToken($input)→UnserializationResultJWESerializerManager::unserialize($input, $name)unserializeToken($input)→UnserializationResultNestedTokenLoader::load($token, …, $signature)loadAndVerify($token, …)→LoadingResultTwo other things the new methods gain:
JWKas well as aJWKSet, so the…WithKey()/…WithKeySet()pairs collapse into one method;func_get_arg()because a signature cannot change in a minor — is a declared argument ofverify()anddecrypt().The
$signature/$signatureIndexdocblock mismatch ofverifyWithKeySet()is fixed on the docblock side: renaming the parameter would break callers already using named arguments.No return type changes
No existing method changed its signature.
verifyWithKeySet()still returnsbool,loadAndVerifyWithKeySet()still returnsJWS,unserialize()still returnsJWS/JWE,load()still returnsJWS. The result objects are only returned by the new methods.The new methods are declared on the interfaces of #709
JWSVerifierInterface,JWEDecrypterInterface,JWSLoaderInterface,JWELoaderInterfaceandNestedTokenLoaderInterfacegain the new method, and the deprecated ones are tagged@deprecatedthere too. Adding a method to a published interface is a hard BC break, so this is only free because those interfaces have not shipped yet — worth a conscious ack before merging.It is also what makes the design hold together:
JWSLoaderreceives aJWSVerifierInterface, so without the declaration it could not callverify()and would have to keep calling the deprecated method internally. It also keeps decoration — the extension mechanism #709 established — working for the new API.Deprecations
verifyWithKeySet(),decryptUsingKey(),decryptUsingKeySet(),loadAndVerifyWithKey(),loadAndVerifyWithKeySet(),loadAndDecryptWithKey()andloadAndDecryptWithKeySet()trigger a deprecation and will be removed in 5.0.0. Their output parameter is required, so the methods cannot survive without it.JWSSerializerManager::unserialize(),JWESerializerManager::unserialize()andNestedTokenLoader::load()keep working as they are: their output parameter is optional, so only passing it is deprecated, and only the argument goes away in 5.0.0. Callingunserialize($input)orload($token, $encryptionKeySet, $signatureKeySet)triggers nothing.Algorithm interfaces
As in #654, changing
encryptContent(),encryptKey(),wrapKey()andgetAgreementKey()cannot be done additively without breaking every third-party implementation. 4.3.0 only documents the signature they will have in 5.0.0 and ships the objects they will return:EncryptedContent(ciphertext + tag) andWrappedKey(key + additional header parameters).Bundle
Both families of services implement the new methods:
EventDispatching*decorators dispatch from the new method and implement the deprecated ones on top of it, so an event is dispatched exactly once whichever API the application uses;Two decisions worth a look
decrypt()does not throw on failure. The issue's example has it throwing, butdecryptUsingKeySet()already throws for an empty key set or a JWE without recipients, and those must keep propagating. A dedicated exception would be needed to tell "no key worked" apart from them;isDecrypted()keeps the old semantics, mirrorsisVerified()and makes the deprecated method a two-line wrapper.NestedTokenLoaderis included although it is not in the issue's table — itsload()has the same?int &$signatureidiom.Checks
phpunit: 1033 tests green, including 23 new ones covering the result objects, the deprecations and the unchanged behaviour of the deprecated methods.ecs,rector --dry-run: clean on the touched files.phpstan: no new error, and the set of unmatched baseline entries is identical to the one on a pristine4.3.x. Nineteen baseline entries are dropped — the by-ref,int|nullandarguments.counttypes they described are gone,JWEDecrypternow iteratesJWKSet::all()instead of the@internaliterator, and the deprecated loader methods declare@param-out int.