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

nginx X-Accel-Redirect requires unquoted URLs. #56

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

stuxcrystal
Copy link

As the title says, nginx X-Accel-Redirect expects an unquoted string.

Therefore quoting the URL in the nginx backend will cause internal 404s.

@willstott101
Copy link

willstott101 commented Nov 22, 2016

I've been having this issue myself too. It was added this way to support unicode characters in the URIs in #54. For now I am using django-sendfile==0.3.10 to workaround the issue for characters such as ! but Unicode characters will raise an exception when Django tries to write the headers.

I'm wondering what nginx and django version, decoded the URI correctly for you @Proper-Job , when you wrote the patch, if you don't mind helping out?

My Django is 1.8.16 and my nginx is 1.4.6

@Proper-Job
Copy link
Contributor

@willstott101 @stuxcrystal I have 0.3.11 deployed on nginx 1.6.2 and django 1.8.14 with unicode and non-unicode filenames without any 404 exceptions.
Does downgrading to 0.3.10 fix your issue without touching the server config?

@willstott101
Copy link

willstott101 commented Nov 22, 2016

Thanks @Proper-Job

0.3.10:
Ascii characters such as ! are served with sendfile fine.
Unicode characters such as Б fail with a UnicodeEncodeError when gunicorn writes the request to the socket (just noticed this, I assumed it was django thing before).

0.3.11:
Ascii and unicode characters 404, and from the nginx log it looks like it's looking for a file with the literal, escaped URL. Such as /var/opt/.../hmm/%21.png

I'll see if I can test with a newer version of nginx. This is all on my development machine, Ubuntu 14.04

@willstott101
Copy link

willstott101 commented Nov 22, 2016

I switched to a PPA and installed nginx 1.11.5 and it's working flawlessly with sendfile 0.3.11, so it appears the version of nginx available on the official ubuntu trusty repos is too old to decode the header.

And this page: http://nginx.org/en/CHANGES seems to have the change pinned to version 1.5.9 Ctrl+F: escaped URIs in "X-Accel-Redirect"

As to what should be done by this library (if anything) to support older versions of nginx is a different issue. Maybe as a setting? But that smells a bit to me.

@Proper-Job
Copy link
Contributor

@willstott101 nice digging!

adnrs96 added a commit to adnrs96/django-sendfile that referenced this pull request Apr 12, 2017
In this commit we are adding support for Nginx version older than
1.5.9. This is achieved through conditionally quoting the URL's in
X-Accel-Redirect headers according to compatibility with Nginx.
Since newer versions of Nginx expect URL's in X-Accel-Redirect to
be quoted unlike the versions older that Nginx 1.5.9. In this commit
we start to check the Nginx version using 'nginx -v' and then caching
the result be making use of a decorator. Using this result we decide
whether the outgoing URL should be quoted or not.
Fixes: johnsensible#56, johnsensible#58. Closes johnsensible#45.
adnrs96 added a commit to adnrs96/django-sendfile that referenced this pull request Apr 12, 2017
In this commit we are adding support for Nginx version older than
1.5.9. This is achieved through conditionally quoting the URL's in
X-Accel-Redirect headers according to compatibility with Nginx.
Since newer versions of Nginx expect URL's in X-Accel-Redirect to
be quoted unlike the versions older that Nginx 1.5.9. In this commit
we start to check the Nginx version using 'nginx -v' and then caching
the result be making use of a decorator. Using this result we decide
whether the outgoing URL should be quoted or not.
Fixes: johnsensible#56, johnsensible#58. Closes johnsensible#45.
adnrs96 added a commit to adnrs96/django-sendfile that referenced this pull request Apr 13, 2017
In this commit we are adding support for Nginx version older than
1.5.9. This is achieved through conditionally quoting the URL's in
X-Accel-Redirect headers according to compatibility with Nginx.
Since newer versions of Nginx expect URL's in X-Accel-Redirect to
be quoted unlike the versions older that Nginx 1.5.9. From this commit
onwards we start to expect a 'NGINX_VERSION' config setting to be setup
in the django settings in order to facilitate decision making regarding
quoting URL's. If such a setting is not found we just assume Nginx version
to be greater then 1.5.9 and make decision accordingly and then cache the
decision by making use of a decorator. Using this result we decide whether
the outgoing URL should be quoted or not.
Fixes: johnsensible#56, johnsensible#58. Closes johnsensible#45.
adnrs96 added a commit to adnrs96/django-sendfile that referenced this pull request Apr 13, 2017
In this commit we are adding support for Nginx version older than
1.5.9. This is achieved through conditionally quoting the URL's in
X-Accel-Redirect headers according to compatibility with Nginx.
Since newer versions of Nginx expect URL's in X-Accel-Redirect to
be quoted unlike the versions older that Nginx 1.5.9. From this commit
onwards we start to expect a 'NGINX_VERSION' config setting to be setup
in the django settings in order to facilitate decision making regarding
quoting URL's. If such a setting is not found we just assume Nginx version
to be greater then 1.5.9 and make decision accordingly and then cache the
decision by making use of a decorator. Using this result we decide whether
the outgoing URL should be quoted or not.
Fixes: johnsensible#56, johnsensible#58. Closes johnsensible#45.
adnrs96 added a commit to adnrs96/django-sendfile that referenced this pull request Apr 13, 2017
In this commit we are adding support for Nginx version older than
1.5.9. This is achieved through conditionally quoting the URL's in
X-Accel-Redirect headers according to compatibility with Nginx.
Since newer versions of Nginx expect URL's in X-Accel-Redirect to
be quoted unlike the versions older that Nginx 1.5.9. From this commit
onwards we start to expect a 'NGINX_VERSION' config setting to be setup
in the django settings in order to facilitate decision making regarding
quoting URL's. If such a setting is not found we just assume Nginx version
to be greater then 1.5.9 and make decision accordingly and then cache the
decision by making use of a decorator. Using this result we decide whether
the outgoing URL should be quoted or not.
Fixes: johnsensible#56, johnsensible#58. Closes johnsensible#45.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants