HTTP/1.1 connection reuse returns unsolicited server data as the next request's response (regression in 8.22.0)

How it was found

Since 8.22.0 libcurl reuses an idle HTTP/1.1 connection that already has unread bytes waiting on it, as long as the pool health check ran less than a second ago, and those bytes are then returned as the response to the next request sent on that connection, so every later response on it is shifted by one. The cause is lib/conncache.c Curl_cpool_conn_seems_healthy(), line 954 in curl-8_22_0: if(curlx_ptimediff_ms(pnow, &conn->lastchecked) < 1000) return TRUE; skips the input_pending check a few lines below it, the one whose own comment says such a connection is not healthy; it came in with 2f1e94eaed (#22169) and 8.21.0 ran that check on every reuse (url.c line 634, "connection has input pending, not reusable"). RFC 9112 section 9.2 says a client MUST NOT consider data received with no outstanding request a valid response and SHOULD close the connection. Reproduced on release builds of curl-8_22_0, rc-8_23_0-3 and master 15ad1b8565 over http and https, easy and multi interface and the command line tool; 8.21.0 opens a new connection every time, without the stray response both versions reuse normally, and on 8.22.0 a 900 ms gap between requests is poisoned while 1100 ms is refused. Steps: save the server below as stray.py, run python3 stray.py 8310, then run curl --rate 5/s http://127.0.0.1:8310/req1 http://127.0.0.1:8310/req2 http://127.0.0.1:8310/req3 -o r1 -o r2 -o r3 and read the three files: 8.22.0 gives r1=RESP1 r2=STRAY r3=RESP2 while the server log shows it received and answered /req2 and /req3 normally on that one connection, 8.21.0 gives RESP1 three times over three connections. Restoring the pending input poll for idle non multiplexed connections and keeping the one second limit only for the protocol liveness callback fixes it on 8.22.0 and master in my tests, i have a patch for both.

stray.py (answers request N with RESPN, then sends one unsolicited response 50 ms after request 1 is done):

import socket, threading, time, sys port = int(sys.argv[1]) s = socket.socket(); s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) s.bind(('127.0.0.1', port)); s.listen(5) def handle(c): n = 0; buf = b'' while True: while b'\r\n\r\n' not in buf: d = c.recv(65536) if not d: return buf += d req, buf = buf.split(b'\r\n\r\n', 1); n += 1 print('REQ %d %s' % (n, req.split(b'\r\n')[0].decode()), flush=True) body = b'RESP%d' % n c.sendall(b'HTTP/1.1 200 OK\r\nContent-Length: %d\r\n\r\n%s' % (len(body), body)) if n == 1: time.sleep(0.05) c.sendall(b'HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\nSTRAY') while True: c, _ = s.accept(); threading.Thread(target=handle, args=(c,), daemon=True).start()

Ai helped mostly with script writing for findings

Impact

The application silently gets a response that was never the answer to its request and from then on every response on that connection belongs to the previous request, so whoever can place bytes on the idle connection before the next request picks the status, headers such as Set-Cookie or Location, and body that the application attributes to it. This is the client side desync class that browsers and 8.21.0 defend against by dropping such connections, it matters most where the stray bytes come from a desynced intermediary or a forwarding proxy connection shared by several origins, and it needs no option, logs nothing and is hit by ordinary sequential use.