src: callbacks chunk boundaries: header/complete

(Proxygen fork merge D508755 D521404, orig author [email protected])

Fix: #231
PR-URL: #233
Reviewed-By: Fedor Indutny <[email protected]>

Author: KjellSchubert

http_parser.c

@@ -1782,9 +1782,9 @@ size_t http_parser_execute (http_parser *parser,
 
         if (parser->flags & F_TRAILING) {
           /* End of a chunked request */
-          UPDATE_STATE(NEW_MESSAGE());
-          CALLBACK_NOTIFY(message_complete);
-          break;
+          UPDATE_STATE(s_message_done);
+          CALLBACK_NOTIFY_NOADVANCE(chunk_complete);
+          REEXECUTE();
         }
 
         UPDATE_STATE(s_headers_done);
@@ -1994,6 +1994,7 @@ size_t http_parser_execute (http_parser *parser,
         } else {
           UPDATE_STATE(s_chunk_data);
         }
+        CALLBACK_NOTIFY(chunk_header);
         break;
       }
 
@@ -2033,6 +2034,7 @@ size_t http_parser_execute (http_parser *parser,
         STRICT_CHECK(ch != LF);
         parser->nread = 0;
         UPDATE_STATE(s_chunk_size_start);
+        CALLBACK_NOTIFY(chunk_complete);
         break;
 
       default:
@@ -2144,13 +2146,15 @@ http_parser_settings_init(http_parser_settings *settings)
 
 const char *
 http_errno_name(enum http_errno err) {
-  assert(err < (sizeof(http_strerror_tab)/sizeof(http_strerror_tab[0])));
+  assert(((size_t) err) <
+      (sizeof(http_strerror_tab) / sizeof(http_strerror_tab[0])));
   return http_strerror_tab[err].name;
 }
 
 const char *
 http_errno_description(enum http_errno err) {
-  assert(err < (sizeof(http_strerror_tab)/sizeof(http_strerror_tab[0])));
+  assert(((size_t) err) <
+      (sizeof(http_strerror_tab) / sizeof(http_strerror_tab[0])));
   return http_strerror_tab[err].description;
 }
 

http_parser.h

@@ -160,6 +160,8 @@ enum flags
   XX(CB_body, "the on_body callback failed")                         \
   XX(CB_message_complete, "the on_message_complete callback failed") \
   XX(CB_status, "the on_status callback failed")                     \
+  XX(CB_chunk_header, "the on_chunk_header callback failed")         \
+  XX(CB_chunk_complete, "the on_chunk_complete callback failed")     \
                                                                      \
   /* Parsing-related errors */                                       \
   XX(INVALID_EOF_STATE, "stream ended at an unexpected time")        \
@@ -240,6 +242,11 @@ struct http_parser_settings {
   http_cb      on_headers_complete;
   http_data_cb on_body;
   http_cb      on_message_complete;
+  /* When on_chunk_header is called, the current chunk length is stored
+   * in parser->content_length.
+   */
+  http_cb      on_chunk_header;
+  http_cb      on_chunk_complete;
 };
 
 

test.c

@@ -39,6 +39,7 @@
 
 #define MAX_HEADERS 13
 #define MAX_ELEMENT_SIZE 2048
+#define MAX_CHUNKS 16
 
 #define MIN(a,b) ((a) < (b) ? (a) : (b))
 
@@ -65,6 +66,10 @@ struct message {
   char headers [MAX_HEADERS][2][MAX_ELEMENT_SIZE];
   int should_keep_alive;
 
+  int num_chunks;
+  int num_chunks_complete;
+  int chunk_lengths[MAX_CHUNKS];
+
   const char *upgrade; // upgraded body
 
   unsigned short http_major;
@@ -301,6 +306,8 @@ const struct message requests[] =
     { { "Transfer-Encoding" , "chunked" }
     }
   ,.body= "all your base are belong to us"
+  ,.num_chunks_complete= 2
+  ,.chunk_lengths= { 0x1e }
   }
 
 #define TWO_CHUNKS_MULT_ZERO_END 9
@@ -327,6 +334,8 @@ const struct message requests[] =
     { { "Transfer-Encoding", "chunked" }
     }
   ,.body= "hello world"
+  ,.num_chunks_complete= 3
+  ,.chunk_lengths= { 5, 6 }
   }
 
 #define CHUNKED_W_TRAILING_HEADERS 10
