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

[Validator] Wire NotCompromisedPassword in FrameworkBundle and handle non UTF-8 password #30898

Merged
merged 1 commit into from Apr 6, 2019

Conversation

@tgalopin
Copy link
Member

commented Apr 6, 2019

Q A
Branch? master
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #30870
License MIT
Doc PR -

Live from #eu-fossa

Fix #30870

@OskarStark
Copy link
Contributor

left a comment

A test case would be nice

@tgalopin tgalopin force-pushed the tgalopin:pwned-convert-encoding branch from a1be3d5 to 76fa504 Apr 6, 2019

@ostrolucky
Copy link
Contributor

left a comment

I don't think it makes too much sense to make this configurable. People who don't use UTF-8 should change internal_encoding php.ini setting

@derrabus
Copy link
Contributor

left a comment

Please add a test case.

@stof

This comment has been minimized.

Copy link
Member

commented Apr 6, 2019

FrameworkBundle should define a service for that constraint validator, passing %kernel.charset% for this argument. Otherwise, customizing this charset will be a pain in a fullstack project.

@tgalopin

This comment has been minimized.

Copy link
Member Author

commented Apr 6, 2019

@stof that's the aim, but it can be the subject of another PR IMO.

@tgalopin tgalopin force-pushed the tgalopin:pwned-convert-encoding branch from 76fa504 to 239810d Apr 6, 2019

@tgalopin

This comment has been minimized.

Copy link
Member Author

commented Apr 6, 2019

Updated

@OskarStark
Copy link
Contributor

left a comment

Nice one 🎉

@fabpot

fabpot approved these changes Apr 6, 2019

@tgalopin tgalopin changed the title [Validator] Convert encoding to UTF-8 when needed in NotPwnedValidator [Validator] Wire NotCompromisedPassword in FrameworkBundle and handle non UTF-8 password Apr 6, 2019

@tgalopin tgalopin changed the title [Validator] Wire NotCompromisedPassword in FrameworkBundle and handle non UTF-8 password [WIP][Validator] Wire NotCompromisedPassword in FrameworkBundle and handle non UTF-8 password Apr 6, 2019

@tgalopin tgalopin force-pushed the tgalopin:pwned-convert-encoding branch from 239810d to bc0ba26 Apr 6, 2019

@tgalopin tgalopin requested a review from dunglas as a code owner Apr 6, 2019

@tgalopin

This comment has been minimized.

Copy link
Member Author

commented Apr 6, 2019

Based on #30889, otherwise ready to review.

@fabpot

This comment has been minimized.

Copy link
Member

commented Apr 6, 2019

@tgalopin Can you rebase?

@tgalopin tgalopin force-pushed the tgalopin:pwned-convert-encoding branch from bc0ba26 to 4f74a33 Apr 6, 2019

@tgalopin tgalopin changed the title [WIP][Validator] Wire NotCompromisedPassword in FrameworkBundle and handle non UTF-8 password [Validator] Wire NotCompromisedPassword in FrameworkBundle and handle non UTF-8 password Apr 6, 2019

@tgalopin

This comment has been minimized.

Copy link
Member Author

commented Apr 6, 2019

Updated

@tgalopin tgalopin force-pushed the tgalopin:pwned-convert-encoding branch from 4f74a33 to c5cd75d Apr 6, 2019

@tgalopin

This comment has been minimized.

Copy link
Member Author

commented Apr 6, 2019

Updated

@nicolas-grekas nicolas-grekas added this to the next milestone Apr 6, 2019

@fabpot fabpot force-pushed the tgalopin:pwned-convert-encoding branch from c5cd75d to 8ac712b Apr 6, 2019

@fabpot

This comment has been minimized.

Copy link
Member

commented Apr 6, 2019

Thank you @tgalopin.

@fabpot fabpot merged commit 8ac712b into symfony:master Apr 6, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Apr 6, 2019

feature #30898 [Validator] Wire NotCompromisedPassword in FrameworkBu…
…ndle and handle non UTF-8 password (tgalopin)

This PR was squashed before being merged into the 4.3-dev branch (closes #30898).

Discussion
----------

[Validator] Wire NotCompromisedPassword in FrameworkBundle and handle non UTF-8 password

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #30870
| License       | MIT
| Doc PR        | -

Live from #eu-fossa

Fix #30870

Commits
-------

8ac712b [Validator] Wire NotCompromisedPassword in FrameworkBundle and handle non UTF-8 password

@tgalopin tgalopin deleted the tgalopin:pwned-convert-encoding branch Apr 6, 2019

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019

@fabpot fabpot referenced this pull request May 9, 2019

Merged

Release v4.3.0-BETA1 #31435

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.