Skip to content

Commit c5dc734

Browse files
authored
Fix issue with requests received before srv_conf section is loaded (#7)
* WIP: Fix recv processing Do not bill requests if they happened during config reload * WIP: Fix recv processing Added debug in case we do not billed ws request * WIP: Fix recv processing Added debug error in case we do not billed ws request * WIP: Fix recv processing Debugging * WIP: Fix recv processing Debugging * WIP: Fix recv processing Final form * WIP: Fix recv processing Fix tests
1 parent df2773f commit c5dc734

File tree

5 files changed

+20
-12
lines changed

5 files changed

+20
-12
lines changed

docker-compose.yaml

+3-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ services:
88
volumes:
99
- ./docker/nginx.conf:/etc/nginx/nginx.conf
1010
- logs:/var/log/nginx
11-
11+
echo:
12+
image: jmalloc/echo-server:0.3.3
1213
test_e2e:
1314
build:
1415
context: .
@@ -20,7 +21,7 @@ services:
2021
- logs:/var/log/nginx
2122
depends_on:
2223
- nginx
23-
24+
- echo
2425
test:
2526
build:
2627
context: .

docker/Dockerfile.e2e

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
FROM rust:alpine3.13
1+
FROM rust:1.63.0-alpine3.16
22

33
USER root
44

5-
RUN apk add --no-cache musl-dev && cargo install websocat
5+
RUN apk add --no-cache musl-dev openssl-dev pkgconfig && cargo install websocat
66

77
COPY ./docker/test_e2e.sh /usr/local/bin/test_e2e
88
RUN chmod +x /usr/local/bin/test_e2e

docker/nginx.conf

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ http {
1414
location / {
1515
proxy_buffering off;
1616
proxy_request_buffering off;
17-
proxy_pass https://echo.websocket.org;
17+
proxy_pass http://echo:8080/;
1818
proxy_ssl_server_name on;
1919
proxy_http_version 1.1;
2020
proxy_set_header Connection "upgrade";

docker/test_e2e.sh

+4-5
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,12 @@ set -exo pipefail
66

77
cat <<EOF > /tmp/ws-expected.log
88
OPEN
9-
CLIENT->SERVER: 1311 (websocket)
10-
SERVER->CLIENT: 1311 (websocket)
9+
SERVER->CLIENT: 30 (websocket)
10+
CLIENT->SERVER: 7 (websocket)
11+
SERVER->CLIENT: 7 (websocket)
1112
CLOSE
1213
EOF
1314

14-
echo "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. Sed ut perspiciatis unde omnis iste natus error sit voluptatem accusantium doloremque laudantium, totam rem aperiam, eaque ipsa quae ab illo inventore veritatis et quasi architecto beatae vitae dicta sunt explicabo. Nemo enim ipsam voluptatem quia voluptas sit aspernatur aut odit aut fugit, sed quia consequuntur magni dolores eos qui ratione voluptatem sequi nesciunt. Neque porro quisquam est, qui dolorem ipsum quia dolor sit amet, consectetur, adipisci velit, sed quia non numquam eius modi tempora incidunt ut labore et dolore magnam aliquam quaerat voluptatem. Ut enim ad minima veniam, quis nostrum exercitationem ullam corporis suscipit laboriosam, nisi ut aliquid ex ea commodi consequatur? Quis autem vel eum iure reprehenderit qui in ea voluptate velit esse quam nihil molestiae consequatur, vel illum qui dolorem eum fugiat quo voluptas nulla pariatur?" \
15-
| tr -d \\n \
16-
| websocat --text -1 ws://${ENDPOINT}
15+
echo "asdasd" | websocat --text ws://${ENDPOINT}
1716

1817
diff ${LOG_FILE_PATH} /tmp/ws-expected.log

src/ngx_http_websocket_stat_module.c

+10-2
Original file line numberDiff line numberDiff line change
@@ -182,13 +182,21 @@ ssize_t
182182
my_recv(ngx_connection_t *c, u_char *buf, size_t size)
183183
{
184184
ngx_http_request_t *r = c->data;
185-
ngx_http_websocket_srv_conf_t *srvcf = ngx_http_get_module_srv_conf(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);
185+
187186

188187
int n = orig_recv(c, buf, size);
189188
if (n <= 0) {
190189
return n;
191190
}
191+
if (r->srv_conf == NULL || r->ctx == NULL) {
192+
// Related to the bug when we calculated request right after reload and server config yet to load so it was NULL and caused crash.
193+
// Part r->ctx == NULL allows to prevents same problem in case module_ctx would be unloaded for some reason.
194+
// This message would would be not shown usually as all (or all i've seen) problematic requests had zero size
195+
ngx_log_error(NGX_LOG_NOTICE, ngx_cycle->log, 0, "Websocket package was processed without billing due to issue CORE-4874.");
196+
return n;
197+
}
198+
ngx_http_websocket_srv_conf_t *srvcf = ngx_http_get_module_srv_conf(r, ngx_http_websocket_stat_module);
199+
ngx_http_websocket_stat_request_ctx *request_ctx = ngx_http_get_module_ctx(r, ngx_http_websocket_stat_module);
192200

193201
ssize_t sz = n;
194202
template_ctx_s template_ctx;

0 commit comments

Comments
 (0)