range_filter_module get duplicated Accept-Ranges response headers

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

range_filter_module get duplicated Accept-Ranges response headers

vergil
Hello,

Recently,  we found if we use nginx slice module , and upstream server is
such as a static file server,  nginx will response duplicated
`Accept-Ranges` headers if client request is not included range header.

the minimal config example as follow:

```
server {
        listen                      80;
        server_name          _;
        default_type           text/html;
       
        location /get_file {
            slice 256k;
            proxy_set_header Range $slice_range;
            proxy_pass       <a href="http://127.0.0.1:8899;">http://127.0.0.1:8899;
        }
}
```

use curl to get 1mb file:
```
curl -s http://localhost/get_file/1mb.test -D- -o /dev/null

HTTP/1.1 200 OK
Date: Mon, 06 Jul 2020 10:32:58 GMT
Content-Type: application/octet-stream
Content-Length: 1048576
Connection: keep-alive
Last-Modified: Mon, 06 Jul 2020 07:34:23 GMT
Cache-Control: public, max-age=43200
Expires: Mon, 06 Jul 2020 22:32:58 GMT
ETag: "1594020863.76-1048576-4019326287"
Accept-Ranges: bytes
Accept-Ranges: bytes
```

but if I add range header to curl request, will get expected response.

Then I review the ngx_http_range_filter_module,  in `goto next_filter`( line
253) ,  should we handle  NGX_HTTP_OK response? like:

```
next_filter:

    if (r->headers_out.status == NGX_HTTP_OK) {
        r->headers_out.accept_ranges = NULL;
        return ngx_http_next_header_filter(r);
    }

    r->headers_out.accept_ranges = ngx_list_push(&r->headers_out.headers);
    if (r->headers_out.accept_ranges == NULL) {
        return NGX_ERROR;
    }

    r->headers_out.accept_ranges->hash = 1;
    ngx_str_set(&r->headers_out.accept_ranges->key, "Accept-Ranges");
    ngx_str_set(&r->headers_out.accept_ranges->value, "bytes");

    return ngx_http_next_header_filter(r);
```

I am confused if it is a bug?

Posted at Nginx Forum: https://forum.nginx.org/read.php?2,288569,288569#msg-288569

_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx
Reply | Threaded
Open this post in threaded view
|

Re: range_filter_module get duplicated Accept-Ranges response headers

Roman Arutyunyan
Hello,

On Mon, Jul 06, 2020 at 06:49:15AM -0400, webber wrote:

> Hello,
>
> Recently,  we found if we use nginx slice module , and upstream server is
> such as a static file server,  nginx will response duplicated
> `Accept-Ranges` headers if client request is not included range header.
>
> the minimal config example as follow:
>
> ```
> server {
>         listen                      80;
>         server_name          _;
>         default_type           text/html;
>        
>         location /get_file {
>             slice 256k;
>             proxy_set_header Range $slice_range;
>             proxy_pass       <a href="http://127.0.0.1:8899;">http://127.0.0.1:8899;
>         }
> }
> ```
>
> use curl to get 1mb file:
> ```
> curl -s http://localhost/get_file/1mb.test -D- -o /dev/null
>
> HTTP/1.1 200 OK
> Date: Mon, 06 Jul 2020 10:32:58 GMT
> Content-Type: application/octet-stream
> Content-Length: 1048576
> Connection: keep-alive
> Last-Modified: Mon, 06 Jul 2020 07:34:23 GMT
> Cache-Control: public, max-age=43200
> Expires: Mon, 06 Jul 2020 22:32:58 GMT
> ETag: "1594020863.76-1048576-4019326287"
> Accept-Ranges: bytes
> Accept-Ranges: bytes
> ```
>
> but if I add range header to curl request, will get expected response.
Normally nginx does not send Accept-Ranges with partial responses.  The fact
that the response is partial is itself an indication that the server supports
ranges.  If the upstream server is nginx too, there should be no problem with
slice.  However if the upstream server sends Accept-Ranges with partial
responses, the client ends up with the two Accept-Ranges.

Attached is a patch that removes the original Accept-Ranges.  This brings
back the normal nginx behavior - one Accept-Ranges for full response and
none for partial.  Can you try it and report back if it works for you?

> Then I review the ngx_http_range_filter_module,  in `goto next_filter`( line
> 253) ,  should we handle  NGX_HTTP_OK response? like:
>
> ```
> next_filter:
>
>     if (r->headers_out.status == NGX_HTTP_OK) {
>         r->headers_out.accept_ranges = NULL;
>         return ngx_http_next_header_filter(r);
>     }
>
>     r->headers_out.accept_ranges = ngx_list_push(&r->headers_out.headers);
>     if (r->headers_out.accept_ranges == NULL) {
>         return NGX_ERROR;
>     }
>
>     r->headers_out.accept_ranges->hash = 1;
>     ngx_str_set(&r->headers_out.accept_ranges->key, "Accept-Ranges");
>     ngx_str_set(&r->headers_out.accept_ranges->value, "bytes");
>
>     return ngx_http_next_header_filter(r);
> ```
>
> I am confused if it is a bug?
>
> Posted at Nginx Forum: https://forum.nginx.org/read.php?2,288569,288569#msg-288569
>
> _______________________________________________
> nginx mailing list
> [hidden email]
> http://mailman.nginx.org/mailman/listinfo/nginx
--
Roman Arutyunyan

_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx

slice-ar (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: range_filter_module get duplicated Accept-Ranges response headers

vergil
Hi,

Thanks for your reply. I have tried the patch that removes the original
Accept-Ranges in slice filter module, but it is not work as my expected.
Because I think response header `Accept-Ranges` should be added if client
send a range request.

Actually, in my production environment, my upstream server is Apache
TrafficServer, and according to RFC 7233, section 2.3:
Accept-Ranges(https://tools.ietf.org/html/rfc7233#section-2.3) , I think
original Accept-Ranges header should not be removed in all case. I changed
your patch as follow,  original Accept-Ranges header will be removed just
for no-range request , that will work for me, could you please review that?

diff --git a/src/http/modules/ngx_http_slice_filter_module.c
b/src/http/modules/ngx_http_slice_filter_module.c
index c1edbca2..570deaa5 100644
--- a/src/http/modules/ngx_http_slice_filter_module.c
+++ b/src/http/modules/ngx_http_slice_filter_module.c
@@ -180,6 +180,11 @@ ngx_http_slice_header_filter(ngx_http_request_t *r)
     r->headers_out.content_range->hash = 0;
     r->headers_out.content_range = NULL;

+    if (!r->headers_in.range) {
+        r->headers_out.accept_ranges->hash = 0;
+        r->headers_out.accept_ranges = NULL;
+    }
+
     r->allow_ranges = 1;
     r->subrequest_ranges = 1;
     r->single_range = 1;

Posted at Nginx Forum: https://forum.nginx.org/read.php?2,288569,288627#msg-288627

_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx
Reply | Threaded
Open this post in threaded view
|

Re: range_filter_module get duplicated Accept-Ranges response headers

vergil
In reply to this post by Roman Arutyunyan
Hi,

Sorry to bother you, I rechange the patch as follow:

Previously, if an Accept-Ranges header was already present in the first
slice
response, client received two copies of this header. Now, the slice filters
removes the Accept-Ranges header for partital request and if original server

response Accept-Ranges header, has  from the response prior to setting the
r->allow_ranges flag.


diff --git a/src/http/modules/ngx_http_slice_filter_module.c
b/src/http/modules/ngx_http_slice_filter_module.c
index c1edbca2..9a215e43 100644
--- a/src/http/modules/ngx_http_slice_filter_module.c
+++ b/src/http/modules/ngx_http_slice_filter_module.c
@@ -180,6 +180,11 @@ ngx_http_slice_header_filter(ngx_http_request_t *r)
     r->headers_out.content_range->hash = 0;
     r->headers_out.content_range = NULL;

+    if (!r->headers_in.range && r->headers_out.accept_ranges) {
+        r->headers_out.accept_ranges->hash = 0;
+        r->headers_out.accept_ranges = NULL;
+    }
+
     r->allow_ranges = 1;
     r->subrequest_ranges = 1;
     r->single_range = 1;

Posted at Nginx Forum: https://forum.nginx.org/read.php?2,288569,288629#msg-288629

_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx
Reply | Threaded
Open this post in threaded view
|

Re: range_filter_module get duplicated Accept-Ranges response headers

Roman Arutyunyan
In reply to this post by vergil
Hi,

On 8 Jul 2020, at 07:14, webber <[hidden email]> wrote:

Hi,

Thanks for your reply. I have tried the patch that removes the original
Accept-Ranges in slice filter module, but it is not work as my expected.
Because I think response header `Accept-Ranges` should be added if client
send a range request.

Actually, in my production environment, my upstream server is Apache
TrafficServer, and according to RFC 7233, section 2.3:
Accept-Ranges(https://tools.ietf.org/html/rfc7233#section-2.3) , I think
original Accept-Ranges header should not be removed in all case. I changed
your patch as follow,  original Accept-Ranges header will be removed just
for no-range request , that will work for me, could you please review that?

RFC 7233 does not require a server to send Accept-Ranges at all, and certainly
does not require it to send the header with a 206 response. Indeed Apache does
send Accept-Ranges both with 200 and 206, but nginx only sends it with 200.
This is how nginx works and I don’t think changing this only for the slice module
is a good idea. By the way, the example of a 206 response in RFC 7233 does
not have the Accept-Ranges too: https://tools.ietf.org/html/rfc7233#section-4.1.

The rule nginx follows is this. Accept-Ranges should be sent by a party that does
ranges and is a way to signal the support for ranges. If ranges are done by the nginx
range filter, it should add the header. The Accept-Ranges that came from the
upstream server is unrelated to what the range filter is doing even if it’s exactly the
same header. That’s why the existing Accept-Ranges header should be removed
anyway.

Another question is whether the range filter should add a new header for both
200 and 206 or only for 200. The way it currently works for any response (not only
a slice response) is to add Accept-Ranges only for 200. Whether this should be
changed nginx-wise is a totally different question and I personally don’t think it makes
sense.

diff --git a/src/http/modules/ngx_http_slice_filter_module.c
b/src/http/modules/ngx_http_slice_filter_module.c
index c1edbca2..570deaa5 100644
--- a/src/http/modules/ngx_http_slice_filter_module.c
+++ b/src/http/modules/ngx_http_slice_filter_module.c
@@ -180,6 +180,11 @@ ngx_http_slice_header_filter(ngx_http_request_t *r)
    r->headers_out.content_range->hash = 0;
    r->headers_out.content_range = NULL;

+    if (!r->headers_in.range) {
+        r->headers_out.accept_ranges->hash = 0;
+        r->headers_out.accept_ranges = NULL;
+    }
+
    r->allow_ranges = 1;
    r->subrequest_ranges = 1;
    r->single_range = 1;

Posted at Nginx Forum: https://forum.nginx.org/read.php?2,288569,288627#msg-288627

_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx


_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx
Reply | Threaded
Open this post in threaded view
|

Re: range_filter_module get duplicated Accept-Ranges response headers

vergil
Hi,

OK, I see. Now I agree that Accept-Ranges header should be removed anyway in
slice module, and any plan to fix that?

Posted at Nginx Forum: https://forum.nginx.org/read.php?2,288569,288632#msg-288632

_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx