Skip to content

Make the JWS/JWE builders truly immutable and deprecate create() #683

Description

@Spomky

Description

The JWSBuilder and JWEBuilder mix three responsibilities: they are shared services holding an
AlgorithmManager, they accumulate mutable state, and they advertise an immutable with*() API that
they do not fully honour. The visible symptom is create(), which is not a named constructor but a
reset:

create() only exists because the builders leak state (see the 4.2.x bug reports linked below), and
NestedTokenBuilder has to call it defensively before every build.

Three consequences, all of them user-visible:

  1. State leaksJWSBuilder::addSignature() mutates the receiver, so a shared service can be
    poisoned for the rest of the process.
  2. Order-sensitive APIaddRecipient() must be called after the shared protected header
    (otherwise Parameter "enc" is missing.), and calling a shared-header setter after a recipient
    raises a fatal error. Both come from validating during accumulation instead of at build().
  3. Untyped internal state$signatures and $recipients are arrays of arrays, which is how the
    dead sender_key entry survived unnoticed.

Example

Target design for 5.0.0 — the service is stateless, the state is an immutable value:

// before (4.x)
$jws = $jwsBuilder
    ->create()                       // reset, mandatory in some flows, pointless in others
    ->withPayload($payload)
    ->addSignature($key, $header)
    ->build();

// after (5.0.0)
$jws = JWSBuilder::create($algorithmManager)   // real named constructor
    ->withPayload($payload)
    ->addSignature($key, $header)              // pure clone, no cross-validation here
    ->build();                                 // every check happens here, order no longer matters

Plan for 4.3.0 (no BC break)

  • Make every with*() / add*() method purely functional (clone first, never touch $this).
  • Move all cross-field validation from the accumulation methods to build(), so that any call order
    works. Keep the current exception types and messages.
  • Replace the internal array{key: JWK, header: array, ...} entries by @internal readonly value
    objects (SignatureSpec, RecipientSpec).
  • Deprecate JWSBuilder::create() and JWEBuilder::create() with trigger_deprecation(): they become
    no-ops once the builders are immutable.
  • Add a static create(AlgorithmManager $algorithms): static named constructor as the replacement.

Plan for 5.0.0

  • Remove the create() reset; keep only the static named constructor.
  • Make the builders final (depends on the service-interfaces issue) and the state properties
    readonly.

Related

Metadata

Metadata

Assignees

Labels

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions