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

PHP warnings cause the response to be a mess consisting of a partial JSON and an error JSON #327

Open
sschultze opened this issue Jul 13, 2023 · 1 comment

Comments

@sschultze
Copy link

sschultze commented Jul 13, 2023

Steps to reproduce

  • Create a new project with the slim skeleton
  • Return a big JSON from an action (return $this->respondWithData($bigJson))
  • Before returning the JSON, cause a PHP warning (e.g. by accessing a field from an associative array that doesn't exist)

What happens?

The server returns the action response partially, and then continues with the error JSON. So you get an invalid mess.

What would be expected?

The server returns the action response entirely. -or- The server returns the error JSON entirely.

Causes of the bug

When debugging I found out that the following happens:

  • The action is executed, and its response is written with the ResponseEmitter.
  • The shutdown handler runs.
  • error_get_last() returns something, so a new error response is created
  • The newly created ResponseEmitter tries to clean the output buffer with ob_clean(), but only the not yet flushed part of the big JSON is cleaned. What has been flushed (the first part of the big JSON) cannot be un-flushed.
  • The error JSON is appended.

Workaround 1

In public/index.php, register a PHP error handler with set_error_handler which throws an exception.

Workaround 2

Create a global flag which allows the reponse emitter to check if a response has already been written.

Disclaimer

I am new to PHP (only used it many many years ago) - so I might have misunderstood something.

@ingluisjimenez
Copy link

I would say the shutdown error handler should be updated so that you can configure what type of errors it should handle, as right now, it treats warnings and notices as errors and those would break the response or send an error response when you are sending a 200.

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

2 participants