nginx returns 412 given if-match, thus ignoring etag from upstream

[Alexander Sergeyev]

The used workflow as follows: the upstream wsgi server knows etags/versions of resources, but these resources are static and available for nginx to serve directly from fs. The upstream returns x-accel-redirect to an internal location, but also provides etag header to pass along with response.

The sample configuration:

http {

etag off;

upstream test_upstream {

server 127.0.0.1:8080;

}

server {

listen 0.0.0.0:8000;

location /x-accel-images {

internal;
add_header ETag $upstream_http_etag;
alias /tmp;

}

location / {

uwsgi_pass test_upstream;
uwsgi_param QUERY_STRING $query_string;
uwsgi_param REQUEST_METHOD $request_method;
uwsgi_param CONTENT_TYPE $content_type;
uwsgi_param CONTENT_LENGTH $content_length;
uwsgi_param REQUEST_URI $request_uri;
uwsgi_param PATH_INFO $document_uri;
uwsgi_param DOCUMENT_ROOT $document_root;
uwsgi_param SERVER_PROTOCOL $server_protocol;
uwsgi_param REQUEST_SCHEME $scheme;
uwsgi_param HTTPS $https if_not_empty;
uwsgi_param REMOTE_ADDR $remote_addr;
uwsgi_param REMOTE_PORT $remote_port;
uwsgi_param SERVER_PORT $server_port;
uwsgi_param SERVER_NAME $server_name;
uwsgi_param HTTP_PROXY "";

}

}

}

This works okay when no special caching headers are in play. If-None-Match is handled on the upstream, works as expected. But a problem arises when If-Match is requested by a client.

The upstream server receives a request from nginx, sees that if-match is safisfied and returns a 200 response to nginx (providing etag along with it). But nginx receives the upstream response, seemingly ignores the upstream etag and returns 412 to the client.

More specifically, with "etag on" (just for debugging):
2017/03/31 12:32:11 [debug] 21044#0: *1 http im:""123"" etag:"58dd1610-e"
2017/03/31 12:32:11 [debug] 21044#0: *1 http special response: 412, "/x-accel-images/123/456?"

Here it looks like nginx generates its own etag, sees unsatisfied if-match and returns 412.

With "etag off":
<...>
2017/03/31 12:34:20 [debug] 21076#0: *1 uwsgi param: "HTTP_HOST: 127.0.0.1:8000"
2017/03/31 12:34:20 [debug] 21076#0: *1 uwsgi param: "HTTP_USER_AGENT: curl/7.53.1"
2017/03/31 12:34:20 [debug] 21076#0: *1 uwsgi param: "HTTP_ACCEPT: */*"
2017/03/31 12:34:20 [debug] 21076#0: *1 uwsgi param: "HTTP_IF_MATCH: "123""
<...>
2017/03/31 12:34:20 [debug] 21076#0: *1 http uwsgi status 200 "200 OK"
2017/03/31 12:34:20 [debug] 21076#0: *1 http uwsgi header: "X-Accel-Redirect: /x-accel-images/123/456"
2017/03/31 12:34:20 [debug] 21076#0: *1 http uwsgi header: "ETag: "123""
2017/03/31 12:34:20 [debug] 21076#0: *1 http uwsgi header: "Content-Type: text/html; charset=utf-8"
2017/03/31 12:34:20 [debug] 21076#0: *1 http uwsgi header: "Content-Length: 0"
2017/03/31 12:34:20 [debug] 21076#0: *1 http uwsgi header done
<...>
2017/03/31 12:34:20 [debug] 21076#0: *1 internal redirect: "/x-accel-images/123/456?"
2017/03/31 12:34:20 [debug] 21076#0: *1 rewrite phase: 0
2017/03/31 12:34:20 [debug] 21076#0: *1 test location: "/"
2017/03/31 12:34:20 [debug] 21076#0: *1 test location: "x-accel-images"
2017/03/31 12:34:20 [debug] 21076#0: *1 using configuration "/x-accel-images"
2017/03/31 12:34:20 [debug] 21076#0: *1 http cl:-1 max:1048576
2017/03/31 12:34:20 [debug] 21076#0: *1 rewrite phase: 2
2017/03/31 12:34:20 [debug] 21076#0: *1 post rewrite phase: 3
2017/03/31 12:34:20 [debug] 21076#0: *1 access phase: 4
2017/03/31 12:34:20 [debug] 21076#0: *1 post access phase: 5
2017/03/31 12:34:20 [debug] 21076#0: *1 content phase: 6
2017/03/31 12:34:20 [debug] 21076#0: *1 content phase: 7
2017/03/31 12:34:20 [debug] 21076#0: *1 content phase: 8
2017/03/31 12:34:20 [debug] 21076#0: *1 http filename: "/tmp/123/456"
2017/03/31 12:34:20 [debug] 21076#0: *1 add cleanup: 00000000017558A0
2017/03/31 12:34:20 [debug] 21076#0: *1 http static fd: 9
2017/03/31 12:34:20 [debug] 21076#0: *1 http special response: 412, "/x-accel-images/123/456?"
2017/03/31 12:34:20 [debug] 21076#0: *1 HTTP/1.1 412 Precondition Failed
Server: nginx/1.11.11
Date: Fri, 31 Mar 2017 09:34:20 GMT
Content-Type: text/html
Content-Length: 190
Connection: keep-alive

