Opened 4 years ago

Closed 2 years ago

#2176 closed defect (fixed)

Get only 200 responses when "if_modified_since" is set to "off"

Reported by: rjorel@… Owned by:
Priority: minor Milestone:
Component: documentation Version: 1.19.x
Keywords: Headers If-Modified-Since Cc:
uname -a: Linux 93fee44bfd94 5.11.0-16-generic #17-Ubuntu SMP Wed Apr 14 20:12:43 UTC 2021 x86_64 Linux
nginx -V: nginx version: nginx/1.18.0
built with OpenSSL 1.1.1i 8 Dec 2020 (running with OpenSSL 1.1.1j 16 Feb 2021)
TLS SNI support enabled
configure arguments: --prefix=/var/lib/nginx --sbin-path=/usr/sbin/nginx --modules-path=/usr/lib/nginx/modules --conf-path=/etc/nginx/nginx.conf --pid-path=/run/nginx/nginx.pid --lock-path=/run/nginx/nginx.lock --http-client-body-temp-path=/var/lib/nginx/tmp/client_body --http-proxy-temp-path=/var/lib/nginx/tmp/proxy --http-fastcgi-temp-path=/var/lib/nginx/tmp/fastcgi --http-uwsgi-temp-path=/var/lib/nginx/tmp/uwsgi --http-scgi-temp-path=/var/lib/nginx/tmp/scgi --with-perl_modules_path=/usr/lib/perl5/vendor_perl --user=nginx --group=nginx --with-threads --with-file-aio --with-http_ssl_module --with-http_v2_module --with-http_realip_module --with-http_addition_module --with-http_xslt_module=dynamic --with-http_image_filter_module=dynamic --with-http_geoip_module=dynamic --with-http_sub_module --with-http_dav_module --with-http_flv_module --with-http_mp4_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_auth_request_module --with-http_random_index_module --with-http_secure_link_module --with-http_degradation_module --with-http_slice_module --with-http_stub_status_module --with-http_perl_module=dynamic --with-mail=dynamic --with-mail_ssl_module --with-stream=dynamic --with-stream_ssl_module --with-stream_realip_module --with-stream_geoip_module=dynamic --with-stream_ssl_preread_module --add-dynamic-module=/home/buildozer/aports/main/nginx/src/njs-0.5.0/nginx --add-dynamic-module=/home/buildozer/aports/main/nginx/src/ngx_devel_kit-0.3.1/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/ngx_brotli-1.0.0rc/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/ngx_cache_purge-2.5.1/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/nginx-dav-ext-module-3.0.0/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/echo-nginx-module-0.62/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/encrypted-session-nginx-module-0.08/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/ngx-fancyindex-0.5.1/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/headers-more-nginx-module-0.33/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/lua-nginx-module-0.10.19/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/lua-upstream-nginx-module-0.07/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/nchan-1.2.7/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/nginx-http-shibboleth-2.0.1/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/redis2-nginx-module-0.15/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/set-misc-nginx-module-0.32/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/nginx-upload-progress-module-0.9.2/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/nginx-upstream-fair-0.1.3/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/nginx-rtmp-module-1.2.1/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/nginx-vod-module-1.27/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/ngx_http_geoip2_module-3.3/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/naxsi-1.3/naxsi_src --add-dynamic-module=/home/buildozer/aports/main/nginx/src/mod_zip-1.2.0/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/nginx-module-vts-0.1.18/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/traffic-accounting-nginx-module-2.0/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/ngx_http_untar_module-1.0/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/ngx_upstream_jdomain-1.1.5/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/nginx_cookie_flag_module-1.1.0/ --add-dynamic-module=/home/buildozer/aports/main/nginx/src/array-var-nginx-module-0.05/

Description (last modified by rjorel@…)

Hello,

These last days, I tried to understand how web servers like Nginx deal with cache control of resources like JS, CSS or font files. So I learned some things about If-Modified-Since and If-None-Match headers that allows to determine if a resource must be re-sent or not.

