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 Valentin V. Bartenev, 11 years ago

Type: defectenhancement

the gzip module restricts the HTTP status codes ands doesn't compress responses with the 207 Multi-Status.

Right. And the reason for this was explained here: http://mailman.nginx.org/pipermail/nginx/2012-September/035338.html

comment:2 by Andrea Mayer, 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 Maxim Dounin, 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 Maxim Dounin, 11 years ago

Resolution: wontfix
Status: newclosed

Feedback timeout. Please reopen if there will be interoperability testing results.

comment:5 by Maxim Dounin, 9 years ago

See also #841.

comment:6 by caldavtest@…, 9 years ago

Resolution: wontfix
Status: closedreopened

I'm looking forward to test this patch.

comment:7 by caldavtest@…, 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 …)
  • Thunderbird/38.3.0 Lightning/4.0.3.1
  • Evolution/3.18.2 (doesn't seem to accept gzip, i.e. no compression, but no error, too)
  • DAVdroid/0.9.1.2
Version 1, edited 9 years ago by caldavtest@… (previous) (next) (diff)

comment:8 by caldavtest@…, 9 years ago

Is there anything else I can do?

comment:9 by Maxim Dounin, 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.

in reply to:  9 comment:10 by caldavtest@…, 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).

Last edited 9 years ago by caldavtest@… (previous) (diff)

comment:11 by caldavtest@…, 9 years ago

Is there anything else you need?

comment:12 by caldavtest@…, 8 years ago

Can I somehow help in getting this done?

comment:13 by arnauvp@…, 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.

Note: See TracTickets for help on using tickets.