Valid email address should only start with alphanumeric

[dek5troza]

This fixes issue #27847 where regular expression allowed email address to start with special symbols. Valid email addresses should start with alphanumeric character, and as such will be rendered as email.

Added test cases from the bug report to validate, such input will not be rendered anymore as email address.

[lunny]

Could it start with a number?

[lng2020]

I'm afraid it's not right. According to that issue, the correct test case should be

test(
  "[email protected]",
  `<p>?<a href="mailto:[email protected]" rel="nofollow">[email protected]</a></p>`
)

The expected behavior of that issue is ~ not rendered but [email protected] rendered.

[dek5troza]

I'm afraid it's not right. According to that issue, the correct test case should be

test(
  "[email protected]",
  `<p>?<a href="mailto:[email protected]" rel="nofollow">[email protected]</a></p>`
)

The expected behavior of that issue is ~ not rendered but [email protected] rendered.

Ok, I misunderstood the issue. I ll give it another go. Thanks

[dek5troza]

Ok, so hopefully I understood the issue this time. It should handle those characters in the way you have described to me in the comment. Let me know, if I need to update anything else, and I will do it.

[wxiaoguang]

According to RFC and Golang's mail.ParseAddress: *[email protected] [email protected] are all valid email addresses.

Although the old regexp doesn't look good, I do not see real benefit by introducing another incorrect (hacky?) patch.

---

Sorry, maybe I misunderstood the PR, will do more tests.

[wxiaoguang]

Made a small change (remove the ? from regexp because it doesn't affect the result, there is no ? in regexp body) and added some test cases.

dek5troza#1 (not ideal enough, see below)

---

Well, I am still not sure whether we should really use this patch.

Test code:

func main() {
	a, _ := mail.ParseAddress("[email protected]")
	println(a.Address)
	a, _ = mail.ParseAddress("*[email protected]")
	println(a.Address)
}

Because [email protected] and *[email protected] are valid email addresses .......

[dek5troza]

Made a small change (remove the ? from regexp because it doesn't affect the result, there is no ? in regexp body) and added some test cases.

dek5troza#1 (not ideal enough, see below)

Well, I am still not sure whether we should really use this patch.

Test code:

func main() {
	a, _ := mail.ParseAddress("[email protected]")
	println(a.Address)
	a, _ = mail.ParseAddress("*[email protected]")
	println(a.Address)
}

Because [email protected] and *[email protected] are valid email addresses .......

I was just going by @lng2020 comment. Technically those are valid by RFC5322 but it's arguable how usable those are, as most email service providers would not allow them with rules and filters.

If more changes are needed with this, I ll be happy to help and update.

[dek5troza]

Hey guys, do you want me to update the patch, and separate ~something as valid email, or how would you like me to proceed with this?

[wxiaoguang]

I do not have strong objection for the "arguable" RFC format. But there are still nits:

1. I still think it needs to "remove the ? from regexp because it doesn't affect the result, there is no ? in regexp body".
2. It needs more tests to show the behavior, especially the edge cases like a*a and a~a if they were not covered before.
3. It needs more comments to describe the decision and the difference from the RFC.

[dek5troza]

I do not have strong objection for the "arguable" RFC format. But there are still nits:

1. I still think it needs to "remove the ? from regexp because it doesn't affect the result, there is no ? in regexp body".
2. It needs more tests to show the behavior, especially the edge cases like a*a and a~a if they were not covered before.
3. It needs more comments to describe the decision and the difference from the RFC.

Cool, I ll give it till Monday, if there are more opinions, and start updating on Monday with things you have listed above.

[karlwilbur]

I have worked with email validation before. I have a Ruby gem that does this. I am not suggesting that we try implementing a Ruby gem. 😆 But, maybe my work there might be helpful with sorting out validation and verification (tests of the validation code) within Gitea. Feel free to rip/refactor/repurpose as much of my code as you'd like:

https://rubygems.org/gems/email_validator
https://github.com/K-and-R/email_validator/blob/master/lib/email_validator.rb

I also have a pretty thorough set of tests:
https://github.com/K-and-R/email_validator/blob/master/spec/email_validator_spec.rb

I hope it helps!

[wxiaoguang]

Inactive for long time, ping ~~

[dek5troza]

Inactive for long time, ping ~~

You are right, this slipped through. I ll take a stab at it this weekend.

dismiss