In code:

ngx_http_not_modified_header_filter (src/http/modules/ngx_http_not_modified_filter_module.c):

r->headers_in.if_match is true => calls ngx_http_test_if_match

ngx_http_test_if_match (src/http/modules/ngx_http_not_modified_filter_module.c):

r->headers_out.etag == NULL is true => returning 412

r->headers_out.etag is NULL in ngx_http_not_modified_header_filter(), it looks like add_header is processed later, thus causing this behaviour.

[Maxim Dounin]

That's expected behaviour. Custom headers as configured with add_header are added to the response by ngx_http_headers_filter, and this happens after ngx_http_not_modified_filter which checks If-Not-Modified, If-Match, and other conditional headers. Therefore, while it is possible to add any headers to the response using the add_header directive, these headers do not affect processing of conditional headers.

(Moreover, the "do not affect processing" thing is also generally true for other headers as well, even if they are expected to be handled by filters which come after ngx_http_headers_filter, as headers filter generally do not try to overwrite existing headers and/or set appropriate internal pointers to the headers added.)

This is unlikely to be changed. If you want to supply custom ETag's by your backend, consider checking conditional headers by the backend as well.

[Alexander Sergeyev]

If you want to supply custom ETag's by your backend, consider checking conditional headers by the backend as well.

Well, conditional headers are checked by the backend. The problem is a bit more complicated.

If-Match is verified by the backend and it succeeds (no precondition failure). But then nginx is not passing the response as it is expected, but returns 412 instead. And the reason is that nginx carries out its own check for if-match and during this check there is no etag to match (because it would be added later).

[Alexander Sergeyev]

while it is possible to add any headers to the response using the add_header directive, these headers do not affect processing of conditional headers

This makes sense, yes. But this also means If-Match is broken for X-Accel-Redirect + backend ETag, because there is no way to prevent precondition failure error (nginx doesn't known about upstream etag when it is needed). Unless there is some other way to configure backend etags (ie not add_header).

[Maxim Dounin]

Ok, so the only way seems to use proxying with If-* headers removed after X-Accel-Redirect. This will resolve the problem by disabling conditional headers processing in nginx itself.

[Alexander Sergeyev]

use proxying with If-* headers removed after X-Accel-Redirect

I think you're suggesting something like:

location ~ ^/x-accel-landing/(.*)$ {
    internal;
    proxy_pass http://127.0.0.1:8000/actual-static/$1;
    proxy_set_header If-Match "";
}

location /actual-static {
    alias /tmp;
}

location / {
    uwsgi_pass ...;
}

Is this what you're proposing?

This approach has some problems: (*) rebuffering file data in self-proxied request (*) excessive fd usage.
Also, I cannot make actual-static location internal, thus creating some security issues. Fixing these will employ additional complexity and inefficiency which is sad.

Is this issue techically/architecturally possible to fix given that I would like to try fixing these myself (maybe with some guidance though)?

[Maxim Dounin]

Yes, obviously this won't be as effective as serving static files directly. As for access control, there is more than one way to do things properly.

In your particular case, most simple solution would be to flip the "redirect" flag in ngx_http_upstream_headers_in (in ngx_http_upstream.c) for ETag, so nginx will copy / inherit ETag from the original response with X-Accel-Redirect. Though for obvious reasons this solution isn't something can be committed.

As for the add_header behaviour, changes here are likely to introduce many other problems, and better to be avoided without very good understanding of the consequences. Therefore "unlikely to be changed" above.

[Alexander Sergeyev]

Though for obvious reasons this solution isn't something can be committed.
Therefore "unlikely to be changed" above

I see. There is always an other way to introduce some option for add_header which defaults to the current behaviour or just entirely new directive. But then I suppose it's too rare usecase for this to be justified.

Thank you.

PS. Nevertheless, it's not so obvious pitfall, maybe it should be mentioned somewhere around X-Accel-Redirect or add_header (ie here is not the whole story: ​https://forum.nginx.org/read.php?2,205636,205665).

[Maxim Dounin]

Closing this. As explained above, current behaviour is highly unlikely to be changed.