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

[WS][1.7.8] Cannot receive blank schemas for resources #26559

Closed
2 tasks done
Tracked by #10069
Olimbot opened this issue Nov 9, 2021 · 20 comments · Fixed by #28280
Closed
2 tasks done
Tracked by #10069

[WS][1.7.8] Cannot receive blank schemas for resources #26559

Olimbot opened this issue Nov 9, 2021 · 20 comments · Fixed by #28280
Labels
1.7.7.7 Affects versions Bug Type: Bug Fixed Resolution: issue closed because fixed Minor Severity: minor bug > https://build.prestashop.com/news/severity-classification PR available Solution: issue is being addressed Verified The issue has been reproduced Webservice Label: Which BO under menu is concerned WS Category: Web Service
Milestone

Comments

@Olimbot
Copy link

Olimbot commented Nov 9, 2021

Prerequisites

Describe the bug and add screenshots

It's the same issue that #21748 and #19305

Here you have the PHP version 7.4 compatible with prestashop 1.7.8 but the webservice works with PHP 7.3 but not with 7.4 :
https://devdocs.prestashop.com/1.7/basics/installation/system-requirements/#php-compatibility-chart

New.Recording.-.4_19_2022.8_04_52.PM.mov

Expected behavior

Receive a blank body of resource

Steps to reproduce

  1. Install Prestashop 1.7.8.0 with PHP 7.4
  2. Make GET call to an PrestaShop api : api/products?schema=blank
  3. Disable the debug mode
  4. See error

PrestaShop version(s) where the bug happened

1.7.8.0

PHP version(s) where the bug happened

7.4.2

If your bug is related to a module, specify its name and its version

No response

@Olimbot Olimbot added the Bug Type: Bug label Nov 9, 2021
@florine2623
Copy link
Contributor

Hello @Olimbot ,

I have the same result in PS 1.7.8.0, with PHP 7.3 or PHP 7.4. No error is shown.

Here's the result to http://ps178test:8888/api/products?schema=blank :
Screenshot 2021-11-09 at 16 42 49

In my BO I ticked all GET permissions :
Screenshot 2021-11-09 at 16 44 06

Am I missing something ?
Thanks!

@florine2623 florine2623 added 1.7.8.0 Affects versions NMI Status: issue needs more information Webservice Label: Which BO under menu is concerned labels Nov 9, 2021
@hibatallahAouadni hibatallahAouadni added the WS Category: Web Service label Nov 10, 2021
@Olimbot
Copy link
Author

Olimbot commented Nov 10, 2021

Hello @florine2623 ,
Thanks for your answer. The configuration on BO Prestashop is ok.

I found the pb :
There is on error into the WebserviceOutputBuilder.php (for exemple in line 716) because there is no check if the array is not null and the key exist.
If we have in php.ini :
error_reporting = E_ALL & ~E_DEPRECATED & ~E_STRICT

It doesn't works.

We need to add E_NOTICE :
error_reporting = E_ALL & ~E_NOTICE & ~E_DEPRECATED & ~E_STRICT

And it's works.

@hibatallahAouadni
Copy link
Contributor

hibatallahAouadni commented Nov 11, 2021

Hello @Olimbot

I found the pb :
There is on error into the WebserviceOutputBuilder.php (for exemple in line 716) because there is no check if the array is not > null and the key exist.
If we have in php.ini :
error_reporting = E_ALL & ~E_DEPRECATED & ~E_STRICT

It doesn't works.

We need to add E_NOTICE :
error_reporting = E_ALL & ~E_NOTICE & ~E_DEPRECATED & ~E_STRICT

And it's works.

So, should we close the issue as it was solved or do you suggest adding a test in the WebService to verify the existence of the table?

Thanks!

@Olimbot
Copy link
Author

Olimbot commented Nov 16, 2021

Hello @hibatallahAouadni ,
I think if it's possible, it's better to update webservice and for all schema, check if the array is empty or not.

Regards,
Olivier

@hibatallahAouadni
Copy link
Contributor

Hello @Olimbot

Thanks for your suggestion!

it's better to update webservice and for all schema, check if the array is empty or not.

Ping @PrestaShop/prestashop-core-developers wdyt?
Thanks 🙏

@hibatallahAouadni hibatallahAouadni added Needs Specs Status: issue needs to be specified Waiting for dev Status: action required, waiting for tech feedback Improvement Type: Improvement and removed NMI Status: issue needs more information Bug Type: Bug labels Nov 16, 2021
@atomiix
Copy link
Contributor

atomiix commented Nov 18, 2021

Hey @hibatallahAouadni I agree with @Olimbot, hiding errors is not the solution, we should fix this ;)

@hibatallahAouadni
Copy link
Contributor

Well then, changing the issue's status to Ready 😉
Thanks @atomiix 🙏

@hibatallahAouadni hibatallahAouadni added Ready Status: Issue is ready to be worked on and removed Needs Specs Status: issue needs to be specified Waiting for dev Status: action required, waiting for tech feedback labels Nov 18, 2021
@Progi1984
Copy link
Contributor

@hibatallahAouadni Isn't a regression if the behavior works in previous version and not in PHP 7.4/ PS 1780 ?

@hibatallahAouadni
Copy link
Contributor

Hello @Progi1984

We didn't reproduce the issue neither with 1.7.7.8 nor 1.7.8.0.
But @Olimbot proposed to update webservice and for all schema and to check if the array is empty or not.
So, it's not a regression but an improvement 😉

