X-Accel-Redirect

Server NGINX
1

Hi everyone,

I got a server which serves Data to a client ( in my case MP3s ). The
MP3s are saved in a storage directory in the form Artist - Title.mp3

I’m using X-Accel-Redirect to serve the files from the storage dir to
the user. This workes fine for most cases but fails for special chars
like the question mark. I already tried encoding it - but the char
always gets stripped like in Query strings. But since it’s not an url
I’m providing no stripping should take place. Here are my results:

Example Filename is: /ext/53665/Katy Perry - Who Am I Living
For?.mp3

Plain:
open() “/ext/53665/Katy Perry - Who Am I Living For” failed (2: No such
file or directory)

urlencoded:
open()
“/usr/local/nginx/html%2Fdl%2F53665%2FKaty+Perry±+Who+Am+I+Living+For%253F.mp3”
failed (2: No such file or directory)

Rawurlencoded:
open()
“/usr/local/nginx/html%2Fdl%2F53665%2FKaty%20Perry%20-%20Who%20Am%20I%20Living%20For%3F.mp3”
failed (2: No such file or directory)

Replaced ? with ?
open() "/ext/53665/Katy Perry - Who Am I Living For" failed (2: No such
file or directory)

Escaped ? to %3F
open() “/ext/53665/Katy Perry - Who Am I Living For%3F.mp3” failed (2:
No such file or directory)

It seems like X-Accel-Redirect treats the path as url instead of a
realpath - or even doesn’t apply any decoding on the subrequest.

best regards,

Volker

Posted at Nginx Forum:

2

Hi,
I created a small patch for that issue which works for me. But it needs
to be reviewed by Igor or someone who knows C better than me.
It checks the static request from X-Accel-Redirect for ‘%’ and escapes
them if found.

--- nginx-0.8.50orig/src/http/modules/ngx_http_static_module.c
2010-05-24 14:35:10.000000000 +0200
+++ nginx-0.8.50/src/http/modules/ngx_http_static_module.c
2010-09-09 13:49:49.000000000 +0200
@@ -47,7 +47,7 @@
 static ngx_int_t
 ngx_http_static_handler(ngx_http_request_t *r)
 {
-    u_char                    *last, *location;
+    u_char                    *last, *location, *src, *dst;
     size_t                     root, len;
     ngx_str_t                  path;
     ngx_int_t                  rc;
@@ -83,6 +83,28 @@
     ngx_log_debug1(NGX_LOG_DEBUG_HTTP, log, 0,
                    "http filename: \"%s\"", path.data);

+    /*
+     * X-Accel-Redirect Patch
+     * If the path contains a % it probably must be decoded
+     */
+    if( strstr( path.data, "%" ) != NULL )
+    {
+        ngx_str_t *uri = &path;
+
+        dst = uri->data;
+        src = uri->data;
+
+        ngx_unescape_uri( &dst, &src, uri->len, NGX_UNESCAPE_URI );
+
+        len = uri->len - ( src - dst ) + 1;
+        if ( len )
+        {
+            dst = ngx_copy( dst, src, len);
+        }
+        uri->len = dst - uri->data;
+    }
+
+
     clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);

     ngx_memzero(&of, sizeof(ngx_open_file_info_t));

It would be nice to get a little feedback if this is ok.

best regards,

Volker Richter

Posted at Nginx Forum:

3

Hi,
you cannot use the presence of a character as an indicator to determine
if
a string is encoded or not. After all “%” is a perfectly legitimate
character that can be used for a filename or directory.

The question is whether path.data represents the raw encoded version of
the
path in which case it must always be decoded or if this variable
already
represents the decoded version of the path in which case it should
never
be decoded a second time.

Regards,
Dennis

4

If you’d use an url with a filename that contains an ‘%’ the client
should always encode it to ‘%25’ which would get decoded correctly in
this case.

In RFC1738 the ‘%’ is an “Unsafe Character” and should always be encoded
in URIs

The check could be extended to something like %([a-f0-9]{2}) but
directories or files like %ears or %b1blabla would fail that check, too.

Posted at Nginx Forum:

5

Hello!

On Wed, Sep 08, 2010 at 09:53:14AM -0400, rovervr wrote:

Hi everyone,

I got a server which serves Data to a client ( in my case MP3s ). The
MP3s are saved in a storage directory in the form Artist - Title.mp3

I’m using X-Accel-Redirect to serve the files from the storage dir to
the user. This workes fine for most cases but fails for special chars
like the question mark. I already tried encoding it - but the char
always gets stripped like in Query strings. But since it’s not an url
I’m providing no stripping should take place. Here are my results:

X-Accel-Redirects expects URI, but unescaped one. This was
last discussed yesterday:

http://nginx.org/pipermail/nginx/2010-September/022384.html

Quote:

