MAJOR: http: don't update msg->sov anymore while processing the body
We used to have msg->sov updated for every chunk that was parsed. The issue
is that we want to be able to rewind after chunks were parsed in case we need
to redispatch a request and perform a new hash on the request or insert a
different server header name.
Currently, msg->sov and msg->next make parallel progress. We reached a point
where they're always equal because msg->next is initialized from msg->sov,
and is subtracted msg->sov's value each time msg->sov bytes are forwarded.
So we can now ensure that msg->sov can always be replaced by msg->next for
every state after HTTP_MSG_BODY where it is used as a position counter.
This allows us to keep msg->sov untouched whatever the number of chunks that
are parsed, as is needed to extract data from POST request (eg: url_param).
However, we still need to know the starting position of the data relative to
the body, which differs by the chunk size length. We use msg->sol for this
since it's now always zero and unused in the body.
So with this patch, we have the following situation :
- msg->sov = msg->eoh + msg->eol = size of the headers including last CRLF
- msg->sol = length of the chunk size if any. So msg->sov + msg->sol = DATA.
- msg->next corresponds to the byte being inspected based on the current
state and is always >= msg->sov before starting to forward anything.
Since sov and next are updated in case of header rewriting, a rewind will
fix them both when needed. Of course, ->sol has no reason for changing in
such conditions, so it's fine to keep it relative to msg->sov.
In theory, even if a redispatch has to be performed, a transformation
occurring on the request would still work because the data moved would
still appear at the same place relative to bug->p.
diff --git a/src/backend.c b/src/backend.c
index 72c12d1..17e4b7f 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -299,12 +299,12 @@
struct proxy *px = s->be;
unsigned int plen = px->url_param_len;
unsigned long len = msg->body_len;
- const char *params = b_ptr(req->buf, (int)(msg->sov - req->buf->o));
+ const char *params = b_ptr(req->buf, (int)(msg->sov + msg->sol - req->buf->o));
const char *p = params;
const char *start, *end;
- if (len > buffer_len(req->buf) - msg->sov)
- len = buffer_len(req->buf) - msg->sov;
+ if (len > buffer_len(req->buf) - msg->sov - msg->sol)
+ len = buffer_len(req->buf) - msg->sov - msg->sol;
if (len == 0)
return NULL;
diff --git a/src/compression.c b/src/compression.c
index 8680832..0fd6846 100644
--- a/src/compression.c
+++ b/src/compression.c
@@ -141,7 +141,6 @@
*/
b_adv(in, msg->next);
msg->next = 0;
- msg->sov = 0;
out->size = global.tune.bufsize;
out->i = 0;
@@ -180,7 +179,6 @@
*/
b_adv(in, msg->next);
msg->next = 0;
- msg->sov = 0;
/*
* select the smallest size between the announced chunk size, the input
diff --git a/src/proto_http.c b/src/proto_http.c
index 943439c..1334579 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -1923,9 +1923,11 @@
return;
}
-/* Parse the chunk size at msg->next. Once done, it adjusts ->next to point to the
- * first byte of body, and increments msg->sov by the number of bytes parsed.
- * so that we know we can forward ->sov bytes.
+/* Parse the chunk size at msg->next. Once done, it adjusts ->next to point to
+ * the first byte of data after the chunk size, so that we know we can forward
+ * exactly msg->next bytes. msg->sol contains the exact number of bytes forming
+ * the chunk size. That way it is always possible to differentiate between the
+ * start of the body and the start of the data.
* Return >0 on success, 0 when some data is missing, <0 on error.
* Note: this function is designed to parse wrapped CRLF at the end of the buffer.
*/
@@ -2008,12 +2010,12 @@
}
/* OK we found our CRLF and now <ptr> points to the next byte,
- * which may or may not be present. We save that into ->next and
- * ->sov.
+ * which may or may not be present. We save that into ->next,
+ * and the number of bytes parsed into msg->sol.
*/
+ msg->sol = ptr - ptr_old;
if (unlikely(ptr < ptr_old))
- msg->sov += buf->size;
- msg->sov += ptr - ptr_old;
+ msg->sol += buf->size;
msg->next = buffer_count(buf, buf->p, ptr);
msg->chunk_len = chunk;
msg->body_len += chunk;
@@ -2029,14 +2031,14 @@
* the trailers is found, it is automatically scheduled to be forwarded,
* msg->msg_state switches to HTTP_MSG_DONE, and the function returns >0.
* If not enough data are available, the function does not change anything
- * except maybe msg->next and msg->sov if it could parse some lines, and returns
- * zero. If a parse error is encountered, the function returns < 0 and does not
- * change anything except maybe msg->next and msg->sov. Note that the message
- * must already be in HTTP_MSG_TRAILERS state before calling this function,
+ * except maybe msg->next if it could parse some lines, and returns zero.
+ * If a parse error is encountered, the function returns < 0 and does not
+ * change anything except maybe msg->next. Note that the message must
+ * already be in HTTP_MSG_TRAILERS state before calling this function,
* which implies that all non-trailers data have already been scheduled for
- * forwarding, and that msg->sov exactly matches the length of trailers already
- * parsed and not forwarded. It is also important to note that this function is
- * designed to be able to parse wrapped headers at end of buffer.
+ * forwarding, and that msg->next exactly matches the length of trailers
+ * already parsed and not forwarded. It is also important to note that this
+ * function is designed to be able to parse wrapped headers at end of buffer.
*/
static int http_forward_trailers(struct http_msg *msg)
{
@@ -2083,11 +2085,6 @@
if (bytes < 0)
bytes += buf->size;
- /* schedule this line for forwarding */
- msg->sov += bytes;
- if (msg->sov >= buf->size)
- msg->sov -= buf->size;
-
if (p1 == b_ptr(buf, msg->next)) {
/* LF/CRLF at beginning of line => end of trailers at p2.
* Everything was scheduled for forwarding, there's nothing
@@ -2102,9 +2099,9 @@
}
}
-/* This function may be called only in HTTP_MSG_CHUNK_CRLF. It reads the CRLF or
- * a possible LF alone at the end of a chunk. It automatically adjusts msg->sov,
- * and ->next in order to include this part into the next forwarding phase.
+/* This function may be called only in HTTP_MSG_CHUNK_CRLF. It reads the CRLF
+ * or a possible LF alone at the end of a chunk. It automatically adjusts
+ * msg->next in order to include this part into the next forwarding phase.
* Note that the caller must ensure that ->p points to the first byte to parse.
* It also sets msg_state to HTTP_MSG_CHUNK_SIZE and returns >0 on success. If
* not enough data are available, the function does not change anything and
@@ -2142,8 +2139,7 @@
ptr++;
if (unlikely(ptr >= buf->data + buf->size))
ptr = buf->data;
- /* prepare the CRLF to be forwarded (->sov) */
- msg->sov += bytes;
+ /* Advance ->next to allow the CRLF to be forwarded */
msg->next += bytes;
msg->msg_state = HTTP_MSG_CHUNK_SIZE;
return 1;
@@ -5005,8 +5001,7 @@
* be between MSG_BODY and MSG_DONE (inclusive). It returns zero if it needs to
* read more data, or 1 once we can go on with next request or end the session.
* When in MSG_DATA or MSG_TRAILERS, it will automatically forward chunk_len
- * bytes of pending data + the headers if not already done (between sol and sov).
- * It eventually adjusts sol to match sov after the data in between have been sent.
+ * bytes of pending data + the headers if not already done.
*/
int http_request_forward_body(struct session *s, struct channel *req, int an_bit)
{
@@ -5065,11 +5060,10 @@
http_silent_debug(__LINE__, s);
/* we may have some pending data starting at req->buf->p */
- if (msg->chunk_len || msg->sov) {
- msg->chunk_len += msg->sov;
+ if (msg->chunk_len || msg->next) {
+ msg->chunk_len += msg->next;
msg->chunk_len -= channel_forward(req, msg->chunk_len);
- msg->next -= msg->sov;
- msg->sov = 0;
+ msg->next = 0;
}
if (msg->msg_state == HTTP_MSG_DATA) {
@@ -5087,7 +5081,7 @@
}
else if (msg->msg_state == HTTP_MSG_CHUNK_SIZE) {
/* read the chunk size and assign it to ->chunk_len, then
- * set ->sov and ->next to point to the body and switch to DATA or
+ * set ->next to point to the body and switch to DATA or
* TRAILERS state.
*/
int ret = http_parse_chunk_size(msg);
@@ -6153,8 +6147,7 @@
* be between MSG_BODY and MSG_DONE (inclusive). It returns zero if it needs to
* read more data, or 1 once we can go on with next request or end the session.
* When in MSG_DATA or MSG_TRAILERS, it will automatically forward chunk_len
- * bytes of pending data + the headers if not already done (between sol and sov).
- * It eventually adjusts sol to match sov after the data in between have been sent.
+ * bytes of pending data + the headers if not already done.
*/
int http_response_forward_body(struct session *s, struct channel *res, int an_bit)
{
@@ -6195,7 +6188,6 @@
*/
channel_forward(res, msg->sov);
msg->next = 0;
- msg->sov = 0;
if (msg->flags & HTTP_MSGF_TE_CHNK)
msg->msg_state = HTTP_MSG_CHUNK_SIZE;
@@ -6217,11 +6209,10 @@
/* we may have some pending data starting at res->buf->p */
if (s->comp_algo == NULL) {
- if (msg->chunk_len || msg->sov) {
- msg->chunk_len += msg->sov;
+ if (msg->chunk_len || msg->next) {
+ msg->chunk_len += msg->next;
msg->chunk_len -= channel_forward(res, msg->chunk_len);
- msg->next -= msg->sov;
- msg->sov = 0;
+ msg->next = 0;
}
}
@@ -6266,13 +6257,12 @@
if (compressing) {
b_adv(res->buf, msg->next);
msg->next = 0;
- msg->sov = 0;
}
/* we're in MSG_CHUNK_SIZE now, fall through */
case HTTP_MSG_CHUNK_SIZE - HTTP_MSG_DATA:
/* read the chunk size and assign it to ->chunk_len, then
- * set ->sov and ->next to point to the body and switch to DATA or
+ * set ->next to point to the body and switch to DATA or
* TRAILERS state.
*/
@@ -6289,7 +6279,6 @@
/* skipping data if we are in compression mode */
b_adv(res->buf, msg->next);
msg->next = 0;
- msg->sov = 0;
} else {
if (consumed_data) {
http_compression_buffer_end(s, &res->buf, &tmpbuf, 1);
@@ -6380,11 +6369,10 @@
/* forward any pending data starting at res->buf->p */
if (s->comp_algo == NULL) {
- if (msg->chunk_len || msg->sov) {
- msg->chunk_len += msg->sov;
+ if (msg->chunk_len || msg->next) {
+ msg->chunk_len += msg->next;
msg->chunk_len -= channel_forward(res, msg->chunk_len);
- msg->next -= msg->sov;
- msg->sov = 0;
+ msg->next = 0;
}
}