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

Add support for Nginx "X-Accel-Redirect" header to send_file #72

Closed
stevvooe opened this issue Jun 28, 2010 · 12 comments
Closed

Add support for Nginx "X-Accel-Redirect" header to send_file #72

stevvooe opened this issue Jun 28, 2010 · 12 comments

Comments

@stevvooe
Copy link

The information for nginx's send_file support is here:

http://wiki.nginx.org/NginxXSendfile

This might be the job of middleware (have the middleware hijack the X-Sendfile header and modify it for nginx), but it would be nice to support this natively.

@mitsuhiko
Copy link
Contributor

Nginx does not support XSendfile and X-Accel-Redirect works differently. I cannot support that from Flask because it requires a very specific setting in the server config that Flask cannot probe for. This is something a middlware would have to translate.

@stevvooe
Copy link
Author

Isn't the interface the same? The only base difference is the header name "X-Sendfile" vs. "X-Accel-Redirect". You could just make sure the header name changes based on a config value.

In addition, I don't think flask needs to probe for this specifically for nginx, especially when it wouldn't do so for lighttpd or apache.

@mitsuhiko
Copy link
Contributor

Isn't the interface the same?

No. X-Sendfile takes a filename to a file on a filesystem, X-Accel-Redirect takes a URI that might point to a resource set up in the nginx config to only accept internal requests.

@stevvooe
Copy link
Author

Are the semantics of the field important to Flask? Wouldn't this detail only be important to the proxy?

Under the current implementation, won't there be problems if flask is on a different system from apache or lighttpd and the file doesn't exist on the server? This check joins the filename with the app path:

http://github.com/mitsuhiko/flask/blob/master/flask.py#L375

@mitsuhiko
Copy link
Contributor

How should that work? It does not accept actual filenames. I can just point to non-remote URIs there and the resources is not web available unless the nginx config is modified. So there is no chance that Flask could generically support it.

If you have any ideas how to implement that, let me know.

@stevvooe
Copy link
Author

I think the distinction you're missing is that X-Sendfile doesn't take filenames that are on the Flask machine; it takes filenames that are on the apache/lighttpd machine. The os.path.join check assumes that the file is available on both machines.

Flask should neither check nor assume that the file system used by apache or lighttpd are identical.

@mitsuhiko
Copy link
Contributor

I am well aware of the fact that this is assuming the same machine. That however is not the problem I'm referring to. You can already emit X-Accel-Redirect headers yourself, you just cannot convert the filename even closely to a mapped URI for nginx.

@stevvooe
Copy link
Author

O ok, so the feature is only meant to work on the same machine?

I wouldn't expect to make Flask responsible for generating the mapped uri for X-Accel-Redirect, so why would you expect it to resolve a filename for X-Sendfile?

@mitsuhiko
Copy link
Contributor

The feature works perfectly fine with filenames from other machines if you provide an absolute filename. Furthermore it also works flawless if you use an NFS share.

But how should I emit an X-Accel-Redirect header if I don't know the location?

@mitsuhiko
Copy link
Contributor

Again: if you have suggestions how that could work, I would be happy to support it. I just don't see a way.

@stevvooe
Copy link
Author

The feature works perfectly fine with filenames from other machines if you provide an absolute filename. Furthermore it also works flawless if you use an NFS share.

Agreed. This send_file is doing quite a bit more than what I understood was necessary for X-Sendfile.

I am going to implement a nearly identical function called nginx_send_file to explore these issues further and see if I can provide a patch or a better suggestion (maybe a slick middleware that can support both features).

BTW, great work on flask. Its really close to being a solid framework.

@tonyseek
Copy link

tonyseek commented Oct 8, 2014

@mitsuhiko Could we provide different configuration items such as X_USE_ACCEL and X_ACCEL_LOCATION_BASE just like the behavior of this WSGI middleware?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants