components-web-app / components-web-app/api-components-bundle

Config guards that are declared but never enforced: user.class_name, refresh_token.*, publishable.permission

Open
#222 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
PHP
Stars
32
Forks
8
PR merge metrics
No merged PRs in 30d

Description

Follow-up to #214, which fixed one instance of this pattern. The same shape exists elsewhere and was deliberately left alone in #221 because every candidate fix forces configuration onto applications that currently omit it.

The pattern

A config node is declared with a default (addDefaultsIfNotSet(), canBeDisabled(), or similar) and isRequired() children. Because the node has a default, ArrayNode::finalizeValue inserts it and continues without finalizing, so the isRequired() checks never run when the node is omitted. The extension then reads the missing keys directly, producing undefined-array-key warnings and services wired with null where a typed scalar is expected — a TypeError deferred to first use rather than a clear failure at compile time.

It reads as a guard. It never runs.

Known instances

  • user.class_name
  • refresh_token.*
  • publishable.permission
  • refresh_token.options.class — read unguarded under the doctrine-storage branch

None verified to the depth #214 was; they were spotted while fixing that one. Confirm each empirically (minimal config, count the warnings) before deciding what to do about it.

Why it wasn't fixed with #214

The fix for #214 gave every child a value that preserves today's effective behaviour, so no application changes behaviour or fails to boot. Whether the same approach suits each of these depends on whether a sensible inert default exists for that particular setting — user.class_name in particular may have no meaningful default, in which case the right answer is a clear compile-time failure rather than a default, and that is a breaking change for anyone currently relying on the silent-null path.

That is a per-setting judgement, which is why this is its own issue rather than more scope on #221.

Acceptance

For each setting: either it resolves to a value that preserves current effective behaviour, or it fails loudly at container compile time with a message naming the missing key. No setting should reach a service as null where the signature says otherwise.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with ArrayNode::finalizeValue and the configuration declarations for user.class_name, refresh_token.*, publishable.permission, and refresh_token.options.class. Confirm each case with minimal configuration and warning counts, then assess whether an inert default preserves behavior or whether compile-time failure is required. Done means every setting preserves effective behavior or reports the missing key before service wiring.

Written by the indexing model from the issue text.

Assessment

Tech stack
php, symfony
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.