Opened 5 years ago
Last modified 5 years ago
#1958 accepted defect
`modern_browser` definition for Safari version is wrong/unexpected
| Reported by: | Tim Dawborn | Owned by: | |
|---|---|---|---|
| Priority: | minor | Milestone: | |
| Component: | nginx-module | Version: | |
| Keywords: | safari, modern_browser, browser | Cc: | |
| uname -a: | Linux dev-web03-ap-southeast-2 4.4.0-1096-aws #107-Ubuntu SMP Thu Oct 3 01:51:58 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux | ||
| nginx -V: |
nginx version: nginx/1.10.3 (Ubuntu)
built with OpenSSL 1.0.2g 1 Mar 2016 TLS SNI support enabled configure arguments: --with-cc-opt='-g -O2 -fPIE -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2' --with-ld-opt='-Wl,-Bsymbolic-functions -fPIE -pie -Wl,-z,relro -Wl,-z,now' --prefix=/usr/share/nginx --conf-path=/etc/nginx/nginx.conf --http-log-path=/var/log/nginx/access.log --error-log-path=/var/log/nginx/error.log --lock-path=/var/lock/nginx.lock --pid-path=/run/nginx.pid --http-client-body-temp-path=/var/lib/nginx/body --http-fastcgi-temp-path=/var/lib/nginx/fastcgi --http-proxy-temp-path=/var/lib/nginx/proxy --http-scgi-temp-path=/var/lib/nginx/scgi --http-uwsgi-temp-path=/var/lib/nginx/uwsgi --with-debug --with-pcre-jit --with-ipv6 --with-http_ssl_module --with-http_stub_status_module --with-http_realip_module --with-http_auth_request_module --with-http_addition_module --with-http_dav_module --with-http_geoip_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_image_filter_module --with-http_v2_module --with-http_sub_module --with-http_xslt_module --with-stream --with-stream_ssl_module --with-mail --with-mail_ssl_module --with-threads |
||
Description
http://nginx.org/en/docs/http/ngx_http_browser_module.html
One of the great use cases for ngx_http_browser_module is when your website needs to officially support certain browser versions. E.g. "we support Edge Legacy >= 15, Safari >= 12, and all recent versions of Chrome, Firefox, and Edgium".
With the current implementation of modern_browser, we cannot achieve this, as the version number detected for safari is not the release number, but instead the WebKit build number.
The Safari WebKit build number can be the same across different releases (see the above example user agent strings below). I am currently working around this using a map against the user agent to set a flag variable.
map $http_user_agent $is_safari_lt_12 {
"~ Version/((?:[1-9]|1[0-1])(?:\.\d+)+) (?:Mobile/\w+ )?Safari/(?:\d+(?:\.\d+)*)$" 1;
default 0;
}
and then combining it with ancient_browser and modern_browser directives.
# Redirect requests from IE to the unsupported browser page.
ancient_browser "MSIE ";
ancient_browser "Trident/";
modern_browser unlisted;
if ($ancient_browser) {
rewrite ^/.* /unsupported-browser/ last;
}
if ($is_safari_lt_12) {
rewrite ^/.* /unsupported-browser/ last;
}
It would be much nicer if one could just do
modern_browser safari_version 12;
instead of needing the map and the additional if statement.
More details
https://trac.nginx.org/nginx/browser/nginx/src/http/modules/ngx_http_browser_module.c#L181
The current implementation for modern_browser for safari is to take the numbers that are after Safari/ in the User Agent string. This number is _a_ version number, but is not the _expected_ version number when talking about Safari versions.
https://en.wikipedia.org/wiki/Safari_version_history
The number after Safari/ is the WebKit build number, which is unrelated to the Safari release number. For example, here are some Safari user agent strings:
Mozilla/5.0 (iPad; CPU iPhone OS 12_1_3 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.0 Mobile/15E148 Safari/604.1 Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.1.2 Safari/605.1.15 Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_3) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0.5 Safari/605.1.15
The commonly referred to version number is the number after Version/.
I would like to propose:
1) changing the documentation to make it clearer that the version number one passes to modern_browser safari is the WebKit version number and not the release number; and
2) Adding a new named option for modern_browser that can be used for the Safari release number; e.g. safari_release, safari2, etc.
Thank you for the report. Note that the browser module is somewhat orphaned, and using a
mapto detect appropriate versions might be a better idea.