DynamoEffects / paypal-pro-for-oscommerce

PayPal Pro payment module for osCommerce
http://addons.oscommerce.com/info/3647
5 stars 2 forks source link

Encode all strings with wpp_xml_safe() before injecting into XML #15

Open webr opened 14 years ago

webr commented 14 years ago

I've been getting some error dumps with XML syntax error as the fault string. Then I noticed that they were all from customers living in Tyne & Wear...

So in paypal_wpp.php around line 475 I added htmlspecialchars:

$xml_contents = str_replace($k, htmlspecialchars($v), $xml_contents);

Not sure if this is something that would affect all stores using this module or if it's due to previous mods I've made to mine. Also not sure if this is the cleanest way to fix the issue!

Hope it helps.

DynamoEffects commented 14 years ago

No that's a bad option because some values will be pre-encoded, so you'd be double encoding them causing other errors.

For now, just find this line in the latest version:

$the_state = $state;

and change to:

$the_state = $this->wpp_xml_safe($state);

I have updated the title to remind me what really needs to be done. All strings stored in the $order_info array in before_process() should be encoded before being passed to wpp_execute_transaction()

SteveDallas commented 14 years ago

This idea is a clean single point fix for all strings that will be swapped into the XML templates. However, it will break the PAYPAL_CANCEL_URL and PAYPAL_RETURN_URL strings due to double-encoding, if you have applied the fixes to those strings that I provided. Also, any HTML entities already in the zones table (from the World Zones contribution, for example) will also get double-encoded. PHP 5.2.3 has a switch for htmlspecialchars to prevent double-encoding, but since we have to work with earlier versions, the workaround would be:

$xml_contents = str_replace($k, htmlspecialchars(html_entity_decode($v,ENT_QUOTES,'UTF-8')), $xml_contents);

Note that if you used World Zones and Country/State Selector (or Auto State Dropdowns), this wouldn't be a problem, as Tyne & Wear is entered as 'Tyne and Wear' in the World Zones list.

This ensures that any HTML entities are decoded into the correct character set and then encodes the things that will choke the XML parser.

I haven't tested the above code, but it should work correctly. It does need to be tested using World Zones and using zone data that is encoded before being released to the unsuspecting masses.

webr commented 14 years ago

Thanks guys, I've changed to Brian's suggestion for the time being as it's only the state field that's been causing issues.