Skip to content

fix(sync-service): send connection: keep-alive only on HTTP/1.x - #4837

Open
Nabil372 wants to merge 2 commits into
electric-sql:mainfrom
Nabil372:fix/sync-service-no-connection-header-on-http2
Open

Nabil372 wants to merge 2 commits into
electric-sql:mainfrom
Nabil372:fix/sync-service-no-connection-header-on-http2

Conversation

@Nabil372

Copy link
Copy Markdown

We run Electric with TanStack DB in production, in web apps and in Expo apps on iOS and Android, and I'm sending upstream fixes for a few server bugs we found in that setup. This one we hit directly: our API proxies shape requests to Electric over h2c, and we currently have to send live SSE requests over a separate HTTP/1.1 connection to get around it.

Problem

Live SSE responses, and OPTIONS responses, always set connection: keep-alive:

defp put_sse_headers(conn, %__MODULE__{params: %{live: true, live_sse: true}}) do
conn
|> Plug.Conn.put_resp_header("content-type", "text/event-stream")
|> Plug.Conn.put_resp_header("connection", "keep-alive")
end

connection is a connection-specific header, and HTTP/2 forbids it (RFC 9113, section 8.2.2): "An endpoint MUST NOT generate an HTTP/2 message containing connection-specific header fields ... Any message containing connection-specific header fields MUST be treated as malformed." Bandit passes the header through on HTTP/2, so when Electric is served over HTTP/2 (for example behind a reverse proxy that talks h2c to it) a conforming client rejects every SSE response.

Long polls on the same connection work, because only the SSE and OPTIONS paths set the header.

Repro

Electric over h2c, with the handle and offset from an earlier request:

curl --http2-prior-knowledge -N \
  'http://localhost:3000/v1/shape?table=items&offset=<offset>&handle=<handle>&live=true&live_sse=true'

curl reports Invalid HTTP header field was received: frame type: 1, stream: 1, name: [connection], value: [keep-alive] and the stream is reset. Node's node:http2 client resets it with PROTOCOL_ERROR before any headers reach the caller, and Electric logs Error while streaming response: :protocol_error.

Fix

Response.put_keep_alive_header/1 adds the header only when Plug.Conn.get_http_protocol/1 is HTTP/1.x. The SSE path and Electric.Shapes.Api.Options both use it, so OPTIONS responses follow the same rule. HTTP/1.x responses are unchanged, and HTTP/2 connections stay open without the header.

Tests

  • serve_shape_plug_test.exs: "omits the connection header from an SSE response on HTTP/2"
  • options_shape_plug_test.exs: "omits the connection header on HTTP/2"

Both set the protocol with Plug.Test.put_http_protocol/2 and fail on main. The existing HTTP/1.1 tests that assert connection: keep-alive still pass unchanged.

cd packages/sync-service
mix test test/electric/plug/options_shape_plug_test.exs test/electric/plug/serve_shape_plug_test.exs
# 57 tests, 0 failures (the two new tests fail on main)
mix format --check-formatted

The connection header is connection-specific, which HTTP/2 forbids
(RFC 9113 section 8.2.2). A conforming HTTP/2 client treats an SSE
response carrying it as malformed and resets the stream, so live SSE
through an h2c reverse proxy never reaches its first event.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant