MINOR: filters/http: Slightly update the parsing of chunks

Now, http_parse_chunk_size and http_skip_chunk_crlf return the number of bytes
parsed on success. http_skip_chunk_crlf does not use msg->sol anymore.

On the other hand, http_forward_trailers is unchanged. It returns >0 if the end
of trailers is reached and 0 if not. In all cases (except if an error is
encountered), msg->sol contains the length of the last parsed part of the
trailer headers.

Internal doc and comments about msg->sol has been updated accordingly.

doc/internals/body-parsing.txt

@@ -87,7 +87,9 @@ msg.sol  : start of line. Points to the beginning of the current header line
            states starting with DATA. For chunked encoded messages, this sum
            always corresponds to the beginning of the current chunk of data as
            it appears in the buffer, or to be more precise, it corresponds to
-           the first of the remaining bytes of chunked data to be inspected.
+           the first of the remaining bytes of chunked data to be inspected. In
+           TRAILERS state, it contains the length of the last parsed part of
+           the trailer headers.
 
 msg.eoh  : end of headers. Points to the CRLF (or LF) preceeding the body and
            marking the end of headers. It is where new headers are appended.

include/types/proto_http.h

@@ -296,10 +296,12 @@ enum {
  *                             to a byte matching the current state.
  *
  *  - sol (start of line)    : start of current line before MSG_BODY. Starting
- *                             from MSG_BODY, contains the length of the last
+ *                             from MSG_BODY and until MSG_TRAILERS, contains
- *                             parsed chunk size so that when added to sov it
+ *                             the length of the last parsed chunk size so that
- *                             always points to the beginning of the current
+ *                             when added to sov it always points to the
- *                             data chunk.
+ *                             beginning of the current data chunk.
+ *                             in MSG_TRAILERS state, it contains the length of
+ *                             the last parsed part of the trailer headers.
  *
  *  - eol (End of Line)      : Before HTTP_MSG_BODY, relative offset in the
  *                             buffer of the first byte which marks the end of

src/proto_http.c

@@ -2095,13 +2095,13 @@ void http_change_connection_header(struct http_txn *txn, struct http_msg *msg, i
 	return;
 }
 
-/* Parse the chunk size at msg->next. Once done, it adjusts ->next to point to
+/* Parse the chunk size at msg->next. Once done, caller should adjust ->next to
- * the first byte of data after the chunk size, so that we know we can forward
+ * point to the first byte of data after the chunk size, so that we know we can
- * exactly msg->next bytes. msg->sol contains the exact number of bytes forming
+ * forward exactly msg->next bytes. msg->sol contains the exact number of bytes
- * the chunk size. That way it is always possible to differentiate between the
+ * forming the chunk size. That way it is always possible to differentiate
- * start of the body and the start of the data.
+ * between the start of the body and the start of the data.  Return the number
- * Return >0 on success, 0 when some data is missing, <0 on error.
+ * of byte parsed on success, 0 when some data is missing, <0 on error.  Note:
- * Note: this function is designed to parse wrapped CRLF at the end of the buffer.
+ * this function is designed to parse wrapped CRLF at the end of the buffer.
  */
 static inline int http_parse_chunk_size(struct http_msg *msg)
 {
@@ -2181,40 +2181,42 @@ static inline int http_parse_chunk_size(struct http_msg *msg)
 			goto error;
 	}
 
-	/* OK we found our CRLF and now <ptr> points to the next byte,
+	/* OK we found our CRLF and now <ptr> points to the next byte, which may
-	 * which may or may not be present. We save that into ->next,
+	 * or may not be present. We save the number of bytes parsed into
-	 * and the number of bytes parsed into msg->sol.
+	 * msg->sol.
 	 */
 	msg->sol = ptr - ptr_old;
 	if (unlikely(ptr < ptr_old))
 		msg->sol += buf->size;
 	msg->chunk_len = chunk;
 	msg->body_len += chunk;
-	return 1;
+	return msg->sol;
  error:
 	msg->err_pos = buffer_count(buf, buf->p, ptr);
 	return -1;
 }
 
