Fix getSubject encoding

[PieterCappelle]

Fix for issues #6597 & #8094. Short description: If your store name has a special character in it, such as an apostrophe, it will be converted to a numerical character reference in any email subject. This will fix that problem.

Contribution checklist

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

[korostii]

That's a nice temporary workaround, but shouldn't a proper fix prevent the special characters from getting encoded in the first place?

[miguelbalparda]

Thank you for your submission @PieterCappelle! Have you seen this happening some place else? I'm going to review this and get back to you once I have some feedback.

[korostii]

Hi @miguelbalparda
Scope of both the issue and the solution provided by @PieterCappelle are limited to the email subjects as generated by Magento CE. Could you please clarify what kind of information are you expecting to get by asking "Have you seen this happening some place else?" ?
Do you need more test cases, or are you saying that a pull request is expect to fix multiple similar bugs in order to get merged?

And, a bit off-topic, it seems to be customary to move the pull request to "Review In Progress" if you started reviewing. The "Response Needed" seems to mean that you require additional information prior to proceeding with the PR.

[miguelbalparda]

I want to know if he experienced this issue in a different area so we can add some more code before merging.

[korostii]

I am very sorry to be bothering you, but your answer still has me puzzled.

I just don't see, what kind of a different area would be relevant in this case and there should be an attempt to put in "some more code"? Is there some kind of a limit?
This is an acknowledged bug, how would potentially having a similar bug someplace else prevent the code from merging? Are you also implying that no bugs should be fixed unless all of them are discovered?

[PieterCappelle]

Do I have to do anything right now? Is everything clear?

[miguelbalparda]

Hi @PieterCappelle! All good, my original question is still valid but I'm still processing this PR. Thanks!

[PieterCappelle]

@miguelbalparda No didn't see that anywhere else. Only problem occurs in subject of e-mails I think.

[miguelbalparda]

What do you think about doing the same inside here?
If this is for the encoding of the variables of an email and not for the uncompiled body, putting the decode in there might prevent the same issue from happening in the processed body of the email.

[korostii]

@miguelbalparda, it's not just the variables where the special characters might appear, it's the whole subject. Here's an example.

Besides, having special characters escaped inside the body is okay as long the email template has type="html".

[miguelbalparda]

I'm trying to apply this fix to the entire email and not only to the subject. Since we now know this might happen some place else, it might be a good idea to try to apply this in a more general way instead of only in one particular issue.
We will need to come to a conclusion about where to apply this fix, this can be done for the entire template but it might be better to do it in a different part of the Template class or in the maybe even in the AbstractTemplate class.

[korostii]

In such case it would better to change the default for the {{trans }} and/or {{var }} directives from "|escape" to "|raw". That would at least keep the escaping wherever it is requested by the template explicitly (meaning: intentionally).

However, there might be security considerations. All these variables are being escaped by default for a reason: they might very well contain malicious user-generated content.

[fooman]

Hi @PieterCappelle thanks for the pull request. Would you mind trialling something like

        return sprintf(
            '=?utf-8?B?%s?=',
            base64_encode(
                htmlspecialchars_decode(
                    $this->getProcessedTemplateSubject($this->_getVars()),
                    ENT_QUOTES
                )
            )
        );

which should allow utf-8 characters to be part of the subject line - it's something I use in EmailAttachments

[korostii]

So apparently there is already a different fix merged into develop: e89d09d

Maybe notify a bit sooner next time (in order to prevent people from wasting effort on a pull request which won't be merged)?

[ishakhsuvarov]

@PieterCappelle Unfortunately other fix for the same issue is already delivered into the develop branch, I have to close this PR now. Please feel free to reopen if you have anything to add over the fix mentioned.
Thank you for collaboration.

@korostii We try, but not always succeed in keeping track of the fixes coming through different channels, I am as concerned about wasting effort as everyone else.