Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#1397 closed defect (fixed)

HTTP/2 broken in popular Android libraries with nginx v. 1.13.6

Reported by: serguei.ivantsov@… Owned by:
Priority: critical Milestone:
Component: nginx-core Version: 1.13.x
Keywords: Cc:
uname -a: Linux some.domain.com 4.13.3-xn #1 SMP Thu Sep 21 18:31:01 EEST 2017 x86_64 Intel(R) Xeon(R) CPU E5-2620 v4 @ 2.10GHz GenuineIntel GNU/Linux
nginx -V: nginx version: nginx/1.13.6
built with OpenSSL 1.0.2l 25 May 2017
TLS SNI support enabled
configure arguments: --prefix=/usr --conf-path=/etc/nginx/nginx.conf --error-log-path=/var/log/nginx/error_log --pid-path=/run/nginx.pid --lock-path=/run/lock/nginx.lock --with-cc-opt='-I/usr/include -DNGX_HAVE_INET6=0' --with-ld-opt=-L/usr/lib64 --http-log-path=/var/log/nginx/access_log --http-client-body-temp-path=/var/lib/nginx/tmp/client --http-proxy-temp-path=/var/lib/nginx/tmp/proxy --http-fastcgi-temp-path=/var/lib/nginx/tmp/fastcgi --http-scgi-temp-path=/var/lib/nginx/tmp/scgi --http-uwsgi-temp-path=/var/lib/nginx/tmp/uwsgi --with-compat --with-file-aio --with-http_v2_module --with-pcre --with-threads --without-http_memcached_module --without-http_scgi_module --without-http_ssi_module --without-http_split_clients_module --without-http_upstream_hash_module --without-http_upstream_ip_hash_module --without-http_upstream_keepalive_module --without-http_upstream_least_conn_module --without-http_userid_module --without-http_uwsgi_module --with-http_geoip_module --with-http_gzip_static_module --with-http_stub_status_module --with-http_realip_module --with-http_ssl_module --without-stream_access_module --without-stream_geo_module --without-stream_limit_conn_module --without-stream_map_module --without-stream_return_module --without-stream_split_clients_module --without-stream_upstream_hash_module --without-stream_upstream_least_conn_module --without-stream_upstream_zone_module --without-mail_imap_module --without-mail_pop3_module --without-mail_smtp_module --user=nginx --group=nginx

Description

Both Retrofit and OkHttp affected.
On the client, error is:
java.net.ProtocolException: Expected ':status' header not present

https://stackoverflow.com/questions/46807237/protocolexception-expected-status-header-not-present

Change History (14)

comment:1 by Valentin V. Bartenev, 6 years ago

Could you try to revert this change and test?

comment:2 by serguei.ivantsov@…, 6 years ago

The lib (OkHttp) is working again if revert that change.

Last edited 6 years ago by serguei.ivantsov@… (previous) (diff)

comment:3 by Maxim Dounin, 6 years ago

Ok, so it looks like the lib is not able to handle Dynamic Table Size updates. This looks like a bug in the library to me. We may consider introducing a workaround in nginx, though I'm not sure it makes sense. A better solution would be to fix the library.

comment:4 by Valentin V. Bartenev, 6 years ago

Can you confirm, that you don't use any 3rd-party patches as well?

comment:5 by serguei.ivantsov@…, 6 years ago

It is vanilla library, though it might be not the latest version. I need to ask developers of the affected app.
I've checked sources at GitHub
https://github.com/square/okhttp/blob/master/okhttp/src/main/java/okhttp3/internal/http2/Hpack.java
And appears they have above mentioned "dynamic table" support.

comment:6 by Valentin V. Bartenev, 6 years ago

I asked mostly about nginx. There are known patches for nginx that can break HTTP/2 functionality.

comment:7 by serguei.ivantsov@…, 6 years ago

It is a version from Gentoo repository. Let me check if they apply patches.

comment:8 by serguei.ivantsov@…, 6 years ago

Well, updating to the latest version of the libraries resolves this issue.
It is not a bug of nginx, but looks like this new feature affects a lot of old software.

comment:9 by Maxim Dounin, 6 years ago

See also #1441.

comment:10 by qiqizjl@…, 6 years ago

Hello, we also encountered this problem. In addition to downgrading nginx and upgrade okhttp, what other solutions now?

comment:11 by maxim, 6 years ago

Milestone: 1.13

Ticket retargeted after milestone closed

comment:12 by Maxim Dounin, 6 years ago

For the record, I've posted a patch to add a workaround for this on nginx side here:

http://mailman.nginx.org/pipermail/nginx-devel/2018-July/011269.html

I'm, however, not convinced that it needs to be committed, as there are associated downsides for non-buggy clients.

comment:13 by Maxim Dounin <mdounin@…>, 6 years ago

In 7335:fbb683496705/nginx:

HTTP/2: workaround for clients which fail on table size updates.

There are clients which cannot handle HPACK's dynamic table size updates
as added in 12cadc4669a7 (1.13.6). Notably, old versions of OkHttp library
are known to fail on it (ticket #1397).

This change makes it possible to work with such clients by only sending
dynamic table size updates in response to SETTINGS_HEADER_TABLE_SIZE. As
a downside, clients which do not use SETTINGS_HEADER_TABLE_SIZE will
continue to maintain default 4k table.

comment:14 by Maxim Dounin, 6 years ago

Resolution: fixed
Status: newclosed

The workaround committed.

Note: See TracTickets for help on using tickets.