In T209590#4753835, @Anomie wrote:

When I try making a variant of that request (spaces replaced with %20) using curl, I get back the proper API response.

Your client may have used HTTP/1.1 which does not trigger the bug. Just try again with HTTP/2.

curl 7.62.0 appears to use HTTP/2 by default.

Just in case I tried it again, and curl receives a valid API response with the no-spaces query with any of HTTP/2, HTTP/1.1, and HTTP/1.0.

Firefox 63, Chromium 70, curl 7.58 all work fine with HTTP/1.1 but fail with HTTP/2 on Wikimedia servers. Other HTTP/2 servers seem not to be affected.

@Anomie: when I try https://fa.wikipedia.org/w/api.php?action=query&prop=coordinates&titles=File:Pandora Tomorrow box art.jpg&format=json with my browser, I get a valid API response (and HTTP status is obviously 200). But when I try longer links, I do not get a response. And when I replace the spaces with %20 I also do not get a response. This is with Firefox 63.0.3

Retrying with Chrome 70.0 I get a valid response with https://fa.wikipedia.org/w/api.php?action=query&prop=coordinates&titles=File:Pandora Tomorrow box art.jpg&format=json (the browser automatically replaces spaces with %20), and with longer URLs, with or without %20, I get the following error message:

This site can’t be reached

fa.wikipedia.org unexpectedly closed the connection.

Try:

Checking the connection
Checking the proxy and the firewall
Running Windows Network Diagnostics
ERR_CONNECTION_CLOSED

Repeating it with Microsoft Edge, the short link works, and the long link with %20 works too! The long link with spaces, however, gives the following error message:

Hmmm...can’t reach this page
Try this
Make sure you’ve got the right web address: https://fa.wikipedia.org
Search for "https://fa.wikipedia.org" on Bing
Refresh the page
Details 
The connection to the website was reset.
Error Code: INET_E_DOWNLOAD_FAILURE

I wonder how Microsoft Edge works but Firefox and Chrome do not.

Still not a bug in the API. But it does seem to be a bug somewhere in the infrastructure. I've managed to get consistent results by using only ASCII characters in the URL.

I'm going to rewrite and reassign the task.

@Anomie Thanks, I couldn't have pinpointed this the way you did. Much appreciated.

P7916, which worked for @Huji, does not work for me. The path in P7916 (/wiki/Main_Page?_=xxxx...) is 4698 characters long.

In my tests, the following works (path length 4683):

$ curl --http2 -v "https://en.wikipedia.org/wiki/Main_Page?_=$(printf 'x%.0s' {1..4665})" > /dev/null
[...] normal response
$ echo -n "/wiki/Main_Page?_=$(printf 'x%.0s' {1..4665})" | wc
      0       1    4683

while the issue is reproducible by adding one additional character:

$ curl --http2 -v "https://en.wikipedia.org/wiki/Main_Page?_=$(printf 'x%.0s' {1..4666})" > /dev/null
[...]
> Host: en.wikipedia.org
> User-Agent: curl/7.62.0
> Accept: */*
> 
{ [5 bytes data]
* Connection state changed (MAX_CONCURRENT_STREAMS == 128)!
} [5 bytes data]
* TLSv1.2 (IN), TLS alert, close notify (256):
{ [2 bytes data]
* Empty reply from server
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
* Connection #0 to host en.wikipedia.org left intact
curl: (52) Empty reply from server

The curl version I'm using is 7.62.0, same as @Huji 's.

On a system with an older curl (7.52.1), the error is different:

$ curl -s --http2 -v "https://en.wikipedia.org/wiki/Main_Page?_=$(printf 'x%.0s' {1..4666})" > /dev/null
> Host: en.wikipedia.org
> User-Agent: curl/7.52.1
> Accept: */*
> 
{ [5 bytes data]
* Connection state changed (MAX_CONCURRENT_STREAMS updated)!
} [5 bytes data]
* TLSv1.2 (IN), TLS alert, Client hello (1):
{ [2 bytes data]
* Unexpected EOF
* Curl_http_done: called premature == 1
* Closing connection 0
} [5 bytes data]

The TLS interaction is interesting. Normal:

$ curl -s --http2 -v "https://en.wikipedia.org/wiki/Main_Page?_=$(printf 'x%.0s' {1..4665})" 2>&1 | grep -i tls
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-ECDSA-CHACHA20-POLY1305

Broken:

$ curl -s --http2 -v "https://en.wikipedia.org/wiki/Main_Page?_=$(printf 'x%.0s' {1..4666})" 2>&1 | grep -i tls
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-ECDSA-CHACHA20-POLY1305
* TLSv1.2 (IN), TLS alert, close notify (256):

@Vgutierrez might want to jump in!

Ok, so after capturing a crashing request, we can see the following stuff going on at HTTP2 level:

willikins:~ vgutierrez$ tshark -r ~/crash.pcap.pcapng -o ssl.keylog_file:/tmp/crash.key -Y "ip.host == 91.198.174.192" |grep HTTP2
  311  19.168154 91.198.174.192 → 192.168.1.43 HTTP2 135 SETTINGS, WINDOW_UPDATE
  314  19.171145 192.168.1.43 → 91.198.174.192 HTTP2 119 Magic
  315  19.171145 192.168.1.43 → 91.198.174.192 HTTP2 122 SETTINGS
  316  19.171145 192.168.1.43 → 91.198.174.192 HTTP2 108 WINDOW_UPDATE
  319  19.171358 192.168.1.43 → 91.198.174.192 HTTP2 1356 HEADERS
  320  19.171666 192.168.1.43 → 91.198.174.192 HTTP2 104 SETTINGS
  325  19.334817 91.198.174.192 → 192.168.1.43 HTTP2 104 SETTINGS
  327  19.334820 91.198.174.192 → 192.168.1.43 HTTP2 112 GOAWAY

Inspecting the HTTP2 GOAWAY response we get the following error:

Error: ENHANCE_YOUR_CALM (11)

and as @ema pointed out in our IRC discussion, nginx returns the 420 ENHANCE_YOUR_CALM error in two scenarios:

 if (len > h2c->state.header_limit) {
        ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
                       "client exceeded http2_max_header_size limit");
         return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_ENHANCE_YOUR_CALM);
}

and

    if ((size_t) len > h2scf->max_field_size) {
         ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
                       "client exceeded http2_max_field_size limit");
         return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_ENHANCE_YOUR_CALM);
}

According to nginx documentation those two limits are handled by the following configuration parameters:

In our scenario, it looks like http2_max_field_size is the one limiting us
http2_max_field_size:

Limits the maximum size of an HPACK-compressed request header field. The limit applies equally to both name and value. Note that if Huffman encoding is applied, the actual size of decompressed name and value strings may be larger. For most requests, the default limit should be enough.

I've tested this on cp1008 and increasing http2_max_field_size makes the issue go away.

Thanks for working on this. I found that 420 message hilarious! I resisted making a 420 pun, so instead, I offer you this meme to thank you for all the hard work :)

Note it's ok if the server implements some limits. The problem here is that the client isn't getting the expected 4xx response (e.g. 414 or 431) when the limit is hit, all it sees is a dropped connection.

In T209590#4855448, @Anomie wrote:

Note it's ok if the server implements some limits. The problem here is that the client isn't getting the expected 4xx response (e.g. 414 or 431) when the limit is hit, all it sees is a dropped connection.

Yup. Note that we do that at the Apache layer. Using HTTP/1.1 to avoid this very bug:

$ curl -s --http1.1 -v "https://en.wikipedia.org/$(printf 'x%.0s' {1..8177})" 2>&1 | grep '< HTTP'
< HTTP/1.1 414 Request-URI Too Long

The default on Apache is indeed 8177: LimitRequestLine - len("GET") - len("HTTP/1.1") - len(two spaces)

As there's no limit defined by the RFCs, let's agree on a maximum length that makes sense and enforce it early on in our request processing. Nginx seems like the right place to me.

Thank you all for investigating. What would you recommend as the best practice w.r.t. URL length in client code? E.g. POST requests, hard-coded limits, retrying on 414-coded responses...

As @ema suggests we should set the URL length limit as close to the client as possible, right now the nginx running in the cache nodes is that place.
Usually retrying on a 4xx response code is not the way to go, as a 4xx signals an issue in the client request that should be fixed.
Also we should take into account that even though RFCs don't set URL length limits but user-agents (browsers) do have limitations in place. As a reference we could consider this table obtained here:

It looks like that as long as we want to support Internet Explorer & Edge we should keep our URLs shorter than 2047 characters.

@Vgutierrez my bad, I meant retrying with a shorter URL.

Change 482666 had a related patch set uploaded (by Vgutierrez; owner: Vgutierrez):
[operations/puppet@production] tlsproxy: Set http2_max_field_size to 8k

https://gerrit.wikimedia.org/r/482666

Change 482666 merged by Vgutierrez:
[operations/puppet@production] tlsproxy: Set http2_max_field_size to 8k

https://gerrit.wikimedia.org/r/482666

I still get dropped connections (not 414) with much longer URLs (total header size from 9442 bytes onward). Is that expected?

So, after setting http2_max_field_size to 8k, you can properly fetch the same URLs over HTTP 1.1 and HTTP2.
If the max length is exceeded over HTTP 1.1, a 414 error is returned by the Apache server at the applayer. If this happens over HTTP2, a ENHANCE_YOUR_CALM error is returned by nginx at the cache layer.

As defined in RFC 7540, the ENHANCE_YOUR_CALM error is considered a client error:

ENHANCE_YOUR_CALM (0xb):  The endpoint detected that its peer is
      exhibiting a behavior that might be generating excessive load.

curl fails to show this error for some reason, but as you can see it's being returned by our servers as expected:

>>> import requests
>>> from hyper.contrib import HTTP20Adapter
>>> s = requests.Session()
>>> s.mount('https://en.wikipedia.org', HTTP20Adapter())
>>> s.get('https://en.wikipedia.org/wiki/Main_Page?_={}'.format('x'* 10000))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/vgutierrez/.virtualenvs/http2/lib/python3.7/site-packages/requests/sessions.py", line 546, in get
    return self.request('GET', url, **kwargs)
  File "/Users/vgutierrez/.virtualenvs/http2/lib/python3.7/site-packages/requests/sessions.py", line 533, in request
    resp = self.send(prep, **send_kwargs)
  File "/Users/vgutierrez/.virtualenvs/http2/lib/python3.7/site-packages/requests/sessions.py", line 646, in send
    r = adapter.send(request, **kwargs)
  File "/Users/vgutierrez/.virtualenvs/http2/lib/python3.7/site-packages/hyper/contrib.py", line 80, in send
    resp = conn.get_response()
  File "/Users/vgutierrez/.virtualenvs/http2/lib/python3.7/site-packages/hyper/common/connection.py", line 129, in get_response
    return self._conn.get_response(*args, **kwargs)
  File "/Users/vgutierrez/.virtualenvs/http2/lib/python3.7/site-packages/hyper/http20/connection.py", line 312, in get_response
    return HTTP20Response(stream.getheaders(), stream)
  File "/Users/vgutierrez/.virtualenvs/http2/lib/python3.7/site-packages/hyper/http20/stream.py", line 230, in getheaders
    self._recv_cb(stream_id=self.stream_id)
  File "/Users/vgutierrez/.virtualenvs/http2/lib/python3.7/site-packages/hyper/http20/connection.py", line 771, in _recv_cb
    self._single_read()
  File "/Users/vgutierrez/.virtualenvs/http2/lib/python3.7/site-packages/hyper/http20/connection.py", line 722, in _single_read
    raise ConnectionError(error_string)
hyper.http20.exceptions.ConnectionError: Encountered error ENHANCE_YOUR_CALM 0xb: Processing capacity exceeded

Firefox 64 and Chromium 72 also react to the "enhance your calm" as a dropped connection rather than as a 414.

Firefox gives me an empty page, while Chromium is a bit more explicit about it:

So regardless of the HTTP/2 spec considering it a client error, actual clients seem to be treating it as a transport-layer error rather than an HTTP-layer error.

The swap of Traffic for Traffic-Icebox in this ticket's set of tags was based on a bulk action for all such tickets that haven't been updated in 6 months or more. This does not imply any human judgement about the validity or importance of the task, and is simply the first step in a larger task cleanup effort. Further manual triage and/or requests for updates will happen this month for all such tickets. For more detail, have a look at the extended explanation on the main page of Traffic-Icebox . Thank you!

I think the only viable solution here is that instead of increasing limits, is to allow payload in a request body. ElasticsSearch uses body in GET requests, which is not standard and one could have issues somewhere in the path because of that, so instead maybe all GET API endpoints should also allow POST for read-only queries. Maybe with support for X-HTTP-Method-Override header, so that one could make a POST with X-HTTP-Method-Override: GET and query parameters in the body.

Oh, POST works already instead of GET. This should be documented somewhere in https://www.mediawiki.org/wiki/API:Main_page. Because when I read documentation like https://www.mediawiki.org/wiki/API:Imageinfo it explicitly says a "GET request".

This appears to now work as expected. I'm guessing that HAProxy is better at this than Nginx. I don't recall if we verified it on ATS (ats-tls) during the time we used that in front of Varnish-frontend, but either way it seems fine now. Given that after Varnish it goes through ATS (ats-be), I assume ATS is fine with it as well (at least on HTTP/1.1 as used for ats-be).

(Moving task back from Icebox for re-triage to confirm the above and close out, or move back to Icebox as you see fit.