PrestaShop / PrestaShop

PrestaShop is the universal open-source software platform to build your e-commerce solution.
https://www.prestashop-project.org/
Other
8.14k stars 4.8k forks source link

webservice does not handle XML properly, whitespaces are not ignored #14840

Closed tobiaseriksson closed 2 years ago

tobiaseriksson commented 5 years ago

Using Prestashop version 1.7.5

Describe the bug I use the webservices API, and trying to create a Product it complains when validating id_supplier The XML looks like this

11 The problem seems to be related to that the content of the tag contains spaces and newlines. Bad practice from me maybe, but it worked in 1.6 and now it stopped working. I my opinion the error messages returned upon validation should be more elaborate. I had to figure out where in the code this was done. Anyhow, my printout says Validate::isUnsignedInt '\n 11\n ' is not valid **To Reproduce** See above **Screenshots** N/A **Additional information** PrestaShop version: 1.7.5 PHP version: 7.2.19
tobiaseriksson commented 5 years ago

After some analysis my proposal is that you change the code inside classes/webservice/WebserviceRequest.php in function protected function saveEntityFromXml($successReturnCode) such that the line that says 1519 if (isset($fieldProperties['setter'])) { 1510 // if we have to use a specific setter 1511 if (!$fieldProperties['setter']) { 1512 // if it's forbidden to set this field 1513 $this->setError(400, 'parameter "' . $fieldName . '" not writable. Please remove this attribute of this XML', 93); 1514 1515 return false; 1516 } else { 1517 $object->{$fieldProperties['setter']}((string) $attributes->$fieldName); 1518 }

would be changed so that line 1517 would trim/strip away all the whitespaces, newlines, tabs, spaces, ...

khouloudbelguith commented 5 years ago

Hi @tobiaseriksson,

I used this script with PS1.7.6.0 to create a new product via Webservice

<html><head><title>pUT DATA</title></head><body>
<?php

// Here we define constants /!\ You need to replace this parameters
define('DEBUG', true);
ini_set('display_errors','on');
define('PS_SHOP_PATH', 'http://shop.com');
define('PS_WS_AUTH_KEY', 'Key');
require_once('PSWebServiceLibrary.php');
$psXML = <<<XML
<prestashop>
<product>
  <id/>
  <name>
  <language id ="1">"product_WS" </language></name>
  <price>100</price>
  <id_shop_default>1</id_shop_default>
  <id_manufacturer/>
      <id_supplier>
        1
      </id_supplier>
</product>
</prestashop>
XML;
$webService = new PrestaShopWebservice(PS_SHOP_PATH, PS_WS_AUTH_KEY, DEBUG);
$xml = new SimpleXMLElement($psXML);
$opt = array( 'resource' => 'products' );
$opt['postXml'] = $xml->asXML();
$xml = $webService->add( $opt );
?>

But I have this response

RETURN HTTP BODY
<?xml version="1.0" encoding="UTF-8"?>
<prestashop xmlns:xlink="http://www.w3.org/1999/xlink">
<errors>
<error>
<code><![CDATA[85]]></code>
<message><![CDATA[Validation error: "La propriété Product->id_supplier n'est pas valide."]]></message>
</error>
</errors>
</prestashop>

Thanks to check & feedback.

tobiaseriksson commented 5 years ago

Correct this is the error you would get back and that would not be right ofcourse, XML tags should not care about whitespaces, as far as I understand anyway -Tobias

khouloudbelguith commented 5 years ago

@tobiaseriksson, if we the classic way <id_supplier>1</id_supplier> there is no issue. ping @marionf, @colinegin what do you think? can we add this feature: whitespaces will be ignored?

Thanks!

tobiaseriksson commented 5 years ago

Here is a reference; https://www.oracle.com/technetwork/articles/wang-whitespace-092897.html it's up to you to decide :-)

ghost commented 5 years ago

@PrestaShop/prestashop-core-developers What do you think guys?

Progi1984 commented 5 years ago

@samuel-pires I think to a problem : if we ignore whitespaces, description with newline, spaces, and so on will be deleted.

PierreRambaud commented 5 years ago

If you want to use xml properly with newlines and white-spaces use CDATA.

<id_supplier>
  <![CDATA[1]]>
</id_supplier>
unlocomqx commented 2 years ago

@PierreRambaud using CDATA won't help as far as I tested https://github.com/PrestaShop/PrestaShop/issues/26899#issuecomment-988795621

I think it's better to return int values without CDATA (thus setting the example and making it easier to copy the response to use in a PUT request)

<id_supplier>1</id_supplier>

As far as I understood, CDATA is only needed when the value has the potential to be interpreted as XML and break the document wdyt, if it's a good idea, I could create a separate issue/PR for it

PierreRambaud commented 2 years ago

@PierreRambaud using CDATA won't help as far as I tested #26899 (comment)

I think it's better to return int values without CDATA (thus setting the example and making it easier to copy the response to use in a PUT request)

<id_supplier>1</id_supplier>

As far as I understood, CDATA is only needed when the value has the potential to be interpreted as XML and break the document wdyt, if it's a good idea, I could create a separate issue/PR for it

I confirm that it won't help :sweat_smile: I tried a lot of things, the CDATA is only here to escape unwanted content. The right way to use it is as @unlocomqx said :+1:

PierreRambaud commented 2 years ago

I'm closing it as it's the default XML behavior