Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
aleho opened this issue Dec 11, 2023 · 5 comments · Fixed by caddyserver/caddy#6232
Closed

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

aleho opened this issue Dec 11, 2023 · 5 comments · Fixed by caddyserver/caddy#6232
Labels
enhancement New feature or request

Comments

@aleho
Copy link

aleho commented Dec 11, 2023

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
Copy link
Collaborator

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
Copy link
Author

aleho commented Dec 13, 2023

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
Copy link
Collaborator

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
Copy link
Author

aleho commented Dec 13, 2023

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 dunglas added the enhancement New feature or request label Dec 14, 2023
@dunglas
Copy link
Owner

dunglas commented Dec 14, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants