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

AbstractRequest.php Unescaped & in createElement() #74

Open
soeurdefeo opened this issue Feb 11, 2018 · 4 comments
Open

AbstractRequest.php Unescaped & in createElement() #74

soeurdefeo opened this issue Feb 11, 2018 · 4 comments

Comments

@soeurdefeo
Copy link

The createElement() call in line 112 throws an warning (Warning: DOMDocument::createElement(): unterminated entity reference) and does not set ne child-node correct if you send e.g. for the errorURL an string with & because the value is not escaped by default in this method.
Please add before

$value = str_replace('&', '&', $value);

Similiar is already done in Mpay24Order.php __set() method line 176.

@stefanpolzer
Copy link
Contributor

@soeurdefeo just replacing any & like so, involves some other risks e.g. if the string is already escaped like this: https://example.com/path?param1=value2&param2=value2
we end up with https://example.com/path?param1=value2&param2=value2.

I will check if there is a way where we can fix this without open a other issue as described.
I was not involved in the Mpay24Order::__set() method so I can not say if this is done right there.

@volkanakb
Copy link

@stefanpolzer what about using -> DOMDocument::createTextNode(html_entity_decode('https://example.com/path?param1=value2&param2=value2'));

@stefanpolzer
Copy link
Contributor

@soeurdefeo can you check if this is now solved in the last PR #90 that was now merged into the master tree?

@mdaskalov
Copy link
Collaborator

mdaskalov commented Feb 17, 2020

doesn't work for me. Ampersand is never escaped.
It also doesn't work if you set again previously set Node:

$mdxi->Order->BillingAddr->Name = 'John > Doe';
$mdxi->Order->BillingAddr->Name = 'apos: \', quot: ", lt: <, amp: &, rt: >';

Best practice is to use createTextNode which escapes the text accordingly. I've started a new branch xml-escape to implement it using this method.

The question is if we could/should be backward compatible. If we have already escaped strings we shouldn't escape again otherwise we will have: &amp; -> &amp;amp;

It would be difficult if we want to support both escaped and not escaped versions:

$mdxi->Order->TID = '< & >';
$mdxi->Order->UserField = '&lt;&amp;&gt;';

Maybe html_entity_decode with ENT_XML1 can be used to detect if already escaped...

Alternatively htmlentities could be used with the double_encode flag set to false:

       $value = htmlentities($value, ENT_XML1, "UTF-8", false);

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

No branches or pull requests

4 participants