-/* This function skips trailers in the buffer associated with HTTP
+/* This function skips trailers in the buffer associated with HTTP message
- * message <msg>. The first visited position is msg->next. If the end of
+ * <msg>. The first visited position is msg->next. If the end of the trailers is
- * the trailers is found, it is automatically scheduled to be forwarded,
+ * found, the function returns >0. So, the caller can automatically schedul it
- * msg->msg_state switches to HTTP_MSG_DONE, and the function returns >0.
+ * to be forwarded, and switch msg->msg_state to HTTP_MSG_DONE. If not enough
- * If not enough data are available, the function does not change anything
+ * data are available, the function does not change anything except maybe
- * except maybe msg->next if it could parse some lines, and returns zero.
+ * msg->sol if it could parse some lines, and returns zero.  If a parse error
- * If a parse error is encountered, the function returns < 0 and does not
+ * is encountered, the function returns < 0 and does not change anything except
- * change anything except maybe msg->next. Note that the message must
+ * maybe msg->sol. Note that the message must already be in HTTP_MSG_TRAILERS
- * already be in HTTP_MSG_TRAILERS state before calling this function,
+ * state before calling this function, which implies that all non-trailers data
- * which implies that all non-trailers data have already been scheduled for
+ * have already been scheduled for forwarding, and that msg->next exactly
- * forwarding, and that msg->next exactly matches the length of trailers
+ * matches the length of trailers already parsed and not forwarded. It is also
- * already parsed and not forwarded. It is also important to note that this
+ * important to note that this function is designed to be able to parse wrapped
- * function is designed to be able to parse wrapped headers at end of buffer.
+ * headers at end of buffer.
  */
 static int http_forward_trailers(struct http_msg *msg)
 {
 	const struct buffer *buf = msg->chn->buf;
 
-	/* we have msg->next which points to next line. Look for CRLF. */
+	/* we have msg->next which points to next line. Look for CRLF. But
+	 * first, we reset msg->sol */
+	msg->sol = 0;
 	while (1) {
 		const char *p1 = NULL, *p2 = NULL;
 		const char *start = b_ptr(buf, msg->next + msg->sol);
@@ -2267,15 +2269,15 @@ static int http_forward_trailers(struct http_msg *msg)
 	}
 }
 
-/* This function may be called only in HTTP_MSG_CHUNK_CRLF. It reads the CRLF
+/* This function may be called only in HTTP_MSG_CHUNK_CRLF. It reads the CRLF or
- * or a possible LF alone at the end of a chunk. It automatically adjusts
+ * a possible LF alone at the end of a chunk. The caller should adjust msg->next
- * msg->next in order to include this part into the next forwarding phase.
+ * in order to include this part into the next forwarding phase.  Note that the
- * Note that the caller must ensure that ->p points to the first byte to parse.
+ * caller must ensure that ->p points to the first byte to parse.  It returns
- * It also sets msg_state to HTTP_MSG_CHUNK_SIZE and returns >0 on success. If
+ * the number of bytes parsed on success, so the caller can set msg_state to
- * not enough data are available, the function does not change anything and
+ * HTTP_MSG_CHUNK_SIZE. If not enough data are available, the function does not
- * returns zero. If a parse error is encountered, the function returns < 0 and
+ * change anything and returns zero. If a parse error is encountered, the
- * does not change anything. Note: this function is designed to parse wrapped
+ * function returns < 0. Note: this function is designed to parse wrapped CRLF
- * CRLF at the end of the buffer.
+ * at the end of the buffer.
  */
 static inline int http_skip_chunk_crlf(struct http_msg *msg)
 {
@@ -2303,8 +2305,7 @@ static inline int http_skip_chunk_crlf(struct http_msg *msg)
 		msg->err_pos = buffer_count(buf, buf->p, ptr);
 		return -1;
 	}
-	msg->sol = bytes;
+	return bytes;
-	return 1;
 }
 
 /* Parses a qvalue and returns it multipled by 1000, from 0 to 1000. If the
@@ -4760,8 +4761,7 @@ int http_wait_for_request_body(struct stream *s, struct channel *req, int an_bit
 			stream_inc_http_err_ctr(s);
 			goto return_bad_req;
 		}
-		msg->next += msg->sol;
+		msg->next += ret;
-		msg->sol   = 0;
 		msg->msg_state = msg->chunk_len ? HTTP_MSG_DATA : HTTP_MSG_TRAILERS;
 	}
 
@@ -6889,8 +6889,7 @@ http_msg_forward_chunked_body(struct stream *s, struct http_msg *msg)
 				goto missing_data_or_waiting;
 			if (ret < 0)
 				goto chunk_parsing_error;
-			msg->next += msg->sol;
+			msg->next += ret;
-			msg->sol   = 0;
 			msg->msg_state = HTTP_MSG_CHUNK_SIZE;
 			/* fall through for HTTP_MSG_CHUNK_SIZE */
 
@@ -6904,8 +6903,7 @@ http_msg_forward_chunked_body(struct stream *s, struct http_msg *msg)
 				goto missing_data_or_waiting;
 			if (ret < 0)
 				goto chunk_parsing_error;
-			msg->next += msg->sol;
+			msg->next += ret;
-			msg->sol   = 0;
 			if (msg->chunk_len) {
 				msg->msg_state = HTTP_MSG_DATA;
 				goto switch_states;
@@ -6921,7 +6919,6 @@ http_msg_forward_chunked_body(struct stream *s, struct http_msg *msg)
 					 /* default_ret */ 1,
 					 /* on_error    */ goto error);
 			msg->next += msg->sol;
-			msg->sol   = 0;
 			if (!ret)
 				goto missing_data_or_waiting;
 			break;