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

Bugfix: This should fix #19340, added unsanitizeInputValue around the username/password #19612

Closed

Conversation

AaronClifford
Copy link

@AaronClifford AaronClifford commented Aug 9, 2022

Description:

Bugfix: This should fix #19340, added unsanitizeInputValue around the username/password, I'm unable to test this locally so would require someone with an SMTP server available to them to test.

Review

…around the username/password in the initSmtpTransport function, requires testing.
@AaronClifford AaronClifford marked this pull request as ready for review August 9, 2022 09:31
@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2022

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Sep 3, 2022
@justinvelluppillai justinvelluppillai added Needs Review PRs that need a code review and removed Stale The label used by the Close Stale Issues action labels Sep 4, 2022
Copy link
Contributor

@peterhashair peterhashair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for submitting the PR, I think it should be other way around, the issue described password contains & to &, Common::unsanitizeInputValue will convert to & to &. Actually, I think the issue is not reproducible.

@sgiehl
Copy link
Member

sgiehl commented Sep 8, 2022

This change should actually not be needed at all. It true, that a & is stored as & in the config file. But that is actually on purpose, as all values stored in config are automatically encoded before they get saved and decoded when they are read.
See

/**
* Encode HTML entities
*
* @param mixed $values
* @return mixed
*/
protected function encodeValues(&$values)
{
if (is_array($values)) {
foreach ($values as &$value) {
$value = $this->encodeValues($value);
}
} elseif (is_float($values)) {
$values = Common::forceDotAsSeparatorForDecimalPoint($values);
} elseif (is_string($values)) {
$values = htmlentities($values, ENT_COMPAT, 'UTF-8');
$values = str_replace('$', '$', $values);
}
return $values;
}
/**
* Decode HTML entities
*
* @param mixed $values
* @return mixed
*/
protected function decodeValues(&$values)
{
if (is_array($values)) {
foreach ($values as &$value) {
$value = $this->decodeValues($value);
}
return $values;
} elseif (is_string($values)) {
return html_entity_decode($values, ENT_COMPAT, 'UTF-8');
}
return $values;
}

@sgiehl sgiehl closed this Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SMTP passwords with ampersand are saved as &
4 participants