Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[BUG] Can't use normal email addresses to #, create account, or create ticket. #130

Closed
1 of 2 tasks
apboon opened this issue Jan 22, 2019 · 3 comments
Closed
1 of 2 tasks

Comments

@apboon
Copy link

apboon commented Jan 22, 2019

Is this a BUG REPORT or FEATURE REQUEST?:

  • BUG
  • FEATURE

What happened:
Can't use normal email addresses to #, create account, or create ticket.

What did you expect to happen:
Email address to be evaluated as valid.

How to reproduce it (as minimally and precisely as possible):
# with a modern (post 2014) email address like person@company.agency or person@local-company.amsterdam.

Anything else we need to know?:
Trudesk looks like a wonderful piece of software. Been searching far and wide for exactly this: NodeJS, MongoDB, SPA, JSON API, very user friendly, a breeze to work with, wonderful! :)
But at the moment unable to use the software because our company uses modern email address.. minor problem though..

The project's code contains 3 instances of the RegEx ^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,3})+$ of which I notice:

  • Many characters with special meaning lose their special meaning inside of character classes ([]). The dot (.) does not require escaping inside of a character class. Luckily the stray slash is being ignored and email addresses like john\doe@example.com aren't considered valid. The original RegEx can be written down as ^\w+([.-]?\w+)*@\w+([.-]?\w+)*(\.\w{2,3})+$.

  • Taking another look I notice the ? modifier behind the character classes which does not seem to have a purpose. Partial RegEx \w+([.-]?\w+)* works exactly the same as \w+([.-]\w+)* (without the ?). If no dots or dashes are found, the first part of the RegEx \w+ already actures everything, after which the capturing group optionally captures any trailing .something or -something. Making the dot/dash optional is similar to matching (\w+)*, which is already covered by the initial \w+ part. The original RegEx can therefore be simplified as ^\w+([.-]\w+)*@\w+([.-]\w+)*(\.\w{2,3})+$ without losing any of its meaning. Same outcome as the original; just slightly simplified notation.

  • I took a list of example email addresses from the internet (https://gist.github.com/cjaoude/fd9910626629b53c4d25) and tested the regular expression using https://regex101.com/. Simply updating the \w{2,3} to \w+ (and leaving everything else in tact) makes the RegEx compatible with most modern (post 2014) email addresses (with TLDs like .cafe, etc). The resulting RegEx would be: ^\w+([.-]\w+)*@\w+([.-]\w+)*(\.\w+)+$

Conclusion/solution:
Change from ^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,3})+$ to ^\w+([.-]\w+)*@\w+([.-]\w+)*(\.\w+)+$

Some might argue that the regular expression would still not cover email addresses like john+doe@example.com, but there might be good reason to exclude those as the plus sign has special meaning; mail sent to john+doe@example.com is actually being received in the john@example.com mailbox and flagged with tag doe.

Cheers! :)

Environment:

  • Trudesk Version: Latest/master
  • OS (e.g. from /etc/os-release): Not relevant
  • Node.JS Version: Not relevant
  • MongoDB Version: Not relevant
  • Is this hosted on cloud.trudesk.io: http://docker.trudesk.io/
@polonel
Copy link
Owner

polonel commented Jan 22, 2019

Thank you. I will include these fixes in the next update. I appreciate the diving into the code and helping point out these flaws. This is a huge help right now as I'm refactoring a lot of outdated code.

@apboon
Copy link
Author

apboon commented Jan 22, 2019

Awesomeness! ^^
Hey question.. if I see something that could be improved, do you want me to fork and create a pull request with a fix? Or you rather have people open an issue?
I would not have mind the former.

@polonel
Copy link
Owner

polonel commented Jan 22, 2019

I prefer a pull request. Help tracks your contribution. Just make sure it's against develop branch.