Thanks!

@marionf marionf added Feature Type: New Feature and removed Improvement Type: Improvement labels Dec 28, 2021
@JanMiddelkoop
Copy link

I ran into this issue today with webservices calls to a client's website being absolutely broken. This should probably be fixed with some urgency; it's a really quick fix and I've seen several issues that have this problem: #26568 #19305

If you run into this, replace in classes/webservice/WebserviceOutputBuilder.php:

line 716: $field['value'] = $object_assoc['id'];
replace: $field['value'] = isset($object_assoc['id']) ? $object_assoc['id'] : '';

line 719: $field['value'] = $object_assoc[$field_name];
replace: $field['value'] = isset($object_assoc[$field_name]) ? $object_assoc[$field_name] : '';

Of course, unless someone at PrestaShop cares about this issue and actually fixes the code, this will be broken again once the file is overwritten on update.

@Daaaaad
Copy link

Daaaaad commented Feb 4, 2022

Hello Everyone, I ran into the same issue today... but only when debug mode disabled (PS_MODE_DEV = false).

Could you test again? I think a fix is needed...

@JanMiddelkoop
Copy link

The underlying problem is that the webservice treats E_NOTICE errors generated by PHP the same as E_ERROR and E_WARNING errors; it outputs them via the API to the webservice client. This means that if any theme or module, or in this case the PrestaShop core itself, generates a notice error, the PrestaShop API is effectively broken. The API can work fine, but treats the notice as a serious error and fails the request because of it.

@Daaaaad
Copy link

Daaaaad commented Feb 8, 2022

@JanMiddelkoop Any idea on how to prevent such message to show without changing server configuration?

@JanMiddelkoop
Copy link

@Daaaaad Actually, the webserver is configured to not display any errors to users (display_errors = off) and report all errors to the PHP error log instead. One could argue that PrestaShop is actually overriding the server configuration with it's own bad behaviour.

The notice errors are avoidable and should probably be fixed - but the real problem is the webservice actually sending internal notice errors to a client when it shouldn't. It's bad design and could even leak sensitive security information that can be present in notice errors.

This should be treated as a security issue and should be fixed as soon as possible.

@Daaaaad
Copy link

Daaaaad commented Feb 9, 2022

@JanMiddelkoop If you look at the code, PrestaShop is overriding error_reporting behavior only if PS_MODE_DEV is enabled... which should not be the case in production 😉

@JanMiddelkoop
Copy link

@Daaaaad One can have error reporting set to E_ALL in production. I do, because I want to see notice errors. It helps me debug issues. I want to see them in the error logs though, I don't want them to end up in output. Basically, PrestaShop fails to respect the "display_errors" PHP directive for webservice clients.

@hibatallahAouadni hibatallahAouadni added Bug Type: Bug Minor Severity: minor bug > https://build.prestashop.com/news/severity-classification Regression Type: regression and removed Feature Type: New Feature labels Apr 20, 2022
@hibatallahAouadni hibatallahAouadni added this to Regression issues 1.7.8.x in Backlog Patch via automation Apr 20, 2022
@hibatallahAouadni hibatallahAouadni added the Verified The issue has been reproduced label Apr 20, 2022
@prestonBot prestonBot added the PR available Solution: issue is being addressed label Apr 20, 2022
@hibatallahAouadni hibatallahAouadni changed the title [webservice][1.7.8] Cannot receive blank schemas for resources [WS][1.7.8] Cannot receive blank schemas for resources Apr 20, 2022
@MatShir MatShir added this to Not Ready in PrestaShop 1.7.8.x via automation Apr 25, 2022
@MatShir MatShir removed this from Regression issues 1.7.8.x in Backlog Patch Apr 25, 2022
@MatShir MatShir moved this from Not Ready to To be tested in PrestaShop 1.7.8.x Apr 25, 2022
@hibatallahAouadni
Copy link
Contributor

Hello everyone,

Sorry for the late response but the issue is not a regression, cause I reproduce it on PS1777, see the attached screen record:

image

As you can see there's no CDATA but all tags are empty, it seems with PS1780, we add the CDATA but we didn't check if it's empty or no 😓
So, changing the labels 😅
Thanks @preoteasa for your PR 🚀

Thanks!

@hibatallahAouadni hibatallahAouadni removed the Regression Type: regression label May 13, 2022
@hibatallahAouadni hibatallahAouadni removed this from To be tested in PrestaShop 1.7.8.x May 13, 2022
@hibatallahAouadni hibatallahAouadni added 1.7.7.7 Affects versions and removed 1.7.8.0 Affects versions labels May 13, 2022
@prestashop-issue-bot prestashop-issue-bot bot removed the Ready Status: Issue is ready to be worked on label May 17, 2022
@hibatallahAouadni hibatallahAouadni added the Fixed Resolution: issue closed because fixed label May 17, 2022
@Progi1984 Progi1984 added this to the 8.0.0 milestone May 18, 2022
@hipernet
Copy link

hipernet commented Feb 3, 2023

[PHP Notice #8] Trying to access array offset on value of type null (/var/www/clients/client2/web13/web/classes/webservice/WebserviceOutputBuilder.php, line 716)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.7.7.7 Affects versions Bug Type: Bug Fixed Resolution: issue closed because fixed Minor Severity: minor bug > https://build.prestashop.com/news/severity-classification PR available Solution: issue is being addressed Verified The issue has been reproduced Webservice Label: Which BO under menu is concerned WS Category: Web Service
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants