From 9db1574f3e614b795787aca21a1e7eaccef0ba36 Mon Sep 17 00:00:00 2001 From: Dmitry Volyntsev Date: Tue, 15 Sep 2026 17:35:30 -0700 Subject: [PATCH] Fetch: honor response body framing Previously, fetch inferred body completion from Content-Length alone. This completed close-delimited responses before EOF and conflated framing modes. The fix is to track body framing explicitly, reject unsupported 1xx responses, and exempt 204 and 304 representation lengths from body limits. --- nginx/ngx_js_http.c | 98 +++++++++++++++++++++++++++++------- nginx/ngx_js_http.h | 1 + nginx/t/js_fetch.t | 31 +++++++++++- nginx/t/js_fetch_keepalive.t | 23 ++++++++- 4 files changed, 131 insertions(+), 22 deletions(-) diff --git a/nginx/ngx_js_http.c b/nginx/ngx_js_http.c index 9e70ebc2..43984c1e 100644 --- a/nginx/ngx_js_http.c +++ b/nginx/ngx_js_http.c @@ -23,6 +23,14 @@ typedef struct { } ngx_js_http_keepalive_cache_t; +enum { + NGX_JS_HTTP_BODY_NONE = 0, + NGX_JS_HTTP_BODY_LENGTH, + NGX_JS_HTTP_BODY_CHUNKED, + NGX_JS_HTTP_BODY_CLOSE, +}; + + #define ngx_js_http_version(major, minor) ((major) * 1000 + (minor)) #define NGX_JS_USER_AGENT "nginx-js" @@ -745,7 +753,7 @@ ngx_js_http_read_handler(ngx_event_t *rev) } if (rc == NGX_DONE) { - break; + return; } continue; @@ -893,7 +901,7 @@ ngx_js_http_process_headers(ngx_js_http_t *http) if (len == (sizeof("Content-Length") - 1) && ngx_strncasecmp(hp->header_name_start, - (u_char *) "Content-Length", len) == 0) + (u_char *) "Content-Length", len) == 0) { content_length_n = ngx_atoof(hp->header_start, vlen); if (content_length_n == NGX_ERROR) { @@ -912,6 +920,8 @@ ngx_js_http_process_headers(ngx_js_http_t *http) http->content_length_n = content_length_n; if (!http->header_only + && http->response.code != 204 + && http->response.code != 304 && http->content_length_n > (off_t) http->max_response_body_size) { @@ -945,8 +955,28 @@ ngx_js_http_process_headers(ngx_js_http_t *http) return NGX_ERROR; } + if (http->response.code >= 100 && http->response.code < 200) { + ngx_js_http_error(http, "unsupported http response status"); + return NGX_ERROR; + } + njs_chb_destroy(&http->chain); + if (http->header_only || http->response.code == 204 + || http->response.code == 304) + { + http->body = NGX_JS_HTTP_BODY_NONE; + + } else if (http->chunked) { + http->body = NGX_JS_HTTP_BODY_CHUNKED; + + } else if (http->content_length) { + http->body = NGX_JS_HTTP_BODY_LENGTH; + + } else { + http->body = NGX_JS_HTTP_BODY_CLOSE; + } + http->process = ngx_js_http_process_body; return http->process(http); @@ -970,18 +1000,19 @@ ngx_js_http_process_body(ngx_js_http_t *http) return NGX_ERROR; } - if (!http->header_only - && http->chunked - && http->content_length_n == -1) + if (http->body == NGX_JS_HTTP_BODY_NONE + || http->body == NGX_JS_HTTP_BODY_CLOSE) { + http->ready_handler(http); + return NGX_DONE; + } + + if (http->body == NGX_JS_HTTP_BODY_CHUNKED) { ngx_js_http_error(http, "invalid http chunked response"); return NGX_ERROR; } - if (http->header_only - || http->content_length_n == -1 - || size == http->content_length_n) - { + if (size == http->content_length_n) { http->ready_handler(http); return NGX_DONE; } @@ -996,7 +1027,12 @@ ngx_js_http_process_body(ngx_js_http_t *http) b = http->buffer; - if (http->chunked) { + if (http->body == NGX_JS_HTTP_BODY_NONE) { + http->ready_handler(http); + return NGX_DONE; + } + + if (http->body == NGX_JS_HTTP_BODY_CHUNKED) { rc = ngx_js_http_parse_chunked(&http->http_chunk_parse, b, &http->response.chain); if (rc == NGX_ERROR) { @@ -1005,9 +1041,16 @@ ngx_js_http_process_body(ngx_js_http_t *http) } size = njs_chb_size(&http->response.chain); + if (size < 0) { + ngx_js_http_error(http, "memory error"); + return NGX_ERROR; + } if (rc == NGX_OK) { - http->content_length_n = size; + b->pos = http->http_chunk_parse.pos; + + http->ready_handler(http); + return NGX_DONE; } if (size > http->max_response_body_size * 10) { @@ -1019,20 +1062,27 @@ ngx_js_http_process_body(ngx_js_http_t *http) } else { size = njs_chb_size(&http->response.chain); + if (size < 0) { + ngx_js_http_error(http, "memory error"); + return NGX_ERROR; + } - if (http->header_only) { - need = 0; - - } else if (http->content_length_n == -1) { + if (http->body == NGX_JS_HTTP_BODY_CLOSE) { need = http->max_response_body_size - size; + if (b->last - b->pos > need) { + ngx_js_http_error(http, "http response body is too large"); + return NGX_ERROR; + } + + chsize = b->last - b->pos; + } else { need = http->content_length_n - size; + chsize = ngx_min(need, b->last - b->pos); } - chsize = ngx_min(need, b->last - b->pos); - - if (size + chsize > http->max_response_body_size) { + if (need < 0) { ngx_js_http_error(http, "http response body is too large"); return NGX_ERROR; } @@ -1042,7 +1092,17 @@ ngx_js_http_process_body(ngx_js_http_t *http) b->pos += chsize; } - rc = (need > chsize) ? NGX_AGAIN : NGX_DONE; + if (http->body == NGX_JS_HTTP_BODY_CLOSE) { + rc = NGX_AGAIN; + + } else { + rc = (need > chsize) ? NGX_AGAIN : NGX_DONE; + + if (rc == NGX_DONE) { + http->ready_handler(http); + return NGX_DONE; + } + } } if (b->pos == b->end) { diff --git a/nginx/ngx_js_http.h b/nginx/ngx_js_http.h index 3749f696..8bf06b4e 100644 --- a/nginx/ngx_js_http.h +++ b/nginx/ngx_js_http.h @@ -140,6 +140,7 @@ struct ngx_js_http_s { unsigned content_length:1; unsigned chunked:1; unsigned keepalive:1; + unsigned body:3; off_t content_length_n; #if (NGX_SSL) diff --git a/nginx/t/js_fetch.t b/nginx/t/js_fetch.t index 1bbb4138..80a98590 100644 --- a/nginx/t/js_fetch.t +++ b/nginx/t/js_fetch.t @@ -508,7 +508,7 @@ EOF $t->try_run('no njs.fetch'); -$t->plan(49); +$t->plan(54); $t->run_daemon(\&http_daemon, port(8082)); $t->waitforsocket('127.0.0.1:' . port(8082)); @@ -592,6 +592,18 @@ like(http_get('/framing?loc=content_length_same'), qr/200 OK.*ok:5$/s, like(http_get('/framing?loc=transfer_content_length'), qr/501.*conflicting http response framing/s, 'fetch transfer and content length'); +like(http_get('/body_special?loc=no_body/204'), qr/200 OK.*$/s, + 'fetch 204 response body'); +like(http_get('/body_special?loc=no_body/304'), qr/200 OK.*$/s, + 'fetch 304 response body'); +like(http_get('/framing?loc=continue'), + qr/501.*unsupported http response status/s, + 'fetch interim response unsupported'); +like(http_get('/framing?loc=no_body/304_large'), qr/200 OK.*ok:0$/s, + 'fetch 304 response large content length'); +like(http_get('/framing?loc=length'), + qr/501.*prematurely closed connection/s, + 'fetch incomplete fixed-length response'); like(http_get('/header_iter?loc=duplicate_header_large'), qr/\["A:a","B:a","C:a","D:a","E:a","F:a","G:a","H:a","Moo:a, ?b"]$/s, 'fetch header duplicate large'); @@ -735,6 +747,12 @@ sub http_daemon { "Connection: close" . CRLF . CRLF; + } elsif ($uri eq '/continue') { + print $client + "HTTP/1.1 100 Continue" . CRLF . + "Content-Length: 0" . CRLF . + CRLF; + } elsif ($uri eq '/content_length_conflict') { print $client "HTTP/1.1 200 OK" . CRLF . @@ -826,6 +844,17 @@ sub http_daemon { "Connection: close" . CRLF . CRLF; + } elsif ($uri eq '/no_body/204' || $uri eq '/no_body/304' + || $uri eq '/no_body/304_large') + { + my $code = ($uri eq '/no_body/204') ? 204 : 304; + my $length = ($uri eq '/no_body/304_large') ? 1000000 : 5; + print $client + "HTTP/1.1 $code No Content" . CRLF . + "Content-Length: $length" . CRLF . + "Connection: close" . CRLF . + CRLF; + } elsif ($uri eq '/parted') { print $client "HTTP/1.1 200 OK" . CRLF . diff --git a/nginx/t/js_fetch_keepalive.t b/nginx/t/js_fetch_keepalive.t index 100be40c..34b119e1 100755 --- a/nginx/t/js_fetch_keepalive.t +++ b/nginx/t/js_fetch_keepalive.t @@ -79,6 +79,11 @@ http { js_content test.keepalive; } + location /keepalive_head { + js_fetch_keepalive 4; + js_content test.keepalive_head; + } + location /no_keepalive { js_fetch_keepalive 0; js_content test.keepalive; @@ -158,7 +163,19 @@ $t->write_file('test.js', <try_run('no js_fetch_keepalive'); @@ -166,7 +183,7 @@ $t->try_run('no js_fetch_keepalive'); $t->run_daemon(\&http_daemon, $p2); $t->waitforsocket('127.0.0.1:' . $p2); -$t->plan(16); +$t->plan(17); ############################################################################### @@ -180,6 +197,8 @@ like(http_get('/keepalive?path=count'), qr/1,2,3/, 'keepalive reuses connection'); like(http_get('/keepalive?path=count'), qr/4,5,6/, 'keepalive reuses connection across requests'); +like(http_get('/keepalive_head'), qr/1,2,3/, + 'keepalive reuses head response connection'); like(http_get('/keepalive_simultaneous?n=8'), qr/1,1,1,1,1,1,1,1/, 'keepalive simultaneous requests'); like(http_get('/keepalive_simultaneous?n=8'), qr/2,2,2,2,1,1,1,1/, -- 2.47.3