nchan_websocket_client_heartbeat silently broken in subscriber-only locations

[gunjanjp]

Description

nchan_websocket_client_heartbeat does not work in subscriber-only locations (nchan_subscriber; without nchan_publisher;). The server never sends the configured heartbeat response and instead forcibly closes the WebSocket connection with a 1008 (policy violation) error.

This affects the most common nchan configuration — subscriber-only locations are the standard pattern for WebSocket subscription endpoints.

Environment

• nchan version: 1.3.8 (also verified on current master — code is identical)
• nginx version: 1.30.4
• OS: Debian (also reproduced on Ubuntu)

Steps to Reproduce

1. Configure a subscriber-only location with client heartbeat:

location /sub/channel {
    nchan_subscriber;
    nchan_channel_id test;
    nchan_websocket_client_heartbeat _ping _pong;
}

2. Connect via WebSocket and send the heartbeat message:

import asyncio, websockets

async def test():
    async with websockets.connect("wss://example.com/sub/channel") as ws:
        await ws.send("_ping")
        response = await asyncio.wait_for(ws.recv(), timeout=5)
        print(response)  # Expected: "_pong"

asyncio.run(test())

3. Expected: Server responds with _pong
4. Actual: Connection closed with 1008 (policy violation) Publish; Publishing not allowed.

Root Cause

In src/subscribers/websocket.c, the websocket_reading() function handles incoming text/binary frames in this order:

case WEBSOCKET_OPCODE_TEXT:
case WEBSOCKET_OPCODE_BINARY:

    // Line ~1485: Guard runs FIRST
    if(!fsub->sub.cf->pub.websocket) {
        websocket_send_close_frame_cstr(fsub, CLOSE_POLICY_VIOLATION, "Publishing not allowed.");
        return websocket_reading_finalize(r);    // <-- connection killed here
    }

    // ... payload parsing, unmasking, UTF-8 validation ...

    // Line ~1548: Heartbeat check runs AFTER (never reached)
    if(websocket_heartbeat(fsub, msgbuf) != NGX_OK) {
        websocket_publish(fsub, msgbuf, ...);
    }

The pub.websocket check at line ~1485 rejects all incoming text frames in subscriber-only locations before the heartbeat handler at line ~1548 ever gets a chance to match the message. The heartbeat response mechanism is never invoked.

Why it works in bidirectional locations

When the location has both nchan_subscriber; and nchan_publisher;, pub.websocket is true, the guard passes, and the code reaches the websocket_heartbeat() call. This is why testing with a bidirectional location appears to work.

Workaround

Adding nchan_publisher; to the subscriber location makes the heartbeat work, but this also enables arbitrary publishing from WebSocket clients — a significant security concern for most configurations.

Proposed Fix

Move the pub.websocket guard to run after the heartbeat check:

case WEBSOCKET_OPCODE_TEXT:
case WEBSOCKET_OPCODE_BINARY:

    // ... payload parsing, unmasking, UTF-8 validation ...

    if(websocket_heartbeat(fsub, msgbuf) != NGX_OK) {
        // Only check pub permission for non-heartbeat messages
        if(!fsub->sub.cf->pub.websocket) {
            ws_destroy_msgpool(fsub);
            websocket_send_close_frame_cstr(fsub, CLOSE_POLICY_VIOLATION, "Publishing not allowed.");
            return websocket_reading_finalize(r);
        }
        websocket_publish(fsub, msgbuf, ...);
    }
    else {
        ws_destroy_msgpool(fsub);
    }

This way:

• Heartbeat messages are handled regardless of pub mode (no publish involved — they are purely a keepalive mechanism)
• Non-heartbeat messages are still correctly rejected in subscriber-only locations
• Bidirectional locations are unaffected

A fix is included in PR #716.

Impact

Any deployment using nchan_websocket_client_heartbeat in a subscriber-only location (the standard configuration) will find the feature completely non-functional. Clients sending heartbeat messages will have their connections killed, which is the opposite of the intended keepalive behavior.