Skip to content
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

[Security] Rename firewalls’ pattern to path #31496

Closed
wants to merge 1 commit into from
Closed

[Security] Rename firewalls’ pattern to path #31496

wants to merge 1 commit into from

Conversation

MatTheCat
Copy link
Contributor

@MatTheCat MatTheCat commented May 14, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets N/A
License MIT
Doc PR maybe symfony/symfony-docs#11574

The same way pattern has been replaced by path in the routing component (cf. #5989) consistency would be better if the security component did the same.

@linaori
Copy link
Contributor

linaori commented May 14, 2019

The difference is that the path in a route is an actual path and the pattern in the firewall is a regular expression. I'm not sure this will improve DX, possibly even hurt DX because developers might not know that it allows more than a static path.

I'm 👎 on this change for that reason as pattern is accurate here.

@MatTheCat
Copy link
Contributor Author

developers might not know that it allows more than a static path

It is the same for host. Maybe path_pattern/host_pattern then?

@MatTheCat
Copy link
Contributor Author

MatTheCat commented May 14, 2019

That being said access controls also use path for a pattern. So the firewall is the only place where an option matching a path is not named path.

DX would come from consistency here.

@nicolas-grekas nicolas-grekas added this to the next milestone May 20, 2019
@MatTheCat MatTheCat requested a review from xabbuh as a code owner May 31, 2019 10:19
@MatTheCat MatTheCat changed the base branch from master to 4.4 May 31, 2019 10:19
@fabpot
Copy link
Member

fabpot commented Jul 8, 2019

Closing as explained by @linaori

@fabpot fabpot closed this Jul 8, 2019
@MatTheCat
Copy link
Contributor Author

Great

@MatTheCat MatTheCat deleted the deprecate_firewall_pattern branch July 8, 2019 08:25
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
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.

None yet

5 participants