dunglas / frankenphp

🧟 The modern PHP app server
https://frankenphp.dev
MIT License
6.89k stars 236 forks source link

Intercept responses for nginx-like X-Accel-Redirect handling #365

Closed aleho closed 6 months ago

aleho commented 11 months ago

Caddy allows the responses in reverse_proxy to be intercepted via handle_response like so:

php_fastcgi unix//run/php/fpm.sock {
    # FPM needs full application root
    root /application/public

    @accel header X-Accel-Redirect *
    handle_response @accel {
        root * /files
        rewrite * {rp.header.X-Accel-Redirect}
        file_server
    }
}

This allows us to send a "private" file from backend that is available locally without having to stream it through PHP.

I haven't found it in the docs (but maybe didn't look carefully enough), can this be done in FrankenPHP?

withinboredom commented 11 months ago

That is a really good suggestion. I've never used this feature but I'm aware of it. I guess the question is, should this be enabled by default or require extra configuration like the fastcgi plugin?

I'd be more than happy to pick this one up @dunglas.

aleho commented 11 months ago

I myself am not a fan of implicitly copying Nginx specifiy syntax (and I don't think any of the other X-Accel stuff matters for or even applies to Caddy/FrankenPHP). X-Accel-Redirect being the exception, maybe something configurable might do the trick?

A repeatable option with a simple combination of header_name and file_lookup_root should be enough, or do you see anything else that would be required?

Like:

php_server {
    handle_file_response X-Accel-Redirect /app/var/storage/files
    handle_file_response X-File-Response /files
}

Of course, If-None-Match or If-Not-Modified might be a good thing to support too. My knowledge of Caddy internals is very limited, maybe that's already implicitly handled by Caddy if you tell it to serve a file?

withinboredom commented 11 months ago

I wonder if we even need headers. Maybe something a function like this can be called from application code:

function frankenphp_serve_file(string $path, array $additionalHeaders = []): never {}

The downside to something like this is that it won't be compatible with most libraries that take advantage of X-Accel-Redirect.

aleho commented 11 months ago

The downside to something like this is that it won't be compatible with most libraries that take advantage of X-Accel-Redirect.

Yes, exactly my opinion too. I experimented with FrankenPHP yesterday and found out that we can easily use it as a drop-in replacement (we're already using Caddy too), so I'd always vote for compatibility with existing setups.

I understand that using headers to tell the downstream what to do with a response is somewhat strange design but very flexible as well (see caches).

Keeping X-Accel-Redirect in applications allows the Caddy setup to either use it or even pass it to the next downstream that might be a reverse-proxy doing load-balancing and streaming files from an S3 storage.

dunglas commented 11 months ago

Symfony HttpFoundation (so also Laravel, Drupal etc) have native support for X-Accel-Redirect: https://github.com/symfony/symfony/blob/7.0/src/Symfony/Component/HttpFoundation/BinaryFileResponse.php

So I'll go the header way too, this will allow all these existing apps to leverage this feature.