Hello!
On Fri, Jul 08, 2022 at 07:13:33PM +0000, Lucas Rolff wrote:
> I’m having an nginx instance where I utilise the nginx slice
> module to slice upstream mp4 files when using proxy_cache.
>
> However, I have an interesting origin where if sending a range
> request (which happens when the slice module is enabled), to a
> file that’s less than the slice range, the origin returns a 200
> OK, but with the range related headers such as content-range,
> but obviously the full file is returned since it’s within the
> requested range.
>
> When playing the MP4s through Google Chrome and Firefox it works
> fine when going through the nginx proxy instance, however, it
> somehow breaks Safari (both on MacOS, and iOS) - I guess Safari
> is more strict.
> When playing directly through the origin it works fine in all
> browsers.
>
> The md5 of response from the origin remains the same, so it’s
> not that the response itself is an invalid MP4 file, and even if
> you compare the cache files on disk with a “working” origin and
> the “broken” origin (one sends a 206 Partial Content, another
> sends 200 OK) - the content of the cache files remain the same,
> except obviously the header section of the cache file.
>
> The origin returns a 206 status code, only if the file exceeds
> the slice size, so if I configure a slice size of 5 megabyte,
> only files above 5 megabytes will give 206s. Anything under 5
> megabytes will result in a 200 OK with content-range and the
> correct content-length,
>
> Looking in the slice module itself I see:
> https://github.com/nginx/nginx/blob/master/src/http/modules/ngx_http_slice_filter_module.c#L116-L126
>
>
> if (r->headers_out.status != NGX_HTTP_PARTIAL_CONTENT) {
> if (r == r->main) {
> ngx_http_set_ctx(r, NULL, ngx_http_slice_filter_module);
> return ngx_http_next_header_filter(r);
> }
>
> ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> "unexpected status code %ui in slice response",
> r->headers_out.status);
> return NGX_ERROR;
> }
>
> This seems like the slice module expects a 206 status code to be
> returned,
For the main request, the code accepts two basic valid variants:
- 206, so the slice module will combine multiple responses to
range requests as needed;
- anything else, so the slice module will give up and simply
return the response to the client.
If the module sees a non-206 response to a subrequest, this is an
error, as the slice module expects underlying resources to be
immutable, and does not expect that some ranges can be requested,
while some other aren't. This isn't something related to your
case though.
> however, later in the same function
> https://github.com/nginx/nginx/blob/master/src/http/modules/ngx_http_slice_filter_module.c#L200-L211
>
>
> if (r->headers_out.status == NGX_HTTP_PARTIAL_CONTENT) {
> if (ctx->start + (off_t) slcf->size <= r->headers_out.content_offset) {
> ctx->start = slcf->size
> * (r->headers_out.content_offset / slcf->size);
> }
>
> ctx->end = r->headers_out.content_offset
> + r->headers_out.content_length_n;
>
> } else {
> ctx->end = cr.complete_length;
> }
>
> There it will do an else statement if the status code isn’t 206.
> So would this piece of code ever be reached, since there’s the initial error?
Following the initial check, r->headers_out.status is explicitly
changed to NGX_HTTP_OK. Later on the
ngx_http_next_header_filter() call might again change
r->headers_out.status as long as the client used a range request,
and this is what checked here.
> Additionally I don’t see in RFC7233 that 206 responses are an
> absolute requirement, additionally I don’t see content-range
> being prohibited/forbidden to be used for 200 OK responses.
> Now, if one have a secondary proxy that modifies the response
> headers in between the origin returning 200 OK with the
> Content-Range header, and then strip out the Content-Range
> header, the nginx slice module seems to handle it fine, so
> somehow the combination of 200 OK and a Content-Range header
> being present seems to break the slice module from functioning.
>
> I’m just curious why this happens within the slice module, and
> if there’s any possible solution for it (like allowing the
> combination of 200 OK and Content-Range, since those two would
> still indicate that the origin/upstream supports range requests)
> - obviously it would be nice to fix the upstream server but
> sometimes that’s sadly not possible.
>From the above explanation it is probably already clear that
"disabling slice when an origin returns 200 OK" is what actually
happens.
The issue does not appear without the slice module in your testing
because the Content-Range header seems to be only present in your
backend 200 responses when there was a Range header in the
request, and this is what happens only with the slice module.
I've done some limited testing with Safari and manually added
Content-Range header, and there seem to be at least two issues:
- Range filter in nginx does not expect the Content-Range header
to be already present in 200 responses and simply adds another
one. This results in incorrect range responses with multiple
Content-Range headers, and this breaks Safari.
- Safari also breaks if its test request with "Range: bytes=0-1"
results in 200 with the Content-Range header.
My initial fix was to simply disable handling of 200 responses
with Content-Range headers in the range filter, so such responses
wouldn't be touched at all. This is perfectly correct and
probably the most secure thing to do, but does not work with
Safari due to the second issue outlined above.
Another approach would be to clear pre-existing Content-Range
headers in the range filter. This seems to work, at least in my
testing. See below for the patch.
> I know the parts of the slice module haven’t been touched for
> years, so obviously it works for most people, just dipping my
> toes here to see if there’s a possible solution other than
> disabling slice when an origin returns 200 OK for files smaller
> than the slice size.
Note that that slice module is generally unsafe to use for
arbitrary upstream servers: it relies on expectations which are
beyond the HTTP standard requirements. In particular:
- It requires resources to be immutable, so different range
responses can be combined together.
- It does not try to handle edge cases, such as 416 returned by
the upstream on empty files (which is correct per RFC, but
requires complicated additional handling to convert 416 to 200, so
it is better to just return 200 OK).
In general, the slice module is to be used only in your own
infrastructure when you control the backend and can be sure that
the slice module expectations are met. As such, disabling it for
backends which do something unexpected might actually be a good
idea. On the other hand, in this particular case the nginx
behaviour can be adjusted to handle things gracefully.
Below is a patch to clear pre-existing Content-Range headers
in the range filter. Please test if it works for you.
# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1657439390 -10800
# Sun Jul 10 10:49:50 2022 +0300
# Node ID 219217ea49a8d648f5cadd046f1b1294ef05693c
# Parent 9d98d524bd02a562d9cd83f4e369c7e992c5753b
Range filter: clearing of pre-existing Content-Range headers.
Some servers might emit Conten-Range header on 200 responses, and this
does not seem to contradict RFC 9110: as per RFC 9110, the Content-Range
header have no meaning for status codes other than 206 and 417. Previously
this resulted in duplicate Content-Range headers in nginx responses handled
by the range filter. Fix is to clear pre-existing headers.
diff --git a/src/http/modules/ngx_http_range_filter_module.c b/src/http/modules/ngx_http_range_filter_module.c
--- a/src/http/modules/ngx_http_range_filter_module.c
+++ b/src/http/modules/ngx_http_range_filter_module.c
@@ -425,6 +425,10 @@ ngx_http_range_singlepart_header(ngx_htt
return NGX_ERROR;
}
+ if (r->headers_out.content_range) {
+ r->headers_out.content_range->hash = 0;
+ }
+
r->headers_out.content_range = content_range;
content_range->hash = 1;
@@ -582,6 +586,11 @@ ngx_http_range_multipart_header(ngx_http
r->headers_out.content_length = NULL;
}
+ if (r->headers_out.content_range) {
+ r->headers_out.content_range->hash = 0;
+ r->headers_out.content_range = NULL;
+ }
+
return ngx_http_next_header_filter(r);
}
@@ -598,6 +607,10 @@ ngx_http_range_not_satisfiable(ngx_http_
return NGX_ERROR;
}
+ if (r->headers_out.content_range) {
+ r->headers_out.content_range->hash = 0;
+ }
+
r->headers_out.content_range = content_range;
content_range->hash = 1;
--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx mailing list -- nginx@nginx.org
To unsubscribe send an email to nginx-leave@nginx.org