[nginx] SPDY: ngx_http_spdy_state_headers() error handling cleanup.

Valentin Bartenev vbart at nginx.com
Thu May 15 15:25:49 UTC 2014


details:   http://hg.nginx.org/nginx/rev/31349361558e
branches:  
changeset: 5691:31349361558e
user:      Valentin Bartenev <vbart at nginx.com>
date:      Wed Apr 30 20:34:20 2014 +0400
description:
SPDY: ngx_http_spdy_state_headers() error handling cleanup.

 - Specification-friendly handling of invalid header block or special headers.
   Such errors are not fatal for session and shouldn't lead to connection close;

 - Avoid mix of NGX_HTTP_PARSE_INVALID_REQUEST/NGX_HTTP_PARSE_INVALID_HEADER.

diffstat:

 src/http/ngx_http_spdy.c |  104 ++++++++++++++++++++++++++++------------------
 1 files changed, 63 insertions(+), 41 deletions(-)

diffs (252 lines):

diff -r fb56f5d612a0 -r 31349361558e src/http/ngx_http_spdy.c
--- a/src/http/ngx_http_spdy.c	Wed Apr 30 20:34:20 2014 +0400
+++ b/src/http/ngx_http_spdy.c	Wed Apr 30 20:34:20 2014 +0400
@@ -105,6 +105,8 @@ static u_char *ngx_http_spdy_state_heade
     u_char *pos, u_char *end);
 static u_char *ngx_http_spdy_state_headers_skip(ngx_http_spdy_connection_t *sc,
     u_char *pos, u_char *end);
+static u_char *ngx_http_spdy_state_headers_error(ngx_http_spdy_connection_t *sc,
+    u_char *pos, u_char *end);
 static u_char *ngx_http_spdy_state_window_update(ngx_http_spdy_connection_t *sc,
     u_char *pos, u_char *end);
 static u_char *ngx_http_spdy_state_data(ngx_http_spdy_connection_t *sc,
@@ -1083,11 +1085,10 @@ ngx_http_spdy_state_headers(ngx_http_spd
         if (buf->last - buf->pos < NGX_SPDY_NV_NUM_SIZE) {
 
             if (complete) {
-                ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
-                              "client sent SYN_STREAM frame "
-                              "with invalid HEADERS block");
-
-                return ngx_http_spdy_state_protocol_error(sc);
+                ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
+                               "premature end of spdy header block");
+
+                return ngx_http_spdy_state_headers_error(sc, pos, end);
             }
 
             return ngx_http_spdy_state_save(sc, pos, end,
@@ -1181,13 +1182,13 @@ ngx_http_spdy_state_headers(ngx_http_spd
                 ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
                                "spdy again while last chunk");
 
-                return ngx_http_spdy_state_protocol_error(sc);
+                return ngx_http_spdy_state_headers_error(sc, pos, end);
             }
 
             return ngx_http_spdy_state_save(sc, pos, end,
                                             ngx_http_spdy_state_headers);
 
-        case NGX_HTTP_PARSE_INVALID_REQUEST:
+        case NGX_HTTP_PARSE_INVALID_HEADER:
 
             /* TODO: improve error message */
             ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
@@ -1197,12 +1198,8 @@ ngx_http_spdy_state_headers(ngx_http_spd
 
             return ngx_http_spdy_state_headers_skip(sc, pos, end);
 
-        default: /* NGX_HTTP_PARSE_INVALID_HEADER */
-
-            ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
-                          "client sent invalid HEADERS spdy frame");
-
-            return ngx_http_spdy_state_protocol_error(sc);
+        default: /* NGX_ERROR */
+            return ngx_http_spdy_state_headers_error(sc, pos, end);
         }
 
         /* a header line has been parsed successfully */
@@ -1211,14 +1208,13 @@ ngx_http_spdy_state_headers(ngx_http_spd
 
         if (rc != NGX_OK) {
             if (rc == NGX_HTTP_PARSE_INVALID_HEADER) {
-                ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
-                              "client sent invalid HEADERS spdy frame");
-
-                return ngx_http_spdy_state_protocol_error(sc);
+                ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST);
+                return ngx_http_spdy_state_headers_skip(sc, pos, end);
             }
 
-            if (rc == NGX_HTTP_PARSE_INVALID_REQUEST) {
-                ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST);
+            if (rc != NGX_ABORT) {
+                ngx_http_spdy_close_stream(sc->stream,
+                                           NGX_HTTP_INTERNAL_SERVER_ERROR);
             }
 
             return ngx_http_spdy_state_headers_skip(sc, pos, end);
@@ -1226,11 +1222,10 @@ ngx_http_spdy_state_headers(ngx_http_spd
     }
 
     if (buf->pos != buf->last || sc->zstream_in.avail_in) {
-        ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
-                      "client sent SYN_STREAM frame "
-                      "with invalid HEADERS block");
-
-        return ngx_http_spdy_state_protocol_error(sc);
+        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
+                       "incorrect number of spdy header block entries");
+
+        return ngx_http_spdy_state_headers_error(sc, pos, end);
     }
 
     if (!complete) {
@@ -1293,6 +1288,33 @@ ngx_http_spdy_state_headers_skip(ngx_htt
 
 
 static u_char *
+ngx_http_spdy_state_headers_error(ngx_http_spdy_connection_t *sc, u_char *pos,
+    u_char *end)
+{
+    ngx_http_spdy_stream_t  *stream;
+
+    stream = sc->stream;
+
+    ngx_log_error(NGX_LOG_INFO, sc->connection->log, 0,
+                  "client sent SYN_STREAM frame for stream %ui "
+                  "with invalid header block", stream->id);
+
+    if (ngx_http_spdy_send_rst_stream(sc, stream->id, NGX_SPDY_PROTOCOL_ERROR,
+                                      stream->priority)
+        != NGX_OK)
+    {
+        return ngx_http_spdy_state_internal_error(sc);
+    }
+
+    stream->out_closed = 1;
+
+    ngx_http_spdy_close_stream(stream, NGX_HTTP_BAD_REQUEST);
+
+    return ngx_http_spdy_state_headers_skip(sc, pos, end);
+}
+
+
+static u_char *
 ngx_http_spdy_state_window_update(ngx_http_spdy_connection_t *sc, u_char *pos,
     u_char *end)
 {
@@ -2425,7 +2447,7 @@ ngx_http_spdy_parse_header(ngx_http_requ
         r->lowcase_index = ngx_spdy_frame_parse_uint32(p);
 
         if (r->lowcase_index == 0) {
-            return NGX_HTTP_PARSE_INVALID_HEADER;
+            return NGX_ERROR;
         }
 
         /* null-terminate the previous header value */
@@ -2475,11 +2497,11 @@ ngx_http_spdy_parse_header(ngx_http_requ
             case LF:
             case CR:
             case ':':
-                return NGX_HTTP_PARSE_INVALID_REQUEST;
+                return NGX_HTTP_PARSE_INVALID_HEADER;
             }
 
             if (ch >= 'A' && ch <= 'Z') {
-                return NGX_HTTP_PARSE_INVALID_HEADER;
+                return NGX_ERROR;
             }
 
             r->invalid_header = 1;
@@ -2642,13 +2664,11 @@ ngx_http_spdy_handle_request_header(ngx_
             return sh->handler(r);
         }
 
-        return NGX_HTTP_PARSE_INVALID_REQUEST;
+        return NGX_HTTP_PARSE_INVALID_HEADER;
     }
 
     h = ngx_list_push(&r->headers_in.headers);
     if (h == NULL) {
-        ngx_http_spdy_close_stream(r->spdy_stream,
-                                   NGX_HTTP_INTERNAL_SERVER_ERROR);
         return NGX_ERROR;
     }
 
@@ -2752,7 +2772,7 @@ ngx_http_spdy_parse_method(ngx_http_requ
         if ((*p < 'A' || *p > 'Z') && *p != '_') {
             ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
                           "client sent invalid method");
-            return NGX_HTTP_PARSE_INVALID_REQUEST;
+            return NGX_HTTP_PARSE_INVALID_HEADER;
         }
 
         p++;
@@ -2788,8 +2808,6 @@ ngx_http_spdy_parse_host(ngx_http_reques
 
     h = ngx_list_push(&r->headers_in.headers);
     if (h == NULL) {
-        ngx_http_spdy_close_stream(r->spdy_stream,
-                                   NGX_HTTP_INTERNAL_SERVER_ERROR);
         return NGX_ERROR;
     }
 
@@ -2820,11 +2838,15 @@ ngx_http_spdy_parse_path(ngx_http_reques
     r->uri_end = r->header_end;
 
     if (ngx_http_parse_uri(r) != NGX_OK) {
-        return NGX_HTTP_PARSE_INVALID_REQUEST;
+        return NGX_HTTP_PARSE_INVALID_HEADER;
     }
 
     if (ngx_http_process_request_uri(r) != NGX_OK) {
-        return NGX_ERROR;
+        /*
+         * request has been finalized already
+         * in ngx_http_process_request_uri()
+         */
+        return NGX_ABORT;
     }
 
     return NGX_OK;
@@ -2843,13 +2865,13 @@ ngx_http_spdy_parse_version(ngx_http_req
     p = r->header_start;
 
     if (r->header_end - p < 8 || !(ngx_str5cmp(p, 'H', 'T', 'T', 'P', '/'))) {
-        return NGX_HTTP_PARSE_INVALID_REQUEST;
+        return NGX_HTTP_PARSE_INVALID_HEADER;
     }
 
     ch = *(p + 5);
 
     if (ch < '1' || ch > '9') {
-        return NGX_HTTP_PARSE_INVALID_REQUEST;
+        return NGX_HTTP_PARSE_INVALID_HEADER;
     }
 
     r->http_major = ch - '0';
@@ -2863,20 +2885,20 @@ ngx_http_spdy_parse_version(ngx_http_req
         }
 
         if (ch < '0' || ch > '9') {
-            return NGX_HTTP_PARSE_INVALID_REQUEST;
+            return NGX_HTTP_PARSE_INVALID_HEADER;
         }
 
         r->http_major = r->http_major * 10 + ch - '0';
     }
 
     if (*p != '.') {
-        return NGX_HTTP_PARSE_INVALID_REQUEST;
+        return NGX_HTTP_PARSE_INVALID_HEADER;
     }
 
     ch = *(p + 1);
 
     if (ch < '0' || ch > '9') {
-        return NGX_HTTP_PARSE_INVALID_REQUEST;
+        return NGX_HTTP_PARSE_INVALID_HEADER;
     }
 
     r->http_minor = ch - '0';
@@ -2886,7 +2908,7 @@ ngx_http_spdy_parse_version(ngx_http_req
         ch = *p;
 
         if (ch < '0' || ch > '9') {
-            return NGX_HTTP_PARSE_INVALID_REQUEST;
+            return NGX_HTTP_PARSE_INVALID_HEADER;
         }
 
         r->http_minor = r->http_minor * 10 + ch - '0';



More information about the nginx-devel mailing list