[#6368] - Fix nginx 502 Bad gateway because of invalid http status codes
- Closed
- 10 Aug 2015
- Medium
- Build: staging
- # 6368
- Diff
- Mech7:fix-nginx-status
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
Nginx does not allow an invalid status code, in some exception there is no http status code used and it will return an 502 bad gateway.
It will solve many of the problems found related with this: https://encrypted.google.com/search?hl=en&q=nginx%20502%20joomla
joomla-cms-bot
- | Labels |
Added:
?
|
||
Can you please provide some test instructions. What can I do to generate an error code < 400 or > 599)
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6368.
@brianteeman Anywhere in a component or module (probably anywhere in the application) throw an exception without a valid http code under nginx.
throw new Exception('Exception without code');
Nginx will return a bad gateway as joomla uses the exception code as http status code, currently it only checks if it's between the 400 - 599 range as an exception within the application should return a error http status.
So currently it would only work like this:
throw new Exception('doc not found', 404);
But you don't know where an exception will be thrown and you don't want to put a try / catch everywhere in the code so with this it will default to a 500 error if an exception is thrown somewhere in the code and display the joomla error page as usual.
confirmed on nginx 1.6.2, php-fpm 5.6.6
without patch, i got an 502 Bad Gateway.
with Patch, i got an 500 Error and the Exception was displayed.
test code:
public function onAfterInitialise()
{
throw new RuntimeException('test RuntimeException on onAfterInitialise event', 300);
}so patch looks really god.
The status header needs to be a correct HTTP header instead of the Exception's message. Otherwise, I'd say this is OK.
@mbabker If you mean the reason phrase it should be allowed to set a custom message by the exception http://www.w3.org/Protocols/rfc2616/rfc2616-sec6.html
Also this is how it works currently and could break something if somebody relies on the reason phrase in an api.
@brianteeman Is all ok now? anything holding the merge back?
Anything holding this back? it's been open for a long time.
The status header needs to be a correct HTTP header instead of the Exception's message. Otherwise, I'd say this is OK.
@mbabker so you mean change this:
JFactory::getApplication()->setHeader('status', $status . ' ' . str_replace("\n", ' ', $this->_error->getMessage()));
to
JFactory::getApplication()->setHeader('status', $status);
As the first are the same as bevor?
Or do you mean something like:
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_redirect/models/fields/redirect.php#L31-99
That we get with:
$status = $this->getStatusOnID($status);
and
/**
* A map of integer HTTP 1.1 response codes to the full HTTP Status for the headers.
*
* @var object
* @since 3.4
* @see http://www.iana.org/assignments/http-status-codes/
*/
protected $responseMap = array(
100 => 'HTTP/1.1 100 Continue',
101 => 'HTTP/1.1 101 Switching Protocols',
102 => 'HTTP/1.1 102 Processing',
200 => 'HTTP/1.1 200 OK',
201 => 'HTTP/1.1 201 Created',
202 => 'HTTP/1.1 202 Accepted',
203 => 'HTTP/1.1 203 Non-Authoritative Information',
204 => 'HTTP/1.1 204 No Content',
205 => 'HTTP/1.1 205 Reset Content',
206 => 'HTTP/1.1 206 Partial Content',
207 => 'HTTP/1.1 207 Multi-Status',
208 => 'HTTP/1.1 208 Already Reported',
226 => 'HTTP/1.1 226 IM Used',
300 => 'HTTP/1.1 300 Multiple Choices',
301 => 'HTTP/1.1 301 Moved Permanently',
302 => 'HTTP/1.1 302 Found',
303 => 'HTTP/1.1 303 See other',
304 => 'HTTP/1.1 304 Not Modified',
305 => 'HTTP/1.1 305 Use Proxy',
306 => 'HTTP/1.1 306 (Unused)',
307 => 'HTTP/1.1 307 Temporary Redirect',
308 => 'HTTP/1.1 308 Permanent Redirect',
400 => 'HTTP/1.1 400 Bad Request',
401 => 'HTTP/1.1 401 Unauthorized',
402 => 'HTTP/1.1 402 Payment Required',
403 => 'HTTP/1.1 403 Forbidden',
404 => 'HTTP/1.1 404 Not Found',
405 => 'HTTP/1.1 405 Method Not Allowed',
406 => 'HTTP/1.1 406 Not Acceptable',
407 => 'HTTP/1.1 407 Proxy Authentication Required',
408 => 'HTTP/1.1 408 Request Timeout',
409 => 'HTTP/1.1 409 Conflict',
410 => 'HTTP/1.1 410 Gone',
411 => 'HTTP/1.1 411 Length Required',
412 => 'HTTP/1.1 412 Precondition Failed',
413 => 'HTTP/1.1 413 Payload Too Large',
414 => 'HTTP/1.1 414 URI Too Long',
415 => 'HTTP/1.1 415 Unsupported Media Type',
416 => 'HTTP/1.1 416 Requested Range Not Satisfiable',
417 => 'HTTP/1.1 417 Expectation Failed',
418 => 'HTTP/1.1 418 I\'m a teapot',
422 => 'HTTP/1.1 422 Unprocessable Entity',
423 => 'HTTP/1.1 423 Locked',
424 => 'HTTP/1.1 424 Failed Dependency',
425 => 'HTTP/1.1 425 Reserved for WebDAV advanced collections expired proposal',
426 => 'HTTP/1.1 426 Upgrade Required',
428 => 'HTTP/1.1 428 Precondition Required',
429 => 'HTTP/1.1 429 Too Many Requests',
431 => 'HTTP/1.1 431 Request Header Fields Too Large',
500 => 'HTTP/1.1 500 Internal Server Error',
501 => 'HTTP/1.1 501 Not Implemented',
502 => 'HTTP/1.1 502 Bad Gateway',
503 => 'HTTP/1.1 503 Service Unavailable',
504 => 'HTTP/1.1 504 Gateway Timeout',
505 => 'HTTP/1.1 505 HTTP Version Not Supported',
506 => 'HTTP/1.1 506 Variant Also Negotiates (Experimental)',
507 => 'HTTP/1.1 507 Insufficient Storage',
508 => 'HTTP/1.1 508 Loop Detected',
510 => 'HTTP/1.1 510 Not Extended',
511 => 'HTTP/1.1 511 Network Authentication Required',
);
protected function getStatusOnID(int $status)
{
if (isset($responseMap[$status]))
{
return $responseMap[$status];
}
return $responseMap[500];
}The correct code for the set the header would than be:
$status = $this->getStatusOnID($status);
JFactory::getApplication()->setHeader('status', $status);Or can we leave it as it is now?
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-08-10 13:46:41 |
| Closed_By | ⇒ | zero-24 |
FILE: ...ravis/build/joomla/joomla-cms/libraries/joomla/document/error/error.php
FOUND 5 ERROR(S) AFFECTING 4 LINE(S)
88 | ERROR | Tabs must be used to indent lines; spaces are not allowed
88 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found "if (...)
| | ...{...}\n...else\n"
89 | ERROR | Tabs must be used to indent lines; spaces are not allowed
90 | ERROR | Tabs must be used to indent lines; spaces are not allowed
91 | ERROR | Concat operator must be preceeded by one space