Project

General

Profile

Actions

Defect #36969

closed

EmailAddress regex matches invalid email addresses

Added by salman mp almost 2 years ago. Updated 5 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Accounts / authentication
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

Description

There is a regex in the EmailAddress class, that matches some invalid email address like these:

test,email@example.com
,test@example.com
$test@example.com

class EmailAddress < ActiveRecord::Base
  include Redmine::SafeAttributes

  EMAIL_REGEXP = /\A([^@\s]+)@((?:[-a-z0-9]+\.)+(?:(?:xn--[-a-z0-9]+)|(?:[a-z]{2,})))\z/i

May be better to use URI::MailTo::EMAIL_REGEXP instead.


Files

36969.patch (1.5 KB) 36969.patch Go MAEDA, 2022-04-18 10:42
36969-v2.patch (1.92 KB) 36969-v2.patch Go MAEDA, 2022-04-20 02:43

Related issues

Related to Redmine - Defect #37922: Valid email address is not allowedClosed

Actions
Related to Redmine - Defect #6088: eMail in uncommon formats considered invalidClosed2010-08-09

Actions
Actions #1

Updated by Go MAEDA almost 2 years ago

Setting the target version to 5.1.0.

Actions #2

Updated by Go MAEDA almost 2 years ago

Added a test to the patch.

Actions #3

Updated by Mischa The Evil almost 2 years ago

  • Description updated (diff)

This effectively changes EmailAddress::EMAIL_REGEXP from:

/\A([^@\s]+)@((?:[-a-z0-9]+\.)+(?:(?:xn--[-a-z0-9]+)|(?:[a-z]{2,})))\z/i
to:
/\A[a-zA-Z0-9.!\#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*\z/
as URI::MailTo::EMAIL_REGEXP is defined as such in the Ruby source (https://github.com/ruby/ruby/blob/master/lib/uri/mailto.rb#L55).
This definition is effectively a Ruby port1 of the JavaScript- and Perl-compatible regex example given in the HTML Living Standard:
/^[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/

Some quick notes on this change:
  • it fixes the cases of the first two email address examples, but $test@example.com still matches (and a little search will probably give more edge-cases);
  • the current custom regex includes capture groups while URI::MailTo::EMAIL_REGEXP doesn't (which changes the return value in some cases and thus may break plugins that depend on the current value of EmailAddress::EMAIL_REGEXP) e.g.:
    'test@example.com'.match(EmailAddress::EMAIL_REGEXP)
    => #<MatchData "test@example.com" 1:"test" 2:"example.com">
    
    'test@example.com'.match(URI::MailTo::EMAIL_REGEXP)
    => #<MatchData "test@example.com">
    
  • given the previous note, this might be something that should be shipped in a major release (6.0.0) instead of a minor release (5.1.0).

1 https://github.com/ruby/ruby/blob/master/lib/uri/mailto.rb#L54

Actions #4

Updated by Go MAEDA almost 2 years ago

Mischa The Evil wrote:

  • given the previous note, this might be something that should be shipped in a major release (6.0.0) instead of a minor release (5.1.0).

I don't think the change should be delivered in 6.0.0 instead of 5.1.0.

In Redmine, the change of version number from 5.0.0 to 5.1.0 is not a minor release but a major release. For example, when the version number changed from 3.0.0 to 3.1.0 or from 4.0.0 to 4.1.0, many new features were added and some plugins stopped working.

If this change cannot be delivered in 5.1.0 due to plugin compatibility, I am afraid that 5.1.0 can only include a few bug fixes and cannot include any new features.

Actions #5

Updated by Go MAEDA over 1 year ago

  • Related to Defect #37922: Valid email address is not allowed added
Actions #6

Updated by Go MAEDA 5 months ago

  • Status changed from New to Closed
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed the patch in r22332.

Actions #7

Updated by Go MAEDA about 2 months ago

  • Related to Defect #6088: eMail in uncommon formats considered invalid added
Actions

Also available in: Atom PDF