CodeIgniter
CodeIgniter copied to clipboard
bcit-ci
fix: cgi-fcgi status code
Hi :wave:
I work at Datadog, and while doing some tests with CodeIgniter 3, I stumbled upon what I found to be strange behavior when using the cgi-fcgi sapi. I want to understand whether this is a bug or a feature, or if there is an existing workaround. I mostly want to foster discussion and understand :)
Consider this controller:
<?php
class Foo extends CI_Controller {
function index() {
throw new \Exception('an exception message');
}
}
When accessing the /foo endpoint, I expect a 500 status code: This is indeed the case in the response header of a curl request, for instance.
However, this is not the case after the exception handler has been called. For instance, if you add the following to error_exception.php, a 200 status code will be displayed:
<p>Status Code: <?php echo http_response_code(); ?></p>
Forcing the status code during the call to header effectively solves this issue, if this is one.
You may ask why do we care? Basically, currently, it would appear that when using CGI SAPIs, requests that should be marked as 500s are marked as 200s on Datadog's UI :(
Thank you for your help! 😃
I think so too. The code has been that way forever and underneath in other lines it does set the code. For what reason this was omitted for CGI, I have no idea.
That 15-year-old code was intended for CGI. https://github.com/bcit-ci/CodeIgniter/commit/817163a1bcff02285f763bcf72ff02e86f218cf8#diff-bb769d1a64c842b43be080746afeb59ac119e1cf74fe9ff41ca5310dc18e587dR327
The line returns Status: {$code} {$text}, and $code means an HTTP status code.
Therefore if the HTTP response code is not $code, it is a bug, or just the cgi-fcgi was not considered at the time of writing.
The Status header field contains a 3-digit integer result code that indicates the level of success of the script's attempt to handle the request. https://datatracker.ietf.org/doc/html/rfc3875#section-6.3.3
https://github.com/bcit-ci/CodeIgniter/blob/817163a1bcff02285f763bcf72ff02e86f218cf8/system/codeigniter/Common.php#L323-L336
That 15-year-old code was intended for CGI.
Yes, but why was the response code omitted from the response specifically for CGI? It looks like it's intentional given the other lines that do set it.
I'm not confirmed, but it seems if the status code is set to header(), a server error occurs with CGI.
So it is safe that adding code only for cgi-fcgi. > @PROFeNoM
Source: https://qiita.com/Enchan/items/a95e6cc3a651aee7c936#%E7%99%BA%E7%94%9F%E5%8E%9F%E5%9B%A0%E3%81%A8%E5%AF%BE%E7%AD%96
<?php
// (<?phpã®åã«ä»»æã®æååããªãããã«ãã¦ãã ãã)
header('HTTP', true, 400);
?>
<html><body>
<h1>400 Bad Request</h1>
<p>This page is not working properly.</p>
<p>If this issue occur repeatedly, please contact administrator.</p>
</body></html>
HTTP/1.1 500 Internal Server Error
Server: nginx
Date: Wed, 28 Oct 2020 09:41:38 GMT
Content-Type: text/html; charset=iso-8859-1
Content-Length: 531
Connection: keep-alive
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>500 Internal Server Error</title>
</head><body>
<h1>Internal Server Error</h1>
<p>The server encountered an internal error or
misconfiguration and was unable to complete
your request.</p>
<p>Please contact the server administrator at
[no address given] to inform them of the time this error occurred,
and the actions you performed just before this error.</p>
<p>More information about this error may be available
in the server error log.</p>
</body></html>
So, probably the correct fix is on the if condition if (PHP_SAPI === "cgi") or something
Yes, but why was the response code omitted from the response specifically for CGI? It looks like it's intentional given the other lines that do set it.
After a quick inspection at RFC 3875, it looks like there's not response code except within the Status header.
Hey @kenjis :wave: Thanks for approving 😃 Do you think this PR will get merged, and if so, do you have an ETA?
@PROFeNoM Sorry, I'm not a maintainer. So I don't know.
@gxgpet Could you merge this?