johnsensible
backends-nginx: Add support for Nginx versions older than 1.5.9.
In this PR 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.
This will not break Unicode support as revealed from my investigation. I tried out Django-sendfile 0.3.11 with Nginx 1.4.6. This leads to 404's which are resolved when we stop quoting the URL's. It might be worthwhile mentioning after making such a change filenames such as á.txt and Б.txt were still functional and correctly recognised.
Fixes: #56, #58. Closes #45.
Wouldn't it be simpler to have a configuration parameter for this ? Trying to get nginx version with subprocess when you might not be able to run it or not be on the same machine at all (think docker, etc) seems like over-complication.
Indeed it would be simpler @EvaSDK . Having a configuration parameter for this was the second thing in my mind. Didn't thought of Docker use case otherwise it would have become the first. Workflow for that would be something like this:
- Developer has to set NGINX_VERSION parameter in django configuration with other settings like backends.
- If the NGINX_VERSION is found to be set, it is used to decide whether the URL's should be quoted or not. The decision is cached in variable as is done currently.
- If NGINX_VERSION setting is not found to be set, we assume that NGINX_VERSION is greater than 1.5.9 and thus decision is made and cached. Doing this has a benefit that people using django-sendfile currently will not be forced to make any changes on there part if everything is working fine. On other hands developers with older Nginx can use configuration setting to make Django-sendfile work with there project.
If community feels good with this change I can update the PR. Just leave me a comment regarding this.
I figured it would be better to have a updated commit for community to review than just a workflow, so I have updated the commit. Also added instructions in Readme regarding the same.