Fix email html encoding #12611
Fix email html encoding #12611
Conversation
|
Thanks for the PR @Kallaen 👍 (when reviewing the PR, since it will now allow for some XSS prone characters, we should specifically look for XSS injections in the email address) |
|
Thanks for the reply. Yea, I saw the test failing. I'll look into it again. |
|
To better check for XSS here, can you change the UITestFixture.php class so the users used in UI tests have XSS email addresses? You'd have to change this line: https://github.com/matomo-org/matomo/blob/3.x-dev/tests/PHPUnit/Fixtures/UITestFixture.php#L71 to be something like: It would also be good to change the super user's email address to an XSS one, since I think that user is used throughout the UI tests. To do that, you'd have to add a eg: |
| @@ -572,7 +572,8 @@ public static function isValidEmailString($emailAddress) | |||
| { | |||
| /** @var \Zend_Validate_EmailAddress $zendEmailValidator */ | |||
| $zendEmailValidator = StaticContainer::get('Zend_Validate_EmailAddress'); | |||
| return $zendEmailValidator->isValid($emailAddress); | |||
|
|
|||
| return $zendEmailValidator->isValid(mb_convert_encoding($emailAddress, "UTF-8", "HTML-ENTITIES")); | |||
There was a problem hiding this comment.
There's a core Piwik function for unsanitizing values: \Piwik\Common:: unsanitizeInputValue(...). (see https://github.com/matomo-org/matomo/blob/3.x-dev/core/Common.php#L363 ).
Can you use that instead of mb_convert_encoding()? Depending on a function we control means we can change how it works ourselves & in one place, instead of having to look for every use of, eg, mb_convert_encoding().
| } | ||
| } else { | ||
| $email = mb_convert_encoding($email, "UTF-8", "HTML-ENTITIES"); | ||
| } |
There was a problem hiding this comment.
I noticed in a couple lines the tabbing is off by one char. Can you fix this?
|
Hi @Kallaen |
|
@mattab Sorry, I can't find the time to do it at the moment. |
|
@Kallaen Still busy? Or do you maybe have some time the coming days? |
|
@sgiehl As the github user got deleted I guess this pull request won't be changed anymore. |
|
Closing then. Maybe some is keen on recreating this PR with the suggested changes... |
Fixing: E-Mail addresses with special characters are not working. Changed the mail address to match the original input for the validator, add user and update user.
Made in correspondence to and fixes #11796