Skip to content

[FrameworkBundle] Fail gracefully when forms use disabled CSRF #46960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 19, 2022

Conversation

HeahDude
Copy link
Contributor

Q A
Branch? 4.4
Bug fix? kind of
New feature? no
Deprecations? no
Tickets ~
License MIT
Doc PR ~

Relates to symfony/symfony-docs#16973.

Currently with the following config in Symfony demo:

# config/packages/framework.yaml
framework:
    csrf_protection: false
    form:
        csrf_protection: true

we get:

The service "form.type_extension.csrf" has a dependency on a non-existent service "security.csrf.token_manager".

We should consider this PR as a bug fix to make this exception actionable.

@carsonbot carsonbot added this to the 4.4 milestone Jul 17, 2022
@HeahDude HeahDude force-pushed the fix/form-csrf-missing-exception branch from 8dc9f33 to fab0d6d Compare July 17, 2022 14:10
@HeahDude HeahDude changed the title [FrameworkBundle] Fail gracefully when forms uses disabled CSRF [FrameworkBundle] Fail gracefully when forms use disabled CSRF Jul 17, 2022
@chalasr
Copy link
Member

chalasr commented Jul 17, 2022

Such DX improvements are usually merged as features AFAIK

@xabbuh
Copy link
Member

xabbuh commented Jul 19, 2022

IMO when core DI extension classes or compiler classes fail with a service not found exception fixing them is not a DX enhancement but a real bugfix.

@HeahDude HeahDude force-pushed the fix/form-csrf-missing-exception branch from fab0d6d to 5990182 Compare July 19, 2022 09:12
@fabpot
Copy link
Member

fabpot commented Jul 19, 2022

Thank you @HeahDude.

@fabpot fabpot merged commit 8e72b1f into symfony:4.4 Jul 19, 2022
@HeahDude HeahDude deleted the fix/form-csrf-missing-exception branch July 19, 2022 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants