Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Unexpected expection thrown when testing the email settings with an invalid email #12252

Closed
MikeAlhayek opened this issue Aug 23, 2022 · 5 comments · Fixed by #12253
Closed
Milestone

Comments

@MikeAlhayek
Copy link
Member

Describe the bug

When testing the email settings with an invalid email format, the SmtpService display the following error

An error occurred while sending an email: 'Unexpected 'A' token at offset 5'

To Reproduce

Steps to reproduce the behavior:

  1. Enable OC.Email feature
  2. Click on Configuration >> Settings >> Email
  3. Make sure the email settings is configured. then click on "Test settings" button
  4. Provide invalid formatted email address in the "sender" field.
  5. Provide "To" email, "Subject" and "Body" and click send.

Expected behavior

A clear and concise description of what you expected to happen.

Screenshots

If applicable, add screenshots to help explain your problem.

MikeAlhayek added a commit to MikeAlhayek/OrchardCore that referenced this issue Aug 23, 2022
@hishamco
Copy link
Member

hishamco commented Aug 25, 2022

Before I dig into this PR, while I worked a lot in this area, I'm suggesting to replace your PR with the one introduced in #11130 /cc @sebastienros your thought, because we already supported localization for data annotations

@MikeAlhayek
Copy link
Member Author

Not sure that I understand Hisham. The referenced PR seems to be doing something completely different. PR #12253 fixes missing validation in the SmtpService.

@hishamco
Copy link
Member

My PR handles both the validation and localization in data annotations attributes, so what we need here is something like EmailAddressAttribute which is handle the validation that you are trying to do using IValidatableObject

So, no need to introduce such kind of validation because we already supported localization in data annotations level

@MikeAlhayek
Copy link
Member Author

@hishamco you may want to review that PR.

@hishamco
Copy link
Member

You could review if you can, also I'm asking @sebastienros and @agriffard to review it if it's possible

@sebastienros sebastienros added this to the 1.x milestone Sep 1, 2022
@Skrypt Skrypt modified the milestones: 1.x, 1.5 Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants