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.

diff --git a/nginx/ngx_js_http.c b/nginx/ngx_js_http.c
index 9e70ebc2d8472ab175adea5df079e228ec0e7c46..43984c1e87a4d49ed8ebd35be4771b46ae48a9e4 100644 (file)
--- 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 3749f6969a348bf6b9484906608d0347fb687540..8bf06b4e9f57423b725fe985cefa475eb62b9f61 100644 (file)
--- 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 1bbb4138208d4d4016e70383fb3feba8047e5299..80a9859098bf6b50ee984181d8325119e8f51020 100644 (file)
--- 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.*<empty>$/s,
+       'fetch 204 response body');
+like(http_get('/body_special?loc=no_body/304'), qr/200 OK.*<empty>$/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 100be40c6e1db0a0bf8e64d56cfd09cc8f75b08b..34b119e17a896f8158b10081ad9cf46180d917a4 100755 (executable)
--- 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', <<EOF);
         r.return(200, JSON.stringify(responses));
     }
 
-    export default {engine, keepalive, keepalive_simultaneous};
+    async function keepalive_head(r) {
+        let responses = [];
+
+        for (let i = 0; i < 3; i++) {
+            let reply = await ngx.fetch('http://127.0.0.1:$p1/count',
+                                        {method: 'HEAD'});
+            responses.push(reply.headers.get('Connection-ID'));
+        }
+
+        r.return(200, responses.toString());
+    }
+
+    export default {engine, keepalive, keepalive_head, keepalive_simultaneous};
 EOF
 
 $t->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/,