Fetch: validate response framing

Previously, fetch accepted conflicting Content-Length fields and a response
with both Transfer-Encoding and Content-Length.

The fix is to reject ambiguous framing to avoid interpreting the response
body inconsistently.

diff --git a/nginx/ngx_js_http.c b/nginx/ngx_js_http.c
index 7d2a639504281c6853ab664e18744cd95f7e2bcf..9e70ebc2d8472ab175adea5df079e228ec0e7c46 100644 (file)
--- a/nginx/ngx_js_http.c
+++ b/nginx/ngx_js_http.c
@@ -831,6 +831,7 @@ static ngx_int_t
 ngx_js_http_process_headers(ngx_js_http_t *http)
 {
     size_t                len, vlen;
+    off_t                 content_length_n;
     ngx_int_t             rc;
     ngx_js_http_parse_t  *hp;
 
@@ -892,14 +893,24 @@ 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)
             {
-                http->content_length_n = ngx_atoof(hp->header_start, vlen);
-                if (http->content_length_n == NGX_ERROR) {
+                content_length_n = ngx_atoof(hp->header_start, vlen);
+                if (content_length_n == NGX_ERROR) {
                     ngx_js_http_error(http, "invalid http content length");
                     return NGX_ERROR;
                 }
 
+                if (http->content_length
+                    && http->content_length_n != content_length_n)
+                {
+                    ngx_js_http_error(http, "conflicting http content length");
+                    return NGX_ERROR;
+                }
+
+                http->content_length = 1;
+                http->content_length_n = content_length_n;
+
                 if (!http->header_only
                     && http->content_length_n
                        > (off_t) http->max_response_body_size)
@@ -929,6 +940,11 @@ ngx_js_http_process_headers(ngx_js_http_t *http)
         return NGX_ERROR;
     }
 
+    if (http->chunked && http->content_length) {
+        ngx_js_http_error(http, "conflicting http response framing");
+        return NGX_ERROR;
+    }
+
     njs_chb_destroy(&http->chain);
 
     http->process = ngx_js_http_process_body;

diff --git a/nginx/ngx_js_http.h b/nginx/ngx_js_http.h
index c3ebe15ad61662acf695d3fc5f831f326735921e..3749f6969a348bf6b9484906608d0347fb687540 100644 (file)
--- a/nginx/ngx_js_http.h
+++ b/nginx/ngx_js_http.h
@@ -136,10 +136,10 @@ struct ngx_js_http_s {
     ngx_int_t                      buffer_size;
     ngx_int_t                      max_response_body_size;
 
-    unsigned                       header_only;
-
-    ngx_flag_t                     chunked;
-    ngx_flag_t                     keepalive;
+    unsigned                       header_only:1;
+    unsigned                       content_length:1;
+    unsigned                       chunked:1;
+    unsigned                       keepalive:1;
     off_t                          content_length_n;
 
 #if (NGX_SSL)

diff --git a/nginx/t/js_fetch.t b/nginx/t/js_fetch.t
index 76370ab4b944c93bea88dd16cb67ca68ade551f4..1bbb4138208d4d4016e70383fb3feba8047e5299 100644 (file)
--- a/nginx/t/js_fetch.t
+++ b/nginx/t/js_fetch.t
@@ -92,6 +92,10 @@ http {
             js_content test.chunked_fail;
         }
 
+        location /framing {
+            js_content test.framing;
+        }
+
         location /header {
             js_content test.header;
         }
@@ -357,6 +361,17 @@ $t->write_file('test.js', <<EOF);
         })
     }
 
+    async function framing(r) {
+        try {
+            var reply = await ngx.fetch(`http://127.0.0.1:$p2/\${r.args.loc}`);
+            var body = await reply.text();
+            r.return(200, `ok:\${body.length}`);
+
+        } catch (e) {
+            r.return(501, e.message);
+        }
+    }
+
     function header(r) {
         var url = `http://127.0.0.1:$p2/\${r.args.loc}`;
         var method = r.args.method ? r.args.method : 'get';
@@ -486,14 +501,14 @@ $t->write_file('test.js', <<EOF);
 
     export default {njs: test_njs, body, body_size, broken, broken_response,
                     broken_catch, body_special, chain, chunked_ok,
-                    chunked_fail, header, header_iter, host_header,
+                    chunked_fail, framing, header, header_iter, host_header,
                     multi, loc, property, body_content_length,
                     user_agent_header };
 EOF
 
 $t->try_run('no njs.fetch');
 
-$t->plan(46);
+$t->plan(49);
 
 $t->run_daemon(\&http_daemon, port(8082));
 $t->waitforsocket('127.0.0.1:' . port(8082));
@@ -569,6 +584,14 @@ like(http_get('/chunked_ok'), qr/200/s, 'fetch chunked ok');
 like(http_get('/chunked_fail'), qr/200/s, 'fetch chunked fail');
 like(http_get('/chain'), qr/200 OK.*SUCCESS$/s, 'fetch chain');
 
+like(http_get('/framing?loc=content_length_conflict'),
+       qr/501.*conflicting http content length/s,
+       'fetch conflicting content-length');
+like(http_get('/framing?loc=content_length_same'), qr/200 OK.*ok:5$/s,
+       'fetch identical content-length');
+like(http_get('/framing?loc=transfer_content_length'),
+       qr/501.*conflicting http response framing/s,
+       'fetch transfer and content length');
 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');
@@ -712,6 +735,30 @@ sub http_daemon {
                                "Connection: close" . CRLF .
                                CRLF;
 
+               } elsif ($uri eq '/content_length_conflict') {
+                       print $client
+                               "HTTP/1.1 200 OK" . CRLF .
+                               "Content-Length: 4" . CRLF .
+                               "content-length: 5" . CRLF .
+                               "Connection: close" . CRLF .
+                               CRLF . "12345";
+
+               } elsif ($uri eq '/content_length_same') {
+                       print $client
+                               "HTTP/1.1 200 OK" . CRLF .
+                               "Content-Length: 5" . CRLF .
+                               "content-length: 5" . CRLF .
+                               "Connection: close" . CRLF .
+                               CRLF . "12345";
+
+               } elsif ($uri eq '/transfer_content_length') {
+                       print $client
+                               "HTTP/1.1 200 OK" . CRLF .
+                               "Transfer-Encoding: chunked" . CRLF .
+                               "Content-Length: 5" . CRLF .
+                               "Connection: close" . CRLF .
+                               CRLF . "5" . CRLF . "12345" . CRLF . "0" . CRLF . CRLF;
+
                } elsif ($uri eq '/header') {
                        print $client
                                "HTTP/1.1 200 OK" . CRLF .
@@ -862,6 +909,7 @@ sub http_daemon {
                        }
 
                    print $client  "0" . CRLF . CRLF;
+
                }
        }
 }