-
-
Notifications
You must be signed in to change notification settings - Fork 835
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
fix: change length of email field #4118
Conversation
05d5d65
to
6cb0d27
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the reason it's failing is that the email tokens table also needs its email column to be increased in length, could you please add a migration for that as well? MySQL probably just truncates without failing whereas PgSQL is more strict so it fails. Which is likely why tests succeeded in 1.x
but that also means email tokens (needed for account confirmations) won't work properly for emails longer than 150 in 1.x, it looks like confirming the account in 1.x will trigger the app to change the email back to the truncated value, assuming that's what's happening (https://github.com/flarum/framework/blob/1.x/framework/core/src/User/Command/ConfirmEmailHandler.php#L45)
99e263a
to
e3f0b20
Compare
Yep, seems like it was exactly what you mentioned – Thanks for looking into this. Will PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
2.x
pendent to #4117Fixes #0000
It appears as if the email validator allows 254 chars for the entire email, while the DB field only allows 150. This results in email addresses simply being truncated without any error message when creating users with longer email addresses.
Changes proposed in this pull request:
Increase length of email field to 254 chars to match the validator
Reviewers should focus on:
See https://www.rfc-editor.org/errata/eid1690
Screenshot
Necessity
Confirmed
composer test
).Required changes: