Opened 11 years ago
Last modified 5 years ago
#394 reopened enhancement
gzip module doesn't handle all certain HTTP verbs/statuses
Reported by: | Andrea Mayer | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | |
Component: | nginx-module | Version: | |
Keywords: | gzip | Cc: | |
uname -a: | FreeBSD my.server 8.4-RELEASE-p1 FreeBSD 8.4-RELEASE-p1 #0: Fri Jun 28 03:50:33 UTC 2013 root@amd64-builder.daemonology.net:/usr/obj/usr/src/sys/GENERIC amd64 | ||
nginx -V: |
nginx version: nginx/1.4.2
TLS SNI support enabled configure arguments: --prefix=/usr/local/etc/nginx --with-cc-opt='-I /usr/local/include' --with-ld-opt='-L /usr/local/lib' --conf-path=/usr/local/etc/nginx/nginx.conf --sbin-path=/usr/local/sbin/nginx --pid-path=/var/run/nginx.pid --error-log-path=/var/log/nginx-error.log --user=www --group=www --with-file-aio --with-ipv6 --http-client-body-temp-path=/var/tmp/nginx/client_body_temp --http-fastcgi-temp-path=/var/tmp/nginx/fastcgi_temp --http-proxy-temp-path=/var/tmp/nginx/proxy_temp --http-scgi-temp-path=/var/tmp/nginx/scgi_temp --http-uwsgi-temp-path=/var/tmp/nginx/uwsgi_temp --http-log-path=/var/log/nginx-access.log --with-http_dav_module --add-module=/usr/ports/www/nginx/work/arut-nginx-dav-ext-module-0e07a3e --add-module=/usr/ports/www/nginx/work/ngx-fancyindex-0.3.1 --with-http_geoip_module --with-http_gzip_static_module --with-http_stub_status_module --with-pcre --with-http_ssl_module |
Description
I use nginx as a proxy for a CardDAV server. For CardDAV, HTTP verbs like PROPFIND and REPORT are used and these verbs often return large textual content (text/vcard, text/xml) so compression would be very useful (especially when the DAV collections are synced with a mobile device).
However, HTTP responses from the CardDAV server are only gzip-ed when they have been requested using GET. I thinks this could have two reasons:
- the gzip module restricts the HTTP verbs and doesn't work then using PROPFIND or REPORT, or/and
- the gzip module restricts the HTTP status codes ands doesn't compress responses with the 207 Multi-Status.
I think this is not correct behaviour because every HTTP response entity may and should be compressed when the matching Accept-Encoding was set by the client (and the Content-Type in the server config is correct).
Steps to reproduce:
- Set up a CalDAV/CardDAV server and access it via nginx proxy.
- In nginx, set up gzip compression for proxy requests and the appropriate content types.
- Send a GET request with Accept-Encoding: gzip -> the 200 response is gzip-ed
- Send a PROPFIND request with Accept-Encoding: gzip -> the 207 response is *not* gzip-ed
Expected result:
All responses for all requests should be gzip-ed.
Change History (13)
comment:1 by , 11 years ago
Type: | defect → enhancement |
---|
comment:2 by , 11 years ago
Thanks for the fast response. I have now also found http://www2002.org/CDROM/refereed/444.pdf where the problem is discussed, too. Compression would be really useful for 207 Multistatus (as defined in RFC 4918 Chapter 13) as the response body is text/xml (and can contain larger text content in case of CardDAV addressbook-multiget as defined in RFC 6352 Chapter 8.7).
Transfer-Encoding would possibly be more accurate as the response body doesn't represent a resource ("content"). However, as far as I know, most clients don't support TE. What do you think?
comment:3 by , 11 years ago
For sure Transfer-Encoding isn't practical due to lack of support. Whether or not gzip should be enabled for 207 depends on various clients behaviour. If you can do a review of common WebDAV/CardDAV clients to make sure they either do not send Accept-Encoding: gzip or handle gzipped 207 responses normally - it would be a good starting point.
Just in case, here are trivial experimental patch to enable compression of 207 responses (untested though):
# HG changeset patch # User Maxim Dounin <mdounin@mdounin.ru> # Date 1376915928 -14400 # Node ID 313f533cb5e8ab1228ad5e14ee995b1c818a5d97 # Parent d22eb224aedf4f488d31c90a7a2a75fc0da0f9f8 Gzip filter: compression of 207 Multi-Status responses (ticket #394). Responses with 207 Multi-Status code, as defined by RFC 4918, might be big enough to benefit from compression, especially when used in CardDAV as defined by RFC 6352. This is an experimental patch, needs testing to ensure client interoperability. See ticket #394 for details. diff --git a/src/http/modules/ngx_http_gzip_filter_module.c b/src/http/modules/ngx_http_gzip_filter_module.c --- a/src/http/modules/ngx_http_gzip_filter_module.c +++ b/src/http/modules/ngx_http_gzip_filter_module.c @@ -245,6 +245,7 @@ ngx_http_gzip_header_filter(ngx_http_req if (!conf->enable || (r->headers_out.status != NGX_HTTP_OK + && r->headers_out.status != NGX_HTTP_MULTI_STATUS && r->headers_out.status != NGX_HTTP_FORBIDDEN && r->headers_out.status != NGX_HTTP_NOT_FOUND) || (r->headers_out.content_encoding diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h --- a/src/http/ngx_http_request.h +++ b/src/http/ngx_http_request.h @@ -73,6 +73,7 @@ #define NGX_HTTP_ACCEPTED 202 #define NGX_HTTP_NO_CONTENT 204 #define NGX_HTTP_PARTIAL_CONTENT 206 +#define NGX_HTTP_MULTI_STATUS 207 #define NGX_HTTP_SPECIAL_RESPONSE 300 #define NGX_HTTP_MOVED_PERMANENTLY 301
comment:4 by , 11 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Feedback timeout. Please reopen if there will be interoperability testing results.
comment:6 by , 9 years ago
Resolution: | wontfix |
---|---|
Status: | closed → reopened |
I'm looking forward to test this patch.
comment:7 by , 9 years ago
I have compiled nginx 1.9.7 with this patch and can confirm that it works with:
- curl/7.43.0 (curl --compressed -vX PROPFIND …) – gzip is used and works
- Thunderbird/38.3.0 Lightning/4.0.3.1 – gzip is used and works
- Evolution/3.18.2 – doesn't seem to accept gzip, i.e. no compression, but no error, too
- DAVdroid/0.9.1.2 – gzip is used and works
follow-up: 10 comment:9 by , 9 years ago
Thanks for testing. It would be really useful to also check some common DAV clients, including ones in major OSes like Windows and OS X to make sure the change doesn't cause bad interoperability effects. It would be also cool to test on a real service if you have one where 207 responses are used in practice.
comment:10 by , 9 years ago
(Un)fortunately, I don't have Windows or Mac OS X.
However, I'd still integrate the patch because it is only standards-compliant behaviour and there's no hint that it could cause problems. Also, other services like iCloud do that, too (so there should no problems with Mac OS X at least). Just do
curl --compressed -vX PROPFIND -H 'Content-Type: application/xml; charset=utf-8' -H 'Depth: 0' --data '<d:propfind xmlns:d="DAV:"><d:prop><d:current-user-principal/></d:prop></d:propfind>' -u user@iCloud:password https://contacts.icloud.com/
to see that iCloud returns a gzipped multi-status.
Also, my tests were done with real DAV servers behind an nginx proxy (which is the common configuration).
comment:13 by , 5 years ago
+1 for integrating this. We are using the patched nginx in front of NextCloud 16 and it works with different clients, including Android and OSX.
Right. And the reason for this was explained here: http://mailman.nginx.org/pipermail/nginx/2012-September/035338.html