BUG/MEDIUM: h1: ensure the chunk size parser can deal with full buffers

The HTTP/1 code always has the reserve left available so the buffer is
never full there. But with HTTP/2 we have to deal with full buffers,
and it happens that the chunk size parser cannot tell the difference
between a full buffer and an empty one since it compares the start and
the stop pointer.

Let's change this to instead deal with the number of bytes left to process.

As a side effect, this code ends up being about 10% faster than the previous
one, even on HTTP/1.
This commit is contained in:
Willy Tarreau 2017-11-10 11:17:08 +01:00
parent 8c0ea7d21a
commit b15e3fefc9

View File

@ -192,16 +192,18 @@ static inline int h1_parse_chunk_size(const struct buffer *buf, int start, int s
const char *ptr = b_ptr(buf, start); const char *ptr = b_ptr(buf, start);
const char *ptr_old = ptr; const char *ptr_old = ptr;
const char *end = buf->data + buf->size; const char *end = buf->data + buf->size;
const char *ptr_stop = b_ptr(buf, stop);
unsigned int chunk = 0; unsigned int chunk = 0;
stop -= start; // bytes left
start = stop; // bytes to transfer
/* The chunk size is in the following form, though we are only /* The chunk size is in the following form, though we are only
* interested in the size and CRLF : * interested in the size and CRLF :
* 1*HEXDIGIT *WSP *[ ';' extensions ] CRLF * 1*HEXDIGIT *WSP *[ ';' extensions ] CRLF
*/ */
while (1) { while (1) {
int c; int c;
if (ptr == ptr_stop) if (!stop)
return 0; return 0;
c = hex2i(*ptr); c = hex2i(*ptr);
if (c < 0) /* not a hex digit anymore */ if (c < 0) /* not a hex digit anymore */
@ -211,6 +213,7 @@ static inline int h1_parse_chunk_size(const struct buffer *buf, int start, int s
if (unlikely(chunk & 0xF8000000)) /* integer overflow will occur if result >= 2GB */ if (unlikely(chunk & 0xF8000000)) /* integer overflow will occur if result >= 2GB */
goto error; goto error;
chunk = (chunk << 4) + c; chunk = (chunk << 4) + c;
stop--;
} }
/* empty size not allowed */ /* empty size not allowed */
@ -220,7 +223,7 @@ static inline int h1_parse_chunk_size(const struct buffer *buf, int start, int s
while (HTTP_IS_SPHT(*ptr)) { while (HTTP_IS_SPHT(*ptr)) {
if (++ptr >= end) if (++ptr >= end)
ptr = buf->data; ptr = buf->data;
if (unlikely(ptr == ptr_stop)) if (--stop == 0)
return 0; return 0;
} }
@ -233,14 +236,15 @@ static inline int h1_parse_chunk_size(const struct buffer *buf, int start, int s
if (likely(*ptr == '\r')) { if (likely(*ptr == '\r')) {
if (++ptr >= end) if (++ptr >= end)
ptr = buf->data; ptr = buf->data;
if (ptr == ptr_stop) if (--stop == 0)
return 0; return 0;
} }
if (unlikely(*ptr != '\n')) if (*ptr != '\n')
goto error; goto error;
if (++ptr >= end) if (++ptr >= end)
ptr = buf->data; ptr = buf->data;
--stop;
/* done */ /* done */
break; break;
} }
@ -248,13 +252,13 @@ static inline int h1_parse_chunk_size(const struct buffer *buf, int start, int s
/* chunk extension, ends at next CRLF */ /* chunk extension, ends at next CRLF */
if (++ptr >= end) if (++ptr >= end)
ptr = buf->data; ptr = buf->data;
if (ptr == ptr_stop) if (--stop == 0)
return 0; return 0;
while (!HTTP_IS_CRLF(*ptr)) { while (!HTTP_IS_CRLF(*ptr)) {
if (++ptr >= end) if (++ptr >= end)
ptr = buf->data; ptr = buf->data;
if (ptr == ptr_stop) if (--stop == 0)
return 0; return 0;
} }
/* we have a CRLF now, loop above */ /* we have a CRLF now, loop above */
@ -268,10 +272,10 @@ static inline int h1_parse_chunk_size(const struct buffer *buf, int start, int s
* or may not be present. Let's return the number of bytes parsed. * or may not be present. Let's return the number of bytes parsed.
*/ */
*res = chunk; *res = chunk;
return (ptr - ptr_old) >= 0 ? (ptr - ptr_old) : (ptr - ptr_old + buf->size); return start - stop;
error: error:
*res = 0; // just to stop gcc's -Wuninitialized warning :-( *res = 0; // just to stop gcc's -Wuninitialized warning :-(
return -buffer_count(buf, ptr, ptr_stop); return -stop;
} }
/* initializes an H1 message */ /* initializes an H1 message */