fix: cap WebSocket fragment buffer at configurable max message size
The mod_websocket continuation-frame patch accumulated fragments in an
unbounded dataBuffer with no size limit, fragment count limit, or
timeout. A buggy or malicious client sending endless continuation
frames without FIN could exhaust memory and degrade the entire server.
Add a configurable websocket_max_message_size option (default 2 MB)
that closes the connection with code 1009 ("Message too big") and
resets the buffer when exceeded. Also add wss-perf-analysis.md
documenting the WebSocket performance and drop investigation.
Part-of: <http://gitlab.vnc.biz/uxf/vnctalk-prosody/-/merge_requests/8>
This commit is contained in:
+1
-1
@@ -21,7 +21,7 @@ To change a patch: extract the pristine tarball, apply all patches, edit, and re
|
||||
| `muc.lib.patch` | `plugins/muc/muc.lib.lua` | (a) `broadcast()` also routes to offline *remote* affiliated members; (b) suppress the unavailable self-presence in `publicise_occupant_status`; (c) let owner/admin/member post to a room without being a present occupant (bypasses the `muc-occupant-groupchat` event and its not-in-room rejection); (d) fire `muc-config-sub-mitted` on config changes — **consumed by an external service**. |
|
||||
| `hidden.lib.patch` | `plugins/muc/hidden.lib.lua` | Hide the "publicly searchable" room config option for everyone when public rooms are restricted (upstream exempts admins). |
|
||||
| `mod_muc_unique.patch` | `plugins/mod_muc_unique.lua` | Rework muc#unique handler; answer `item-not-found` for bare-JID requests. |
|
||||
| `mod_websocket.patch` | `plugins/mod_websocket.lua` | Re-add WebSocket continuation-frame (fragmented-message) support that was removed upstream in the 0.12/13.0 rewrite. VNCtalk clients fragment large WebSocket messages (e.g. vCard sets with avatars >~64 KB); 13.0.6's `validate_frame` hard-rejected any frame with `FIN=false` (close code 1003) **silently** — no log was emitted because the rejection fired on the partial-frame path before `handle_frame` was reached, and `websocket_close()` itself does not log. This patch removes the blanket `not frame.FIN` rejection from `validate_frame` and restores the `dataBuffer` fragment-accumulation logic from 0.11.6 inside `handle_frame`: text frames (opcode 0x1) with `FIN=false` start a buffer, continuation frames (opcode 0x0) append to it, and the concatenated data is returned only when a frame with `FIN=true` arrives. |
|
||||
| `mod_websocket.patch` | `plugins/mod_websocket.lua` | Re-add WebSocket continuation-frame (fragmented-message) support that was removed upstream in the 0.12/13.0 rewrite. VNCtalk clients fragment large WebSocket messages (e.g. vCard sets with avatars >~64 KB); 13.0.6's `validate_frame` hard-rejected any frame with `FIN=false` (close code 1003) **silently** — no log was emitted because the rejection fired on the partial-frame path before `handle_frame` was reached, and `websocket_close()` itself does not log. This patch removes the blanket `not frame.FIN` rejection from `validate_frame` and restores the `dataBuffer` fragment-accumulation logic from 0.11.6 inside `handle_frame`: text frames (opcode 0x1) with `FIN=false` start a buffer, continuation frames (opcode 0x0) append to it, and the concatenated data is returned only when a frame with `FIN=true` arrives. The accumulated size is capped by the configurable `websocket_max_message_size` option (default **2 MB**, 2×1024×1024); if exceeded the connection is closed with WebSocket close code 1009 ("Message too big") and the buffer is reset, preventing unbounded memory growth from buggy or malicious clients that never send a FIN frame. |
|
||||
|
||||
## Patches removed at the 0.12.6 upgrade (M1)
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
--- a/plugins/mod_websocket.lua 2026-07-15 09:48:22.424559760 +0200
|
||||
+++ b/plugins/mod_websocket.lua 2026-07-15 09:49:05.585990882 +0200
|
||||
+++ b/plugins/mod_websocket.lua 2026-07-16 12:00:00.000000000 +0200
|
||||
@@ -199,9 +199,6 @@
|
||||
end
|
||||
|
||||
@@ -10,15 +10,17 @@
|
||||
if opcode == 0x01 and frame.data and frame.data:byte(1, 1) ~= 60 then
|
||||
return false, 1007, "Invalid payload start character, RFC 7395 3.3.3";
|
||||
end
|
||||
@@ -249,6 +246,7 @@
|
||||
@@ -249,6 +246,9 @@
|
||||
end
|
||||
end
|
||||
|
||||
+ local max_message_size = module:get_option_number("websocket_max_message_size", 2 * 1024 * 1024);
|
||||
+ local dataBufferLen = 0;
|
||||
+ local dataBuffer;
|
||||
local function handle_frame(frame)
|
||||
module:log("debug", "Websocket received frame: opcode=%0x, %i bytes", frame.opcode, #frame.data);
|
||||
|
||||
@@ -266,12 +264,31 @@
|
||||
@@ -266,12 +266,44 @@
|
||||
return "";
|
||||
elseif opcode == 0xA then -- Pong frame, MAY be sent unsolicited, eg as keepalive
|
||||
return "";
|
||||
@@ -34,8 +36,20 @@
|
||||
+ end
|
||||
+
|
||||
+ if opcode == 0x0 then -- Continuation frame
|
||||
+ dataBufferLen = dataBufferLen + #frame.data;
|
||||
+ if dataBufferLen > max_message_size then
|
||||
+ dataBuffer = nil;
|
||||
+ dataBufferLen = 0;
|
||||
+ return false, 1009, "Message too big";
|
||||
+ end
|
||||
+ dataBuffer[#dataBuffer+1] = frame.data;
|
||||
+ elseif opcode == 0x1 then -- Text frame
|
||||
+ dataBufferLen = #frame.data;
|
||||
+ if dataBufferLen > max_message_size then
|
||||
+ dataBuffer = nil;
|
||||
+ dataBufferLen = 0;
|
||||
+ return false, 1009, "Message too big";
|
||||
+ end
|
||||
+ dataBuffer = { frame.data };
|
||||
+ else
|
||||
log("warn", "Received frame with unsupported opcode %i", opcode);
|
||||
@@ -46,6 +60,7 @@
|
||||
+ if frame.FIN then
|
||||
+ local data = t_concat(dataBuffer, "");
|
||||
+ dataBuffer = nil;
|
||||
+ dataBufferLen = 0;
|
||||
+ return data;
|
||||
+ end
|
||||
+ return "";
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
# WebSocket Performance & Drop Analysis
|
||||
|
||||
## Executive Summary
|
||||
|
||||
User reports of **slow WebSocket connections** and **sudden drops** on the Prosody 13.0.6 installation have three root causes in this repo:
|
||||
|
||||
1. **Synchronous PostgreSQL reads on every message stanza** (`mod_vnc_fcm`, `mod_vnc_muc_fcm`) block Prosody's single event loop, causing global latency spikes.
|
||||
2. **Unbounded WebSocket fragment buffer** (`mod_websocket.patch`) allows unbounded memory growth per connection with no timeout or size limit.
|
||||
3. **Missing transport keepalive** means NATs, ingress controllers, and load balancers silently drop idle WebSocket connections after 60–300 s.
|
||||
|
||||
Secondary factors: SMACKS session limits are tight for mobile churn, and `trusted_proxies` is misconfigured for Kubernetes.
|
||||
|
||||
---
|
||||
|
||||
## Findings
|
||||
|
||||
### 1. Synchronous DB reads on the message hot path (HIGH IMPACT)
|
||||
|
||||
**Location:** `vnctalk/mod_vnc_fcm.lua`, `vnctalk/mod_vnc_muc_fcm.lua`
|
||||
|
||||
Both modules hook every `message/bare`, `message/full`, `pre-message/*`, and `muc-broadcast-message` event at priority 2. On **every** stanza they perform synchronous SQL lookups:
|
||||
|
||||
- `private_storage:get(userName)`
|
||||
- `fcm_token_store:get(userName)`
|
||||
- `vcard_storage:get(userName)` (via `getDisplayName()`)
|
||||
|
||||
For MUC messages, `mod_vnc_muc_fcm` iterates over **all room affiliations** and repeats the above lookups per affiliate. A single message in a 100-member room can trigger **hundreds** of blocking PostgreSQL queries inside Prosody's single-threaded Lua event loop.
|
||||
|
||||
**Result:** Any slowdown in PostgreSQL or spike in message volume blocks *all* connections (WebSocket, BOSH, c2s). Clients see latency, stalls, and eventual timeouts/drops.
|
||||
|
||||
### 2. Unbounded WebSocket fragment buffer (HIGH IMPACT — FIXED)
|
||||
|
||||
**Location:** `patches/mod_websocket.patch`
|
||||
|
||||
Upstream 13.0.6 removed continuation-frame support. The patch restores it by accumulating fragments in a `dataBuffer` table local to each connection. The original patch had **no** maximum size, fragment count limit, or timeout.
|
||||
|
||||
**Fix applied:** The patch now caps the accumulated message size via the configurable `websocket_max_message_size` option (default **2 MB**). If exceeded, the connection is closed with WebSocket close code 1009 ("Message too big") and the buffer is reset. This prevents unbounded memory growth from buggy or malicious clients that never send a FIN frame.
|
||||
|
||||
### 3. Missing WebSocket keepalive (HIGH IMPACT — easy fix)
|
||||
|
||||
**Location:** `config/prosody.cfg.lua.template`
|
||||
|
||||
The template has no keepalive configuration for the WebSocket transport:
|
||||
|
||||
- No `ping_interval` for `mod_ping` (loaded but only replies to client pings by default).
|
||||
- No transport-level WebSocket ping/pong interval from Prosody.
|
||||
- No TCP keepalive tuning.
|
||||
|
||||
Kubernetes ingress controllers (nginx, Traefik, AWS ALB) and corporate NATs typically drop idle TCP/WebSocket connections after **60–300 s**. When a client is idle (e.g., reading a chat), the connection is silently closed by the intermediary. The client sees a "sudden drop" only when it next tries to send.
|
||||
|
||||
### 4. SMACKS session limits too tight for mobile churn (MEDIUM IMPACT)
|
||||
|
||||
**Location:** `config/prosody.cfg.lua.template`, `config/startup.sh`
|
||||
|
||||
Defaults set in `startup.sh`:
|
||||
|
||||
```
|
||||
smacks_hibernation_time = 300
|
||||
smacks_max_old_sessions = 10
|
||||
```
|
||||
|
||||
Mobile clients frequently background/foreground and reconnect. If a user has more than 10 old sessions within the 5-minute hibernation window, the oldest are evicted. The client must perform a full rebind instead of a fast SMACKS resume, which feels like a slow reconnect or a drop.
|
||||
|
||||
### 5. trusted_proxies misconfigured for Kubernetes (LOW DIRECT IMPACT)
|
||||
|
||||
**Location:** `config/prosody.cfg.lua.template`
|
||||
|
||||
```lua
|
||||
trusted_proxies = { "127.0.0.1" }
|
||||
```
|
||||
|
||||
In Kubernetes the ingress controller is **not** localhost. Prosody ignores `X-Forwarded-For` and sees every client as the ingress IP. This breaks IP-based diagnostics and could interact with future rate-limiting, but does not directly cause drops today.
|
||||
|
||||
---
|
||||
|
||||
## Recommended Fixes
|
||||
|
||||
### Immediate (low effort, high reward)
|
||||
|
||||
1. **Enable proactive XMPP pings** to keep the WebSocket alive through NATs/LBs. Add to the global config:
|
||||
```lua
|
||||
ping_interval = 60
|
||||
ping_timeout = 120
|
||||
```
|
||||
Also align the ingress controller idle timeout (e.g., nginx `proxy-read-timeout`) to be > 120 s.
|
||||
|
||||
2. **Add bounds to `mod_websocket.patch`** — **DONE**. The patch now enforces a configurable `websocket_max_message_size` (default 2 MB). Connections exceeding the limit are closed with code 1009 and the buffer is reset.
|
||||
|
||||
3. **Raise SMACKS limits for mobile clients**. In `startup.sh` or the template:
|
||||
```lua
|
||||
smacks_max_old_sessions = 50
|
||||
```
|
||||
Consider whether `smacks_hibernation_time = 300` is appropriate; some mobile deployments use 600–1800 s.
|
||||
|
||||
### Short-term (requires code changes)
|
||||
|
||||
4. **Cache FCM token lookups in memory**. The `mod_vnc_fcm` modules read the same private-data / token rows repeatedly. Add an in-memory LRU cache (e.g., 5-minute TTL) around:
|
||||
- `getNotifyOptionsForUser()`
|
||||
- `getDisplayName()`
|
||||
|
||||
This avoids synchronous PostgreSQL round-trips on the hot path.
|
||||
|
||||
5. **Defer or batch MUC FCM notifications**. In `mod_vnc_muc_fcm`, iterating all affiliations and doing a DB read + HTTP request per member inside `muc-broadcast-message` blocks the room. Move the loop into an async timer or a background queue so the broadcast event returns immediately.
|
||||
|
||||
6. **Fix `trusted_proxies`**. Set it to the cluster CIDR(s) or the ingress controller's source IPs, e.g.:
|
||||
```lua
|
||||
trusted_proxies = { "127.0.0.1", "10.0.0.0/8" }
|
||||
```
|
||||
|
||||
### Diagnostic
|
||||
|
||||
7. **Watch `log_slow_events`**. The module is enabled with a 1.5 s threshold. If PostgreSQL or the FCM path is the bottleneck, the logs will show `Slow event 'message/bare'` or `Slow event 'muc-broadcast-message'`. Use this to confirm fix #4/#5 before and after.
|
||||
|
||||
---
|
||||
|
||||
## Files Involved
|
||||
|
||||
| File | Relevance |
|
||||
|------|-----------|
|
||||
| `patches/mod_websocket.patch` | Unbounded fragment accumulation |
|
||||
| `vnctalk/mod_vnc_fcm.lua` | Synchronous DB reads on every message |
|
||||
| `vnctalk/mod_vnc_muc_fcm.lua` | Synchronous DB reads + loop over MUC affiliations |
|
||||
| `config/prosody.cfg.lua.template` | Missing `ping_interval`, wrong `trusted_proxies` |
|
||||
| `config/startup.sh` | `SMACKS_MAX_UNACKED_STANZAS`, `SMACKS_HIBERNATION_TIME` defaults |
|
||||
| `vnctalk/mod_log_slow_events.lua` | Already logging; verify thresholds |
|
||||
Reference in New Issue
Block a user