klein / klein.php

A fast & flexible router
MIT License
2.66k stars 290 forks source link

Bugfix: File Response Problems #335

Closed mcos closed 8 years ago

mcos commented 8 years ago

This was a fun one to debug. I've been using Response::file() to try to return a file. In this case, I've been using both application/zip and application/vnd.openxmlformats-officedocument.spreadsheetml.sheet Content-Type headers.

When calling this method, the response would hang until the connection timed out, even thought the correctly formatted file was present in the file system of the server.

It turned out that the issue was caused by the call to AbstractResponse::send() within Response::file(), due to the behavior of the call to fastcgi_finish_request from AbstractResponse::send(). It turns out that finishing the request caused the readfile() call to hang, moving the fastcgi_finish_request logic into the Response::file() method fixed the issue.

I also added a conditional to the Response::file() method to not send the Content-Length header when the Transfer-Encoding header is set to chunked, this is in order to properly comply with RFC 2616.

I couldn't come up with an appropriate unit test for this fix, but I'm open to suggestions. I've thoroughly tested this with integration tests and various other debugging strategies.

Rican7 commented 8 years ago

Wait a minute! I have an idea!

Maybe you could try changing the Content-length header value setting to instead be additive with whatever may or may not be in the output buffer via ob_get_length(), something like this, maybe:

$this->header('Content-length', ob_get_length() + filesize($path));
mcos commented 8 years ago

Agh. So I tried the solution you thought of (and it was a good one), however it didn't work (kept timing out), so the problem may not actually be the one that I'm thinking of. This is really kinda weird. I can definitely track the problem to the setting of headers. I'm going to have to debug this one further.

Rican7 commented 8 years ago

Hmm, interesting. Ok, let me know what you find out! šŸ˜ƒ

Rican7 commented 8 years ago

Really great job here, @mcos! Thanks for the sensible fix! šŸ‘