@polonel polonel changed the title Can't use normal email addresses to #, create account, or create ticket. [BUG] Can't use normal email addresses to #, create account, or create ticket. Jan 28, 2019
polonel added a commit that referenced this issue Feb 2, 2019
## [1.0.6](v1.0.5...v1.0.6) (2019-02-02)

### Bug Fixes

* **attachments:** uploading office mime-types [#140](#140) ([b47da40](b47da40))
* **chat:** chat boxes under some buttons ([c337c76](c337c76))
* **dates:** overdue on dashboard ([921e258](921e258))
* **groups:** issue preventing save ([7208253](7208253))
* **ldap:** crash if no results are returned ([8ff63ba](8ff63ba))
* **login:** username whitespaces ([282d725](282d725))
* **messages:** remove ajax link from start conversation ([988dfa9](988dfa9))
* **notifications:** unable to clear all notifications ([4f24c8c](4f24c8c))
* **reports:** invalid date format ([808a740](808a740))
* **reports:** invalid date string ([0914d91](0914d91))
* **socket:** high memory usage on notification updates ([b647d4c](b647d4c))
* **ticket:** priority not updating in realtime ([721f42d](721f42d))
* **unzip:** out dated dependency [#139](#139) ([b0aab01](b0aab01))
* **url:** invalid parse ([1738287](1738287))
* **validation:** email validation for modern tlds [#130](#130) ([febcbdf](febcbdf))
polonel added a commit that referenced this issue Feb 2, 2019
## [1.0.6](v1.0.5...v1.0.6) (2019-02-02)

### Bug Fixes

* **attachments:** uploading office mime-types [#140](#140) ([b47da40](b47da40))
* **chat:** chat boxes under some buttons ([c337c76](c337c76))
* **dates:** overdue on dashboard ([921e258](921e258))
* **editor:** crash on invalid directory ([bc60899](bc60899))
* **groups:** issue preventing save ([7208253](7208253))
* **ldap:** crash if no results are returned ([8ff63ba](8ff63ba))
* **login:** username whitespaces ([282d725](282d725))
* **messages:** remove ajax link from start conversation ([988dfa9](988dfa9))
* **notifications:** unable to clear all notifications ([4f24c8c](4f24c8c))
* **reports:** invalid date format ([808a740](808a740))
* **reports:** invalid date string ([0914d91](0914d91))
* **socket:** high memory usage on notification updates ([b647d4c](b647d4c))
* **ticket:** priority not updating in realtime ([721f42d](721f42d))
* **unzip:** out dated dependency [#139](#139) ([b0aab01](b0aab01))
* **url:** invalid parse ([1738287](1738287))
* **validation:** email validation for modern tlds [#130](#130) ([febcbdf](febcbdf))
polonel added a commit that referenced this issue Feb 2, 2019
* **attachments:** uploading office mime-types [#140](#140) ([b47da40](b47da40))
* **chat:** chat boxes under some buttons ([c337c76](c337c76))
* **dates:** overdue on dashboard ([921e258](921e258))
* **editor:** crash on invalid directory ([bc60899](bc60899))
* **groups:** issue preventing save ([7208253](7208253))
* **ldap:** crash if no results are returned ([8ff63ba](8ff63ba))
* **login:** username whitespaces ([282d725](282d725))
* **messages:** remove ajax link from start conversation ([988dfa9](988dfa9))
* **notifications:** unable to clear all notifications ([4f24c8c](4f24c8c))
* **reports:** invalid date format ([808a740](808a740))
* **reports:** invalid date string ([0914d91](0914d91))
* **socket:** high memory usage on notification updates ([b647d4c](b647d4c))
* **ticket:** priority not updating in realtime ([721f42d](721f42d))
* **unzip:** out dated dependency [#139](#139) ([b0aab01](b0aab01))
* **url:** invalid parse ([1738287](1738287))
* **validation:** email validation for modern tlds [#130](#130) ([febcbdf](febcbdf))
@polonel polonel closed this as completed Feb 2, 2019
@polonel polonel added production and removed review labels Feb 2, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants