Allow apostrophes in email address #12729
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The email validation regex is too restrictive and does not allow apostrophes. Even with this change, the regex is probably still too restrictive. I am personally in favor of advice outlined in [this post](https://davidcel.is/posts/stop-validating-email-addresses-with-regex/), which is, to summarize: 1) Don't validate emails with regex, send an email to it instead 2) If you must, use a very permissive one Since both (1) is probably beyond the scope of this bugfix, and (2) breaks existing tests (and I don't know if there are good reasons for their being there), I want to recommend that we fix the specific problem outlined in the BZ first, and try to address the general problem in a follow up. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1379420
|
Checked commit imtayadeway@959d6f0 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 app/models/user.rb
|
chessbyte
reviewed
Nov 20, 2016
| @@ -36,7 +36,7 @@ class User < ApplicationRecord | |||
|
|
|||
| validates_presence_of :name, :userid | |||
| validates :userid, :uniqueness => {:conditions => -> { in_my_region } } | |||
| validates_format_of :email, :with => /\A([\w\.\-\+]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i, | |||
| validates_format_of :email, :with => /\A([\w\.\-\+']+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i, | |||
There was a problem hiding this comment.
@imtayadeway @gtanzillo Maybe this regex should be used, instead?
chessbyte
pushed a commit
that referenced
this pull request
Nov 20, 2016
Allow apostrophes in email address (cherry picked from commit f728774) https://bugzilla.redhat.com/show_bug.cgi?id=1396489
|
Euwe Backport details: $ git log -1
commit 6da4b096e7bd0bf6d232f34e2e4269a4b4761b0f
Author: Gregg Tanzillo <[email protected]>
Date: Thu Nov 17 17:21:11 2016 -0500
Merge pull request #12729 from imtayadeway/fix/email-validation
Allow apostrophes in email address
(cherry picked from commit f728774fb1db4c4e48246f27dbb9f0f50a4162d8)
https://bugzilla.redhat.com/show_bug.cgi?id=1396489 |
chessbyte
pushed a commit
that referenced
this pull request
Dec 13, 2016
Allow apostrophes in email address (cherry picked from commit f728774) https://bugzilla.redhat.com/show_bug.cgi?id=1396490
|
Darga Backport details: $ git log -1
commit b913c33bdcad5bc30ae8f280fda3cc0bb27c4ebb
Author: Gregg Tanzillo <[email protected]>
Date: Thu Nov 17 17:21:11 2016 -0500
Merge pull request #12729 from imtayadeway/fix/email-validation
Allow apostrophes in email address
(cherry picked from commit f728774fb1db4c4e48246f27dbb9f0f50a4162d8)
https://bugzilla.redhat.com/show_bug.cgi?id=1396490 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The email validation regex is too restrictive and does not allow
apostrophes.
Even with this change, the regex is probably still too
restrictive. I am personally in favor of advice outlined in
this post,
which is, to summarize:
Since both (1) is probably beyond the scope of this bugfix, and (2)
breaks existing tests (and I don't know if there are good reasons for
their being there), I want to recommend that we fix the specific problem
outlined in the BZ first, and try to address the general problem in a
follow up.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1379420
@miq-bot add-label bug, core, blocker
@miq-bot assign @gtanzillo