Closed tux-rampage closed 1 year ago
Hey @tux-rampage 👋🏻
Thanks for the contribution 👍🏻 I'm unfamiliar with PHP and Symfony (any PHP framework), but I'm open to proposals. As far as I read in the docs, I guessed that feature works like the same in Symfony, but...
Additionally this introduces a Security issue by making all files in the current working directory accessible.
RR is not a web-server; it's an application server. It should not be accessible from the internet, thus technically, you can't access the local files, only paths filtered by the LB/web-server/etc.
It would be nice if you share some docs about how this feature should work; it might also be your proposals on how you see this feature. Thanks ⚡
Hi @rustatian
thanks for your reply. I really like the idea behind roadrunner and in fact I've been looking for similar solutions within the PHP ecosystem. This one here seems the most promising.
I'm sure you already aware of the issue, just to make sure we have a common understanding of the topic and please correct me for the parts where I may be misguided or plainly wrong:
PHP has a huge disadvantage when it comes to large data transfers (i.e. applications that cause a lot of or slow i/o): It is Blocking by default. The problem is also outlined quite well in #923 - The worker cannot be re-used while it is not only processing the request (as by occupying CPU cycles) but also while sending the response. For small chunks of data this mostly unnoticed, but it becomes a limiting factor for larger or slow transfers (large responses, db i/o with high latency, etc...).
This can only be fully addressed by using an async framework like Amp or ReactPHP. But this would kick PSR-7 and especially PSR-15 off the table, since both are designed for the traditional blocking nature of PHP.
This whole Blocking-Issue is mitigated by various strategies of the existing SAPIs (Apache mod_php or pfp-fpm) like forking new processes for each request when required.
X-Sendfile is another Strategy here. Instead of opening a large file in php and streaming it to the http frontend which would block the whole Tread in the meantime, the PHP process Responds with this header and delegates sending this (Local!) File to the Application server which makes the Thread available for the next request. X-Sendfile is not a php framework specific implementation, but a http server strategy used by some.
Nginx has a similar option which is called X-Accel-Redirect
. Instead of a Local File-Path, nginx requires this header to reference a resource of a internal location config, which would not only allow delegating local files, but remote resources as well (like files from an Amazon S3, Google Cloud Storage or the like).
The use case for "Why would you open/send the file in php and not address it directly?" is mostly the following: The file should be guarded by the php application and only be sent to the user/client when certain conditions of a business logic are met. That could be a check if the user is authenticated, has proper privileges, has provided a required payment, etc ...
So the traditional flow of the X-Sendfile feature is as follows:
X-Sendfile
response header (addressed as localfile
here)X-Sendfile
header is removed before sending the response headers to the clientlocalfile
and stream its content to the client after sending the response headersContent-Length
or Transfer-Encoding
response headers as neededContent-Length
or Transfer-Encoding
response headers from the PHP response when streaming the contents of localfile
Range
request header and stream localfile
accordingly. Necessary Response headers should be added.Is this even possible with the current HTTP implementation of roadrunner? This is also just a compatibility feature to mitigate effects. #923 - as far as I understood - has way better approaches to handle these concerns and maybe X-Sendfile could also be implemented this way.
Thanks for such a detailed reply 👍🏻
I thought that some outside system should send the X-Sendfile
, but now I understand your worries 😃
This definitely should be changed to work in an 'expected' way.
Is this even possible with the current HTTP implementation of roadrunner?
Yeah, sure.
I'll put this bug in the v2.12.2
milestone.
No duplicates 🥲.
What happened?
The advertised feature does not work as it is used in frameworks like Symfony.
The following worker should cause the webserver (roadrunner) to stream the file
/some/local/file/to/be/sent.mp4
from the given location in theX-Sendfile
response header.With the PSR response created in the above code, apache's
mod_xsendfile
would remove the X-Sendfile header and the potential response body from the php response and stream the local file that is referenced by this header to the client.Roadrunner just passes the (empty) response as is, including the
X-Sendfile
header.As far as I checked the YouTube video to this feature and the go code - and correct me when I'm wrong here, it seems that Roadrunner expects
X-Sendfile
to be passed by the user agent as request header which is not what frameworks like Symfony expect and it is not how X-Sendfile (or X-Accel-Redirect for nginx) works. Additionally this introduces a Security issue by making all files in the current working directory accessible.Version (rr --version)
rr version 2.12.1 (build time: 2022-12-01T12:41:56+0000, go1.19.3), OS: linux, arch: amd64
How to reproduce the issue?
Configure Roadrunner as described in the docs: https://roadrunner.dev/docs/middleware-sendfile/2.x/en And use the worker code above. Make sure the File used for
X-Sendfile
in PHP exists and is readable.Relevant log output
No response