Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-9715

Fix email address regular expression or adjust email regular expression unit tests

    Details

    • Type: Bug
    • Status: Unverified Fix
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.8-RC1
    • Component/s: None
    • Labels:
      None

      Description

      There were 2 failures:
       
      1) phpbb_regex_email_test::test_negative_match with data set #2 ('Foo.@example.com')
      Failed asserting that <integer:1> matches expected <integer:0>.
       
       
      2) phpbb_regex_email_test::test_negative_match with data set #3 ('foo..123@example.com')
      Failed asserting that <integer:1> matches expected <integer:0>.
      

      It has to be decided whether we want to fix the regex or exclude those two tests.

      The following tests are already excluded because the formats aren't widely used.

      //array('"John Doe"@example.com'),
      //array('Alice@[192.168.2.1]'),		// IPv4
      //array('Bob@[2001:0db8:85a3:08d3:1319:8a2e:0370:7344]'), // IPv6
      

        Issue Links

          Activity

          Hide
          ckwalsh Cullen Walsh [X] (Inactive) added a comment -

          This Regex supposedly conforms to all RFC Email tests:

          /^([\w\!\#$\%\&\'\*\+\-\/\=\?\^\`{\|\}\~]\.)*[\w\!\#$\%\&\'\*\+\-\/\=\?\^\`{\|\}\~]@((((([a-z0-9]

          {1}[a-z0-9\-]{0,62}[a-z0-9]{1}

          )|[a-z])\.)+[a-z]

          {2,6}

          )|(\d

          {1,3}\.){3}\d{1,3}

          (\:\d

          {1,5}

          )?)$/i

          Via http://fightingforalostcause.net/misc/2006/compare-email-regex.php

          Show
          ckwalsh Cullen Walsh [X] (Inactive) added a comment - This Regex supposedly conforms to all RFC Email tests: /^( [\w\!\#$\%\&\'\*\+\-\/\=\?\^\`{\|\}\~] \.)* [\w\!\#$\%\&\'\*\+\-\/\=\?\^\`{\|\}\~] @((((( [a-z0-9] {1} [a-z0-9\-] {0,62} [a-z0-9] {1} )| [a-z] )\.)+ [a-z] {2,6} )|(\d {1,3}\.){3}\d{1,3} (\:\d {1,5} )?)$/i Via http://fightingforalostcause.net/misc/2006/compare-email-regex.php
          Hide
          bantu Andreas Fischer added a comment - - edited

          Cullen, do you want to work on this?

          Basically just replace the regex in get_preg_expression() if that doesn't break anything.
          I'd suggest to extend the tests with the tests on the page you linked.

          Show
          bantu Andreas Fischer added a comment - - edited Cullen, do you want to work on this? Basically just replace the regex in get_preg_expression() if that doesn't break anything. I'd suggest to extend the tests with the tests on the page you linked.
          Hide
          bantu Andreas Fischer added a comment -

          Show
          bantu Andreas Fischer added a comment -
          Hide
          narqelion narqelion [X] (Inactive) added a comment -

          This Regex supposedly conforms to all RFC Email tests:

          "Supposedly" being the operative word here. Despite it's self aggrandized label of "The perfect email regex" it is quite imperfect. The truth is it will fail perfectly legitimate functional email addresses and if you bother to research it you will find that it has been widely criticized. "it's better to accept a few invalid addresses than reject any valid ones" is the only thing that should be taken seriously from the page you linked to. With the new ccTLD's (non-latin) and the explosion of internationalized domain names sure to come I would caution against using such a narrow regex to try and validate email addresses as you will constantly have to tweak it or deal with users submitting bug after bug against it. Worry less about "validating" the format of email addresses because the only way to 100% verify an email address is to send it an email. The negatives of accepting an invalid address in the form entry are far outweighed by the negatives of rejecting valid email addresses during the registration process especially when email validation is the chosen method of account activation.

          Show
          narqelion narqelion [X] (Inactive) added a comment - This Regex supposedly conforms to all RFC Email tests: "Supposedly" being the operative word here. Despite it's self aggrandized label of "The perfect email regex" it is quite imperfect. The truth is it will fail perfectly legitimate functional email addresses and if you bother to research it you will find that it has been widely criticized. "it's better to accept a few invalid addresses than reject any valid ones" is the only thing that should be taken seriously from the page you linked to. With the new ccTLD's (non-latin) and the explosion of internationalized domain names sure to come I would caution against using such a narrow regex to try and validate email addresses as you will constantly have to tweak it or deal with users submitting bug after bug against it. Worry less about "validating" the format of email addresses because the only way to 100% verify an email address is to send it an email. The negatives of accepting an invalid address in the form entry are far outweighed by the negatives of rejecting valid email addresses during the registration process especially when email validation is the chosen method of account activation.
          Hide
          bantu Andreas Fischer added a comment -

          Does the new regular expression disallow any valid email address the old one has accepted? If that is the case, please list those address forms so we can add them to our tests. Or even better, add them yourself and let me pull your changes.

          Show
          bantu Andreas Fischer added a comment - Does the new regular expression disallow any valid email address the old one has accepted? If that is the case, please list those address forms so we can add them to our tests. Or even better, add them yourself and let me pull your changes.

            People

            • Assignee:
              bantu Andreas Fischer
              Reporter:
              bantu Andreas Fischer
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development