Don't send errors as HTML if request looks like a json request #15357
Conversation
| */ | ||
| public function isJSONRequest() { | ||
| if (stristr($_SERVER['HTTP_X_REQUEST_WITH'], 'XMLHttpRequest') | ||
| || stristr($_SERVER['HTTP_ACCEPT'],'application/json') |
There was a problem hiding this comment.
Should use this->server['HTTP_ACCEPT]
butonic
force-pushed
the
dont_send_errors_as_html_if_request_looks_like_a_json_request_master
branch
from
46a6424
to
15d7825
Compare
| @@ -44,14 +44,20 @@ | |||
|
|
|||
| //show the user a detailed error page | |||
| OC_Response::setStatus(OC_Response::STATUS_SERVICE_UNAVAILABLE); | |||
| OC_Template::printExceptionErrorPage($ex); | |||
| if (\OC::$server->getRequest()->isJSONRequest()) { | |||
There was a problem hiding this comment.
what about moving this check inside printExceptionErrorPage() adn even send back the error message as json?
There was a problem hiding this comment.
I know this looks ugly. But as a developer you expect printExceptionErrorPage() to output something ... it starts with print.
There was a problem hiding this comment.
it starts with print.
in case of json request it will print out json - on cli it will print out plain text - html for all the rest
|
The inspection completed: 2 new issues, 1 updated code elements |
|
Sometimes I have the feeling that showing an exception page is really a corner case that happens only at setup time or if something is wrong with the setup. |
|
I'd like to have a solution which somewhat merges with this last-minute-8.1-hack - https://github.com/owncloud/core/blob/master/remote.php#L43 |
|
Any update for this ? |
|
No activity -> 9.0 |
DeepDiver1975
force-pushed
the
dont_send_errors_as_html_if_request_looks_like_a_json_request_master
branch
from
15d7825
to
5e24d29
Compare
| } catch (Exception $ex) { | ||
| \OCP\Util::logException('index', $ex); | ||
|
|
||
| //show the user a detailed error page | ||
| OC_Response::setStatus(OC_Response::STATUS_INTERNAL_SERVER_ERROR); | ||
| OC_Template::printExceptionErrorPage($ex); | ||
| if (\OC::$server->getRequest()->isJSONRequest()) { |
There was a problem hiding this comment.
this should be inverted - right?
If it's NOT a json request the exception page is displayed
|
Fixes #10134 |
|
We're past feature freeze, changing base.php or base interfaces can be dangerous. Moving to 9.1 CC @cmonteroluque |
|
Is this critical enough to push this forward ? Else let's close this and schedule time for a proper discussion about the approach we want here (which we could track in a ticket) |
@butonic agree to close? |
|
lets reopen this one day .... |
DeepDiver1975
deleted the
dont_send_errors_as_html_if_request_looks_like_a_json_request_master
branch
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
starting point for #15332 (comment) feel free to hijack.