@@ -357,6 +366,8 @@ const struct message requests[] =
     , { "Content-Type", "text/plain" }
     }
   ,.body= "hello world"
+  ,.num_chunks_complete= 3
+  ,.chunk_lengths= { 5, 6 }
   }
 
 #define CHUNKED_W_BULLSHIT_AFTER_LENGTH 11
@@ -383,6 +394,8 @@ const struct message requests[] =
     { { "Transfer-Encoding", "chunked" }
     }
   ,.body= "hello world"
+  ,.num_chunks_complete= 3
+  ,.chunk_lengths= { 5, 6 }
   }
 
 #define WITH_QUOTES 12
@@ -1195,7 +1208,8 @@ const struct message responses[] =
   ,.body =
          "This is the data in the first chunk\r\n"
          "and this is the second one\r\n"
-
+  ,.num_chunks_complete= 3
+  ,.chunk_lengths= { 0x25, 0x1c }
   }
 
 #define NO_CARRIAGE_RET 5
@@ -1349,6 +1363,8 @@ const struct message responses[] =
     , { "Connection", "close" }
     }
   ,.body= ""
+  ,.num_chunks_complete= 1
+  ,.chunk_lengths= {}
   }
 
 #define NON_ASCII_IN_STATUS_LINE 10
@@ -1531,6 +1547,7 @@ const struct message responses[] =
     }
   ,.body_size= 0
   ,.body= ""
+  ,.num_chunks_complete= 1
   }
 
 #if !HTTP_PARSER_STRICT
@@ -1604,6 +1621,8 @@ const struct message responses[] =
              , { "Transfer-Encoding", "chunked" }
              }
   ,.body= "\n"
+  ,.num_chunks_complete= 2
+  ,.chunk_lengths= { 1 }
   }
 
 #define EMPTY_REASON_PHRASE_AFTER_SPACE 20
@@ -1839,6 +1858,35 @@ response_status_cb (http_parser *p, const char *buf, size_t len)
   return 0;
 }
 
+int
+chunk_header_cb (http_parser *p)
+{
+  assert(p == parser);
+  int chunk_idx = messages[num_messages].num_chunks;
+  messages[num_messages].num_chunks++;
+  if (chunk_idx < MAX_CHUNKS) {
+    messages[num_messages].chunk_lengths[chunk_idx] = p->content_length;
+  }
+
+  return 0;
+}
+
+int
+chunk_complete_cb (http_parser *p)
+{
+  assert(p == parser);
+
+  /* Here we want to verify that each chunk_header_cb is matched by a
+   * chunk_complete_cb, so not only should the total number of calls to
+   * both callbacks be the same, but they also should be interleaved
+   * properly */
+  assert(messages[num_messages].num_chunks ==
+         messages[num_messages].num_chunks_complete + 1);
+
+  messages[num_messages].num_chunks_complete++;
+  return 0;
+}
+
 /* These dontcall_* callbacks exist so that we can verify that when we're
  * paused, no additional callbacks are invoked */
 int
@@ -1907,6 +1955,23 @@ dontcall_response_status_cb (http_parser *p, const char *buf, size_t len)
   abort();
 }
 
+int
+dontcall_chunk_header_cb (http_parser *p)
+{
+  if (p) { } // gcc
+  fprintf(stderr, "\n\n*** on_chunk_header() called on paused parser ***\n\n");
+  exit(1);
+}
+
+int
+dontcall_chunk_complete_cb (http_parser *p)
+{
+  if (p) { } // gcc
+  fprintf(stderr, "\n\n*** on_chunk_complete() "
+          "called on paused parser ***\n\n");
+  exit(1);
+}
+
 static http_parser_settings settings_dontcall =
   {.on_message_begin = dontcall_message_begin_cb
   ,.on_header_field = dontcall_header_field_cb
@@ -1916,6 +1981,8 @@ static http_parser_settings settings_dontcall =
   ,.on_body = dontcall_body_cb
   ,.on_headers_complete = dontcall_headers_complete_cb
   ,.on_message_complete = dontcall_message_complete_cb
+  ,.on_chunk_header = dontcall_chunk_header_cb
+  ,.on_chunk_complete = dontcall_chunk_complete_cb
   };
 
 /* These pause_* callbacks always pause the parser and just invoke the regular
@@ -1986,6 +2053,22 @@ pause_response_status_cb (http_parser *p, const char *buf, size_t len)
   return response_status_cb(p, buf, len);
 }
 
+int
+pause_chunk_header_cb (http_parser *p)
+{
+  http_parser_pause(p, 1);
+  *current_pause_parser = settings_dontcall;
+  return chunk_header_cb(p);
+}
+
+int
+pause_chunk_complete_cb (http_parser *p)
+{
+  http_parser_pause(p, 1);
+  *current_pause_parser = settings_dontcall;
+  return chunk_complete_cb(p);
+}
+
 static http_parser_settings settings_pause =
   {.on_message_begin = pause_message_begin_cb
   ,.on_header_field = pause_header_field_cb
@@ -1995,6 +2078,8 @@ static http_parser_settings settings_pause =
   ,.on_body = pause_body_cb
   ,.on_headers_complete = pause_headers_complete_cb
   ,.on_message_complete = pause_message_complete_cb
+  ,.on_chunk_header = pause_chunk_header_cb
+  ,.on_chunk_complete = pause_chunk_complete_cb
   };
 
 static http_parser_settings settings =
@@ -2006,6 +2091,8 @@ static http_parser_settings settings =
   ,.on_body = body_cb
   ,.on_headers_complete = headers_complete_cb
   ,.on_message_complete = message_complete_cb
+  ,.on_chunk_header = chunk_header_cb
+  ,.on_chunk_complete = chunk_complete_cb
   };
 
 static http_parser_settings settings_count_body =
@@ -2017,6 +2104,8 @@ static http_parser_settings settings_count_body =
   ,.on_body = count_body_cb
   ,.on_headers_complete = headers_complete_cb
   ,.on_message_complete = message_complete_cb
+  ,.on_chunk_header = chunk_header_cb
+  ,.on_chunk_complete = chunk_complete_cb
   };
 
 static http_parser_settings settings_null =
@@ -2028,6 +2117,8 @@ static http_parser_settings settings_null =
   ,.on_body = 0
   ,.on_headers_complete = 0
   ,.on_message_complete = 0
+  ,.on_chunk_header = 0
+  ,.on_chunk_complete = 0
   };
 
 void
@@ -2196,6 +2287,12 @@ message_eq (int index, const struct message *expected)
     MESSAGE_CHECK_STR_EQ(expected, m, body);
   }
 
+  assert(m->num_chunks == m->num_chunks_complete);
+  MESSAGE_CHECK_NUM_EQ(expected, m, num_chunks_complete);
+  for (i = 0; i < m->num_chunks && i < MAX_CHUNKS; i++) {
+    MESSAGE_CHECK_NUM_EQ(expected, m, chunk_lengths[i]);
+  }
+
   MESSAGE_CHECK_NUM_EQ(expected, m, num_headers);
 
   int r;
@@ -3488,7 +3585,11 @@ main (void)
         , { "Content-Type", "text/plain" }
         }
       ,.body_size= 31337*1024
+      ,.num_chunks_complete= 31338
       };
+    for (i = 0; i < MAX_CHUNKS; i++) {
+      large_chunked.chunk_lengths[i] = 1024;
+    }
     test_message_count_body(&large_chunked);
     free(msg);
   }