Skip to content

Commit

Permalink
Fix MSC4108 'rendez-vous' responses with some reverse proxy in the fr…
Browse files Browse the repository at this point in the history
…ont of Synapse (#18178)

MSC4108 relies on ETag to determine if something has changed on the
rendez-vous channel.
Strong and correct ETag comparison works if the response body is
bit-for-bit identical, which isn't the case if a proxy in the middle
compresses the response on the fly.

This adds a `no-transform` directive to the `Cache-Control` header,
which tells proxies not to transform the response body.

Additionally, some proxies (nginx) will switch to `Transfer-Encoding:
chunked` if it doesn't know the Content-Length of the response, and
'weakening' the ETag if that's the case. I've added `Content-Length`
headers to all responses, to hopefully solve that.

This basically fixes QR-code login when nginx or cloudflare is involved,
with gzip/zstd/deflate compression enabled.
  • Loading branch information
sandhose authored Feb 25, 2025
1 parent a5c3fe6 commit b9276e2
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog.d/18178.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix MSC4108 QR-code login not working with some reverse-proxy setups.
6 changes: 5 additions & 1 deletion rust/src/rendezvous/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ fn prepare_headers(headers: &mut HeaderMap, session: &Session) {
headers.typed_insert(AccessControlAllowOrigin::ANY);
headers.typed_insert(AccessControlExposeHeaders::from_iter([ETAG]));
headers.typed_insert(Pragma::no_cache());
headers.typed_insert(CacheControl::new().with_no_store());
headers.typed_insert(CacheControl::new().with_no_store().with_no_transform());
headers.typed_insert(session.etag());
headers.typed_insert(session.expires());
headers.typed_insert(session.last_modified());
Expand Down Expand Up @@ -192,10 +192,12 @@ impl RendezvousHandler {
"url": uri,
})
.to_string();
let length = response.len() as _;

let mut response = Response::new(response.as_bytes());
*response.status_mut() = StatusCode::CREATED;
response.headers_mut().typed_insert(ContentType::json());
response.headers_mut().typed_insert(ContentLength(length));
prepare_headers(response.headers_mut(), &session);
http_response_to_twisted(twisted_request, response)?;

Expand Down Expand Up @@ -299,6 +301,7 @@ impl RendezvousHandler {
// proxy/cache setup which strips the ETag header if there is no Content-Type set.
// Specifically, we noticed this behaviour when placing Synapse behind Cloudflare.
response.headers_mut().typed_insert(ContentType::text());
response.headers_mut().typed_insert(ContentLength(0));

http_response_to_twisted(twisted_request, response)?;

Expand All @@ -316,6 +319,7 @@ impl RendezvousHandler {
response
.headers_mut()
.typed_insert(AccessControlAllowOrigin::ANY);
response.headers_mut().typed_insert(ContentLength(0));
http_response_to_twisted(twisted_request, response)?;

Ok(())
Expand Down
6 changes: 4 additions & 2 deletions tests/rest/client/test_rendezvous.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,11 @@ def test_msc4108(self) -> None:
headers = dict(channel.headers.getAllRawHeaders())
self.assertIn(b"ETag", headers)
self.assertIn(b"Expires", headers)
self.assertIn(b"Content-Length", headers)
self.assertEqual(headers[b"Content-Type"], [b"application/json"])
self.assertEqual(headers[b"Access-Control-Allow-Origin"], [b"*"])
self.assertEqual(headers[b"Access-Control-Expose-Headers"], [b"etag"])
self.assertEqual(headers[b"Cache-Control"], [b"no-store"])
self.assertEqual(headers[b"Cache-Control"], [b"no-store, no-transform"])
self.assertEqual(headers[b"Pragma"], [b"no-cache"])
self.assertIn("url", channel.json_body)
self.assertTrue(channel.json_body["url"].startswith("https://"))
Expand All @@ -141,9 +142,10 @@ def test_msc4108(self) -> None:
self.assertEqual(headers[b"ETag"], [etag])
self.assertIn(b"Expires", headers)
self.assertEqual(headers[b"Content-Type"], [b"text/plain"])
self.assertEqual(headers[b"Content-Length"], [b"7"])
self.assertEqual(headers[b"Access-Control-Allow-Origin"], [b"*"])
self.assertEqual(headers[b"Access-Control-Expose-Headers"], [b"etag"])
self.assertEqual(headers[b"Cache-Control"], [b"no-store"])
self.assertEqual(headers[b"Cache-Control"], [b"no-store, no-transform"])
self.assertEqual(headers[b"Pragma"], [b"no-cache"])
self.assertEqual(channel.text_body, "foo=bar")

Expand Down

0 comments on commit b9276e2

Please sign in to comment.