Latency problem with one browser

[Maxim Dounin]

Hello!

On Sun, Mar 04, 2012 at 02:39:53PM +0100, Piotr Sikora wrote:

> Hi Maxim,
> 
> >>Mozilla/5.0 (PlayBook; U; RIM Tablet OS 1.0.0; en-US) \
> >>AppleWebKit/534.8+ (KHTML like Gecko) version/0.0.1 Safari/534.8+
> >
> >Detecting all browsers (thousands of them) which pretend to be
> >Safari is not an option.  And, after all, browsers use other's
> >product token for reason, and workarounds for bugs is what they
> >expect to get as a result.
> 
> Except that PlayBook isn't a browser, it's a device ;)
> 
> >I'm planning to disable this Safari workaround by default instead,
> >the following patch is already sitting in my queue:
> >
> ># HG changeset patch
> ># User Maxim Dounin <mdounin at mdounin.ru>
> ># Date 1330697796 -14400
> ># Node ID 88e257b4bd3d61e30d1421ab6f2e417a8baedb96
> ># Parent  277a1822fc5cf7ad83297c74839ca8c84a49680c
> >Removed safari from keepalive_disable default.
> >
> >The bug in question is likely already fixed (though unfortunately we have
> >no information available as Apple's bugtracker isn't open), and the
> >workaround seems to be too pessimistic for modern versions of Safari
> >as well as other webkit-based browsers pretending to be Safari.
> >
> >diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
> >--- a/src/http/ngx_http_core_module.c
> >+++ b/src/http/ngx_http_core_module.c
> >@@ -3568,8 +3568,7 @@ ngx_http_core_merge_loc_conf(ngx_conf_t
> >ngx_conf_merge_bitmask_value(conf->keepalive_disable,
> >                               prev->keepalive_disable,
> >                               (NGX_CONF_BITMASK_SET
> >-                               |NGX_HTTP_KEEPALIVE_DISABLE_MSIE6
> >-                               |NGX_HTTP_KEEPALIVE_DISABLE_SAFARI));
> >+                               |NGX_HTTP_KEEPALIVE_DISABLE_MSIE6));
> >     ngx_conf_merge_uint_value(conf->satisfy, prev->satisfy,
> >                               NGX_HTTP_SATISFY_ALL);
> >     ngx_conf_merge_uint_value(conf->if_modified_since, prev->if_modified_since,
> 
> I don't really agree with this, it's analogues to removing IE work-around
> instead of restricting it to versions older than IE6 SV1. I was looking at
> this issue over the weekend and the "keepalive_disable safari" option is
> simply too broad and that's what needs to be fixed.
> 
> According to https://bugs.webkit.org/show_bug.cgi?id=5760#c38,
> the underlying issue was in the Foundation framework on iOS/OSX, not Safari,
> which means that Safari on Windows as well as everything else (Android,
> BlackBerry, etc.) that uses WebKit was never affected.
> 
> I agree with you and I believe that it's fixed in the most recent releases
> of iOS/OSX, but I wouldn't bet that it's fixed in iOS 4.x, OSX 10.6.x or
> older releases, so why should we disable the work-around for them?

I believe that the workaround is too pessimistic even for affected 
versions as it disables keepalive completely, while the problem 
will only manifest itself if there are POSTs.

If someone will find itself affected and the cost of disabled 
keepalive is acceptable - it will be still possible to explicitly 
enable the workaround.

Restricting the workaround to something like "Safari on Mac OS X" 
would be fine too, though it's mostly orthogonal to changing the 
default.

Maxim Dounin