Skip to content

Commit 71e9d01

Browse files
zaslavskiizaslavskii
authored
Merge pull request #5 from chainstack/bug/concurrent-requests-corrupt-the-message-size-calculation
Add separate request contexts for server events and client events
2 parents 0588ca1 + cce7dba commit 71e9d01

File tree

2 files changed

+28
-29
lines changed

2 files changed

+28
-29
lines changed

‎src/ngx_http_websocket_stat_frame_counter.c‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -73,23 +73,16 @@ frame_counter_process_message(u_char **buffer, ssize_t *size,
7373
break;
7474
case PAYLOAD_LEN_LARGE:
7575
case PAYLOAD_LEN_HUGE: {
76-
int i;
77-
if (frame_counter->stage == PAYLOAD_LEN_LARGE) {
78-
assert(*size >= 2);
79-
i = 2;
80-
} else {
81-
assert(*size >= 8);
82-
i = 8;
76+
frame_counter->bytes_consumed++;
77+
frame_counter->current_payload_size <<= 8;
78+
frame_counter->current_payload_size |= **buffer;
79+
if ((frame_counter->stage == PAYLOAD_LEN_LARGE && frame_counter->bytes_consumed == 2) ||
80+
(frame_counter->stage == PAYLOAD_LEN_HUGE && frame_counter->bytes_consumed == 8)) {
81+
frame_counter->current_message_size += frame_counter->current_payload_size;
82+
frame_counter->stage = frame_counter->payload_masked ? MASK : PAYLOAD;
83+
frame_counter->bytes_consumed = 0;
8384
}
84-
do {
85-
frame_counter->current_payload_size <<= 8;
86-
frame_counter->current_payload_size |= **buffer;
87-
move_buffer(buffer, size, 1);
88-
} while (--i);
89-
frame_counter->current_message_size += frame_counter->current_payload_size;
90-
frame_counter->stage =
91-
frame_counter->payload_masked ? MASK : PAYLOAD;
92-
frame_counter->bytes_consumed = 0;
85+
move_buffer(buffer, size, 1);
9386
break;
9487
}
9588
case MASK:

‎src/ngx_http_websocket_stat_module.c‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ typedef struct {
1515
ngx_frame_counter_t frame_counter;
1616
} ngx_http_websocket_stat_ctx;
1717

18+
typedef struct {
19+
ngx_http_websocket_stat_ctx recv_ctx;
20+
ngx_http_websocket_stat_ctx send_ctx;
21+
} ngx_http_websocket_stat_request_ctx;
22+
1823
typedef struct {
1924
char from_client : 1;
2025
ngx_http_websocket_stat_ctx *ws_ctx;
@@ -145,15 +150,15 @@ my_send(ngx_connection_t *c, u_char *buf, size_t size)
145150
{
146151
ngx_http_request_t *r = c->data;
147152
ngx_http_websocket_srv_conf_t *srvcf = ngx_http_get_module_srv_conf(r, ngx_http_websocket_stat_module);
148-
ngx_http_websocket_stat_ctx *ctx = ngx_http_get_module_ctx(r, ngx_http_websocket_stat_module);
153+
ngx_http_websocket_stat_request_ctx *request_ctx = ngx_http_get_module_ctx(r, ngx_http_websocket_stat_module);
149154

150-
if (check_ws_age(ctx->ws_conn_start_time, r) != NGX_OK) {
155+
if (check_ws_age(request_ctx->send_ctx.ws_conn_start_time, r) != NGX_OK) {
151156
return NGX_ERROR;
152157
}
153158

154159
template_ctx_s template_ctx;
155160
template_ctx.from_client = 0;
156-
template_ctx.ws_ctx = ctx;
161+
template_ctx.ws_ctx = &request_ctx->send_ctx;
157162

158163
int n = orig_send(c, buf, size);
159164
if (n <= 0) {
@@ -164,7 +169,7 @@ my_send(ngx_connection_t *c, u_char *buf, size_t size)
164169
ssize_t sz = n;
165170

166171
while (sz > 0) {
167-
if (frame_counter_process_message(&buf, &sz, &(ctx->frame_counter))) {
172+
if (frame_counter_process_message(&buf, &sz, &request_ctx->send_ctx.frame_counter)) {
168173
ws_do_log(get_ws_log_template(&template_ctx, srvcf), r, &template_ctx);
169174
}
170175
}
@@ -178,7 +183,7 @@ my_recv(ngx_connection_t *c, u_char *buf, size_t size)
178183
{
179184
ngx_http_request_t *r = c->data;
180185
ngx_http_websocket_srv_conf_t *srvcf = ngx_http_get_module_srv_conf(r, ngx_http_websocket_stat_module);
181-
ngx_http_websocket_stat_ctx *ctx = ngx_http_get_module_ctx(r, ngx_http_websocket_stat_module);
186+
ngx_http_websocket_stat_request_ctx *request_ctx = ngx_http_get_module_ctx(r, ngx_http_websocket_stat_module);
182187

183188
int n = orig_recv(c, buf, size);
184189
if (n <= 0) {
@@ -188,14 +193,14 @@ my_recv(ngx_connection_t *c, u_char *buf, size_t size)
188193
ssize_t sz = n;
189194
template_ctx_s template_ctx;
190195
template_ctx.from_client = 1;
191-
template_ctx.ws_ctx = ctx;
196+
template_ctx.ws_ctx = &request_ctx->recv_ctx;
192197

193-
if (check_ws_age(ctx->ws_conn_start_time, r) != NGX_OK) {
198+
if (check_ws_age(request_ctx->recv_ctx.ws_conn_start_time, r) != NGX_OK) {
194199
return NGX_ERROR;
195200
}
196201

197202
while (sz > 0) {
198-
if (frame_counter_process_message(&buf, &sz, &ctx->frame_counter)) {
203+
if (frame_counter_process_message(&buf, &sz, &request_ctx->recv_ctx.frame_counter)) {
199204
ws_do_log(get_ws_log_template(&template_ctx, srvcf), r, &template_ctx);
200205
}
201206
}
@@ -224,23 +229,24 @@ ngx_http_websocket_stat_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
224229
if (r->headers_in.upgrade) {
225230
if (r->upstream->peer.connection) {
226231
// connection opened
227-
ngx_http_websocket_stat_ctx *ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_websocket_stat_ctx));
232+
ngx_http_websocket_stat_request_ctx *ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_websocket_stat_request_ctx));
228233
if (!ctx) {
229234
return NGX_HTTP_INTERNAL_SERVER_ERROR;
230235
}
231236
ngx_http_set_ctx(r, ctx, ngx_http_websocket_stat_module);
232237
template_ctx_s template_ctx;
233-
template_ctx.ws_ctx = ctx;
238+
template_ctx.ws_ctx = &ctx->recv_ctx;
234239
ws_do_log(srvcf->log_open_template, r, &template_ctx);
235240
orig_recv = r->connection->recv;
236241
r->connection->recv = my_recv;
237242
orig_send = r->connection->send;
238243
r->connection->send = my_send;
239-
ctx->ws_conn_start_time = ngx_time();
244+
ctx->recv_ctx.ws_conn_start_time = ngx_time();
245+
ctx->send_ctx.ws_conn_start_time = ctx->recv_ctx.ws_conn_start_time;
240246
} else {
241-
ngx_http_websocket_stat_ctx *ctx = ngx_http_get_module_ctx(r, ngx_http_websocket_stat_module);
247+
ngx_http_websocket_stat_request_ctx *ctx = ngx_http_get_module_ctx(r, ngx_http_websocket_stat_module);
242248
template_ctx_s template_ctx;
243-
template_ctx.ws_ctx = ctx;
249+
template_ctx.ws_ctx = &ctx->recv_ctx;
244250
ws_do_log(srvcf->log_close_template, r, &template_ctx);
245251
}
246252
}

0 commit comments

Comments
 (0)