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

Escape   in Product Names for PAYONE #1256

Closed
seaneble opened this issue Sep 22, 2014 · 20 comments
Closed

Escape   in Product Names for PAYONE #1256

seaneble opened this issue Sep 22, 2014 · 20 comments
Assignees
Milestone

Comments

@seaneble
Copy link
Contributor

Good morning,

a new week, a new PAYONE 'challenge'…

I discovered that there is a problem with the current implementation for PAYONE that leads to incorrect hash calculation. We use product names with [nbsp] to prevent ugly line wraps (e.g. in front of units). These are rendered as the   character in HTML (as value of the hidden input fields). Browsers do not encode this value for some reason (!), whereas urlencode does correctly convert it to %C2%A0.
To circumvent this, I added a simple str_replace for [nbsp] to a regular whitespace.

Line 128 of Model/Payment/Payone.php:

            $arrData['de[' . $i . ']']   = specialchars(str_replace('[nbsp]', ' ', $objItem->getName() . $strOptions));

This solves it for me and PAYONE now accepts the requests. I guess one could find a cleaner solution, but I already invested two hours to find the cause.

@seaneble
Copy link
Contributor Author

Opened chromium issue 416377.

@aschempp aschempp added the bug label Sep 24, 2014
@aschempp aschempp added this to the 2.1.8 milestone Sep 24, 2014
@aschempp
Copy link
Member

Same probably applies for PayPal and other payment methods.

@Toflar
Copy link
Member

Toflar commented Sep 25, 2014

Can you check if this fixes your issue?

@seaneble
Copy link
Contributor Author

I cannot imagine how this additional method call would help in any way.
As far as I understand, it transforms the Contao-internal strings like [nbsp] into their corresponding HTML entity. This method provides the ability to see such characters in plain view in the backend.

If you use this method call now, any [nbsp] in the database will be replaced for   before it goes to the template, which in turn calls the method again.

But the real problem in this case is to have the character in the hidden input fields at all. This is actually not an Isotope bug, because Chrome under Linux should encode such characters. My line of code is only a workaround until the upstream issue has been resolved (and the fix works for older Chrome versions as well).

So, if Isotope would like to support Chrome in this special situation, it would be necessary that   never makes it to the template at all. I did this by replacing it with a regular blank.

@aschempp
Copy link
Member

aschempp commented Oct 1, 2014

Why would [nbsp] replaced in the template?

Basically, we would need to remove all HTML entities, right? Also things like ­ or &amp. What if we simply call htmlspecialchars_decode?

@aschempp aschempp modified the milestones: 2.1.9, 2.1.8 Oct 1, 2014
@aschempp
Copy link
Member

Any feedback on this topic?

@aschempp aschempp modified the milestones: 2.1.10, 2.1.9 Nov 18, 2014
@seaneble
Copy link
Contributor Author

Sorry for the delay. The chromium bug is very specific and Red Hat is currently investigating the source.

I don't think Isotope needs to do anything here. Escaping characters might be a workaround, but investing time is not appropriate seeing the nature of the chromium bug.

@seaneble
Copy link
Contributor Author

Bad idea to close this issue. There seem to be other weird constellations in which having entities in the request to PAYONE is not a good idea.

Unfortunately, Yanicks line

specialchars(
    \String::restoreBasicEntities($objItem->getName() . $strOptions),
    true
)

still leaves all the entities like   intact. I don't know about all the contao functions, but could we introduce some function call that actually replaces entities with harmless characters?

For now, I have included my str_replace again because that's the only way the customer can use the credit card integration at the moment.

@seaneble seaneble reopened this Nov 19, 2014
@aschempp
Copy link
Member

Did you find any other solution than str_replace?

@aschempp aschempp modified the milestones: 2.1.11, 2.1.10 Jan 13, 2015
@seaneble
Copy link
Contributor Author

Nope, it helps, so I closed our internal ticket. If you choose to integrate the same line or a similar functionality with a more complicated function call, that would be fine. If you opt to change nothing, we would have to patch Isotope after each update of the file. Either way would work.

@aschempp aschempp modified the milestones: 2.3.0, 2.1.11 Jan 16, 2015
@aschempp
Copy link
Member

The best possible solution is likely to remove all HTML stuff from forms sent to the payment providers. This does not only affect PayOne but also others like PayPal.
Because this will lead to a "different system behavior", we decided to do this in the next minor release.

@aschempp aschempp modified the milestones: 2.x.x, 2.3.0 Jun 14, 2015
@aschempp aschempp modified the milestones: 2.4.0, 2.x.x Apr 2, 2016
@pxpl
Copy link

pxpl commented Jul 29, 2016

I have similar problems because of '&' and '(' and ')' in the product name. (Error message 'Hashwert nicht korrekt').
Therefore I changed
$arrData['de[' . $i . ']'] = specialchars( \StringUtil::restoreBasicEntities($objItem->getName() . $strConfig), true );
to
$strName = \StringUtil::restoreBasicEntities($objItem->getName() . $strConfig);
$arrData['de[' . $i . ']'] = html_entity_decode($strName);

and this works for me, but I guess there are better solutions?

@aschempp
Copy link
Member

aschempp commented Aug 4, 2016

@qzminski please prepare a PR to fix this. Maybe the cleaning method should go in Haste StringUtils? Or is there an existing method to remove all special stuff and HTML markup?

@qzminski
Copy link
Member

qzminski commented Aug 9, 2016

See #1687. Can anyone test it please?

@seaneble
Copy link
Contributor Author

seaneble commented Aug 12, 2016

Yeah! That one works (for my problem, I don't know about @pxpl).

I would suggest moving the method to some more generic place, so other payment methods etc. can use it as well. Thanks, @qzminski!

@qzminski
Copy link
Member

I also wonder, maybe we could only do:

  1. str_replace() to replace   for the value in general
  2. htmlspecialchars(\StringUtil::restoreBasicEntities(), true) for the value in general
  3. html_entity_decode() for the value but only for hash generation

That would result in properly encoded field value in HTML and the hash generated with decoded entities (just like does Payone probably).

@aschempp
Copy link
Member

We basically need to convert every value to plain text, right?

@pxpl
Copy link

pxpl commented Aug 15, 2016

#1687 works for me, too!

@aschempp
Copy link
Member

aschempp commented Sep 3, 2016

Closed in favor of #1687

@aschempp
Copy link
Member

aschempp commented Oct 3, 2016

Should be finally fixed in 8410fd4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants