Opened 6 months ago

Last modified 5 months ago

#2176 accepted defect

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

Reported by: rjorel@… Owned by:
Priority: minor Milestone:
Component: nginx-module 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 (3)

comment:1 by rjorel@…, 6 months ago

Description: modified (diff)

comment:2 by Maxim Dounin, 6 months 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@…, 5 months ago

Thank you for bringing me some clarifications :-)

Last edited 5 months ago by rjorel@… (previous) (diff)
Note: See TracTickets for help on using tickets.