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: | 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 )
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:
- 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) ?
- Is this behavior when deactivating
If-Modified-Since
check normal? Because thinking to removeLast-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 , 4 years ago
Description: | modified (diff) |
---|
comment:2 by , 4 years ago
Status: | new → accepted |
---|
comment:4 by , 2 years ago
Component: | nginx-module → documentation |
---|
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 , 2 years ago
Resolution: | → fixed |
---|---|
Status: | accepted → closed |
Fixed in [52ea1f45b324/nginx_org], thanks to all involved.
No. As per HTTP 2616, both validators have to be checked, and this is what nginx does. See previous discussions here and here.
Current behaviour seems to be incorrect, thanks for reporting this. The
if_modified_since off;
setting should either not affectIf-None-Match
processing (assuming "off" means "the “If-Modified-Since” request header field is ignored", as documentation says), or should disableIf-None-Match
regardless of theIf-Modified-Since
header presence (assuming "off" instead means "never return 304").