X-Accel-Redirect expected to contain non-encoded URI. This isn’t
really right, but it’s how it currently works.

In particular this makes impossible to (normally) serve resources
with ‘?’ in name as you see in your tests, as anything after ‘?’
is treated as query string.

Correct fix would be to change X-Accel-Redirect to accept escaped
URI instead. I believe patches are welcome.

Maxim D.

6

Hi Maxim,

Don’t get me wrong … but isn’t X-Accel-Redirect for serving static
files without the overhead of php-fpm/fastcgi? Can you provide an
example where X-Accel-Redirect is used for dynamic files?

The patch works for me because I just use nginx to serve download files,
but I’d like to provide a patch that fixes the wrong behaviour of
X-Accel-Redirect for everyone.

Posted at Nginx Forum:

7

Hello!

On Thu, Sep 09, 2010 at 08:03:07AM -0400, rovervr wrote:

I created a small patch for that issue which works for me. But it needs
to be reviewed by Igor or someone who knows C better than me.
It checks the static request from X-Accel-Redirect for ‘%’ and escapes
them if found.

This patch is wrong, it breaks access to normal files with ‘%’.
Additionally, it doesn’t change X-Accel-Redirect behaviour for
non-static files.

Instead X-Accel-Redirect value should be unescaped when it got
from upstream, somewhere before ngx_http_internal_redirect() call.
I personally believe ngx_http_parse_unsafe_uri() should be changed
to unescape uri (note that it will also affect ssi and dav
modules). Though I haven’t investigated this carefully enough.

Maxim D.

8

Hello!

On Thu, Sep 09, 2010 at 02:19:55PM -0400, rovervr wrote:

example where X-Accel-Redirect is used for dynamic files?
With X-Accel-Redirect you may redirect request to anything you
want, e.g. to appropriate backend.

Maxim D.

9

This code fixes the double encoding issue while serving static files
without X-Accel-Redirect as stated by Maxim

--- /tmp/nginx-0.8.50orig/src/http/modules/ngx_http_static_module.c
2010-05-24 14:35:10.000000000 +0200
+++ ngx_http_static_module.c    2010-09-09 20:23:29.000000000 +0200
@@ -47,7 +47,7 @@
 static ngx_int_t
 ngx_http_static_handler(ngx_http_request_t *r)
 {
-    u_char                    *last, *location;
+    u_char                    *last, *location, *src, *dst;
     size_t                     root, len;
     ngx_str_t                  path;
     ngx_int_t                  rc;
@@ -83,6 +83,30 @@
     ngx_log_debug1(NGX_LOG_DEBUG_HTTP, log, 0,
                    "http filename: \"%s\"", path.data);

+    /*
+     * X-Accel-Redirect Patch
+     * If the path contains a % it probably must be decoded
+     */
+
+    if( strstr( path.data, "%" ) != NULL  &&
+       r->quoted_uri == 0)
+    {
+        ngx_str_t *uri = &path;
+
+        dst = uri->data;
+        src = uri->data;
+
+        ngx_unescape_uri( &dst, &src, uri->len, NGX_UNESCAPE_URI );
+
+        len = uri->len - ( src - dst ) + 1;
+        if ( len )
+        {
+            dst = ngx_copy( dst, src, len);
+        }
+        uri->len = dst - uri->data;
+    }
+
+
     clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);

     ngx_memzero(&of, sizeof(ngx_open_file_info_t));

Posted at Nginx Forum:

10

This is the last version of the patch for version 0.8.52 which is now
live on our production servers for several days without any flaws.

http://www.coderain.de/nginx/nginx-0.8.52-xred.patch

The escaping takes place at ngx_http_parse_unsafe_uri() as Maxim
suggested.

best regards

volker

Posted at Nginx Forum:

11

Hello!

On Thu, Sep 09, 2010 at 02:29:04PM -0400, rovervr wrote:

This code fixes the double encoding issue while serving static files
without X-Accel-Redirect as stated by Maxim

This one is wrong as well. It won’t solve the problem even for
static files when original request happened to contain escaped
chars.

And, as I already said, there is no sense to guess anything in
static module. The problem should be solved elsewere, preserving
invariant of r->uri being unescaped uri without args.

Maxim D.

12

Hello!

[sorry for long delay, I had no time to review the patch]

On Sun, Oct 03, 2010 at 10:11:58AM -0400, rovervr wrote:

This is the last version of the patch for version 0.8.52 which is now
live on our production servers for several days without any flaws.

http://www.coderain.de/nginx/nginx-0.8.52-xred.patch

The escaping takes place at ngx_http_parse_unsafe_uri() as Maxim
suggested.

s/escaping/unescaping/

This patch is wrong. It will unescape query string as well, which
is expected to remain escaped. Additionaly, at least “…/” unsafe
check should be reconsidered after unescaping.

Maxim D.