I thought that reading source code should be the best solution to understand how etags are generated and how Nginx uses them to generate 304 responses. But I was surprised to read that in src/http/modules/ngx_http_not_modified_filter_module.c:

        if (r->headers_in.if_modified_since
            && ngx_http_test_if_modified(r))
        {
            return ngx_http_next_header_filter(r);
        }

        if (r->headers_in.if_none_match
            && !ngx_http_test_if_match(r, r->headers_in.if_none_match, 1))
        {
            return ngx_http_next_header_filter(r);
        }

whereas I see on MDN web docs and RFC 7232 that If-None-Match takes precedence over If-Modified-Since if web server supports it.

I supposed it's an optimization because etags seems to be generated from last modified times (if I well understood ngx_http_set_etag function in src/http/ngx_http_core_module.c), so checking last modified times before etags is almost the same thing.

So, to be sure that resource validation via etags was done after last modified times, I deactivate If-Modified-Since check with directive if_modified_since off;. After that, I no longer got any 304 responses but only 200 ones. I thought that etag check didn't work, but reading ngx_http_test_if_modified function gave me a clue:

static ngx_uint_t
ngx_http_test_if_modified(ngx_http_request_t *r)
{
    time_t                     ims;
    ngx_http_core_loc_conf_t  *clcf;

    if (r->headers_out.last_modified_time == (time_t) -1) {
        return 1;
    }

    clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);

    if (clcf->if_modified_since == NGX_HTTP_IMS_OFF) {
        return 1;
    }

    // ...

By default, Nginx sends Last-Modified header, so browsers send back If-Modified-Since one. But as I set to off the check, I think ngx_http_test_if_modified stops at the 2e if.

To be sure, I removed If-Modified-Since header using cURL tool, and I also found how to remove Last-Modified header from response. I correctly got 304 responses using these 2 methods separately.

I suppose you're aware of that, but as I cannot be sure I think it's a good idea to report the "issue" to you.

I just have two questions to be sure I well understood:

  1. Am I correct in thinking that the check order is an optimization because etags are generated from timestamps (I suppose checking timestamps is faster than checking etags) ?
  1. Is this behavior when deactivating If-Modified-Since check normal? Because thinking to remove Last-Modified header from response is not obvious to guess here, if we want to keep etag check only.

Regards.
Raphael Jorel

Change History (5)

comment:1 by rjorel@…, 4 years ago

Description: modified (diff)

comment:2 by Maxim Dounin, 4 years ago

Status: newaccepted
  1. Am I correct in thinking that the check order is an optimization because etags are generated from timestamps (I suppose checking timestamps is faster than checking etags) ?

No. As per HTTP 2616, both validators have to be checked, and this is what nginx does. See previous discussions here and here.

  1. Is this behavior when deactivating If-Modified-Since check normal? Because thinking to remove Last-Modified header from response is not obvious to guess here, if we want to keep etag check only.

Current behaviour seems to be incorrect, thanks for reporting this. The if_modified_since off; setting should either not affect If-None-Match processing (assuming "off" means "the “If-Modified-Since” request header field is ignored", as documentation says), or should disable If-None-Match regardless of the If-Modified-Since header presence (assuming "off" instead means "never return 304").

comment:3 by rjorel@…, 4 years ago

Thank you for bringing me some clarifications :-)

Last edited 4 years ago by rjorel@… (previous) (diff)

comment:4 by Maxim Dounin, 2 years ago

Component: nginx-moduledocumentation

Looking into this once again, I tend to think that the current behaviour is actually meaningful: as long as the If-Modified-Since header is present, comparison always fails, and the response is considered modified. This is in line with other if_modified_since possible options, which is documented to specify "how to compare modification time".

It might make sense to better document the current behaviour. Existing documentation was correct when it was introduced, yet no longer correct with introduction of entity tags and If-None-Match handling.

Patch is here:

https://mailman.nginx.org/pipermail/nginx-devel/2023-January/2S6RLEC5MZNDC5CWLEGQGR224LMCHZ5W.html

Review appreciated.

comment:5 by Maxim Dounin, 2 years ago

Resolution: fixed
Status: acceptedclosed

Fixed in [52ea1f45b324/nginx_org], thanks to all involved.

Note: See TracTickets for help on using tickets.