[MINOR] HTTP: factorize all the header insertions

Two new functions http_header_add_tail() and http_header_add_tail2()
make it easier to append headers, and also reduce the number of
sprintf() calls and perform stricter checks.

include/proto/buffers.h

@@ -84,8 +84,9 @@ static inline int buffer_realign(struct buffer *buf)
 
 int buffer_write(struct buffer *buf, const char *msg, int len);
 int buffer_write_chunk(struct buffer *buf, struct chunk *chunk);
-int buffer_replace(struct buffer *b, char *pos, char *end, char *str);
+int buffer_replace(struct buffer *b, char *pos, char *end, const char *str);
-int buffer_replace2(struct buffer *b, char *pos, char *end, char *str, int len);
+int buffer_replace2(struct buffer *b, char *pos, char *end, const char *str, int len);
+int buffer_insert_line2(struct buffer *b, char *pos, const char *str, int len);
 int chunk_printf(struct chunk *chk, int size, const char *fmt, ...);
 void buffer_dump(FILE *o, struct buffer *b, int from, int to);
 

src/buffers.c

@@ -75,7 +75,7 @@ int buffer_write_chunk(struct buffer *buf, struct chunk *chunk)
  * If there's no space left, the move is not done.
  *
  */
-int buffer_replace(struct buffer *b, char *pos, char *end, char *str)
+int buffer_replace(struct buffer *b, char *pos, char *end, const char *str)
 {
 	int delta;
 	int len;
@@ -105,7 +105,7 @@ int buffer_replace(struct buffer *b, char *pos, char *end, char *str)
  * same except that the string length is given, which allows str to be NULL if
  * len is 0.
  */
-int buffer_replace2(struct buffer *b, char *pos, char *end, char *str, int len)
+int buffer_replace2(struct buffer *b, char *pos, char *end, const char *str, int len)
 {
 	int delta;
 
@@ -137,6 +137,44 @@ int buffer_replace2(struct buffer *b, char *pos, char *end, char *str, int len)
 }
 
 
+/*
+ * Inserts <str> followed by "\r\n" at position <pos> in buffer <b>. The <len>
+ * argument informs about the length of string <str> so that we don't have to
+ * measure it. It does not include the "\r\n". If <str> is NULL, then the buffer
+ * is only opened for len+2 bytes but nothing is copied in. It may be useful in
+ * some circumstances.
+ *
+ * The number of bytes added is returned on success. 0 is returned on failure.
+ */
+int buffer_insert_line2(struct buffer *b, char *pos, const char *str, int len)
+{
+	int delta;
+
+	delta = len + 2;
+
+	if (delta + b->r >= b->data + BUFSIZE)
+		return 0;  /* no space left */
+
+	/* first, protect the end of the buffer */
+	memmove(pos + delta, pos, b->data + b->l - pos);
+
+	/* now, copy str over pos */
+	if (len && str) {
+		memcpy(pos, str, len);
+		pos[len] = '\r';
+		pos[len + 1] = '\n';
+	}
+
+	/* we only move data after the displaced zone */
+	if (b->r  > pos) b->r  += delta;
+	if (b->w  > pos) b->w  += delta;
+	if (b->lr > pos) b->lr += delta;
+	b->l += delta;
+
+	return delta;
+}
+
+
 /*
  * Does an snprintf() at the end of chunk <chk>, respecting the limit of
  * at most <size> chars. If the size is over, nothing is added. Returns

src/proto_http.c

@@ -311,6 +311,45 @@ static char *cli_stnames[5] = {"HDR", "DAT", "SHR", "SHW", "CLS" };
 static char *srv_stnames[7] = {"IDL", "CON", "HDR", "DAT", "SHR", "SHW", "CLS" };
 #endif
 
+/*
+ * Adds a header and its CRLF at the tail of buffer <b>, just before the last
+ * CRLF. Text length is measured first, so it cannot be NULL.
+ * The header is also automatically added to the index <hdr_idx>, and the end
+ * of headers is automatically adjusted. The number of bytes added is returned
+ * on success, otherwise <0 is returned indicating an error.
+ */
+int http_header_add_tail(struct buffer *b, struct http_msg *msg,
+			 struct hdr_idx *hdr_idx, const char *text)
+{
+	int bytes, len;
+
+	len = strlen(text);
+	bytes = buffer_insert_line2(b, b->data + msg->eoh, text, len);
+	if (!bytes)
+		return -1;
+	msg->eoh += bytes;
+	return hdr_idx_add(len, 1, hdr_idx, hdr_idx->tail);
+}
+
+/*
+ * Adds a header and its CRLF at the tail of buffer <b>, just before the last
+ * CRLF. <len> bytes are copied, not counting the CRLF. If <text> is NULL, then
+ * the buffer is only opened and the space reserved, but nothing is copied.
+ * The header is also automatically added to the index <hdr_idx>, and the end
+ * of headers is automatically adjusted. The number of bytes added is returned
+ * on success, otherwise <0 is returned indicating an error.
+ */
+int http_header_add_tail2(struct buffer *b, struct http_msg *msg,
+			 struct hdr_idx *hdr_idx, const char *text, int len)
+{
+	int bytes;
+
+	bytes = buffer_insert_line2(b, b->data + msg->eoh, text, len);
+	if (!bytes)
+		return -1;
+	msg->eoh += bytes;
+	return hdr_idx_add(len, 1, hdr_idx, hdr_idx->tail);
+}
 
 /*
  * returns a message to the client ; the connection is shut down for read,
@@ -1511,14 +1550,10 @@ int process_cli(struct session *t)
 
 				/* add request headers from the rule sets in the same order */
 				for (cur_idx = 0; cur_idx < rule_set->nb_reqadd; cur_idx++) {
-					int len;
+					if (unlikely(http_header_add_tail(req,
-
+									  &txn->req,
-					len = sprintf(trash, "%s\r\n", rule_set->req_add[cur_idx]);
+									  &txn->hdr_idx,
-					len = buffer_replace2(req, req->data + txn->req.eoh,
+									  rule_set->req_add[cur_idx])) < 0)
-							      req->data + txn->req.eoh, trash, len);
-					txn->req.eoh += len;
-				
-					if (unlikely(hdr_idx_add(len - 2, 1, &txn->hdr_idx, txn->hdr_idx.tail) < 0))
 						goto return_bad_req;
 				}
 			}
@@ -1598,13 +1633,11 @@ int process_cli(struct session *t)
 				int len;
 				unsigned char *pn;
 				pn = (unsigned char *)&((struct sockaddr_in *)&t->cli_addr)->sin_addr;
-				len = sprintf(trash, "X-Forwarded-For: %d.%d.%d.%d\r\n",
+				len = sprintf(trash, "X-Forwarded-For: %d.%d.%d.%d",
 					      pn[0], pn[1], pn[2], pn[3]);
-				len = buffer_replace2(req, req->data + txn->req.eoh,
-						      req->data + txn->req.eoh, trash, len);
-				txn->req.eoh += len;
 
-				if (hdr_idx_add(len - 2, 1, &txn->hdr_idx, txn->hdr_idx.tail) < 0)
+				if (unlikely(http_header_add_tail2(req, &txn->req,
+								   &txn->hdr_idx, trash, len)) < 0)
 					goto return_bad_req;
 			}
 			else if (t->cli_addr.ss_family == AF_INET6) {
@@ -1613,12 +1646,9 @@ int process_cli(struct session *t)
 				inet_ntop(AF_INET6,
 					  (const void *)&((struct sockaddr_in6 *)(&t->cli_addr))->sin6_addr,
 					  pn, sizeof(pn));
-				len = sprintf(trash, "X-Forwarded-For: %s\r\n", pn);
+				len = sprintf(trash, "X-Forwarded-For: %s", pn);
-				len = buffer_replace2(req, req->data + txn->req.eoh,
+				if (unlikely(http_header_add_tail2(req, &txn->req,
-						      req->data + txn->req.eoh, trash, len);
+								   &txn->hdr_idx, trash, len)) < 0)
-				txn->req.eoh += len;
-
-				if (hdr_idx_add(len - 2, 1, &txn->hdr_idx, txn->hdr_idx.tail) < 0)
 					goto return_bad_req;
 			}
 		}
@@ -1628,12 +1658,8 @@ int process_cli(struct session *t)
 		 */
 		if (((t->fe->options | t->be->beprm->options) & PR_O_HTTP_CLOSE) &&
 		    !(t->flags & SN_CONN_CLOSED)) {
-			int len;
+			if (unlikely(http_header_add_tail2(req, &txn->req, &txn->hdr_idx,
-			len = buffer_replace2(req, req->data + txn->req.eoh,
+							   "Connection: close", 17)) < 0)
-					      req->data + txn->req.eoh, "Connection: close\r\n", 19);
-			txn->req.eoh += len;
-
-			if (hdr_idx_add(17, 1, &txn->hdr_idx, txn->hdr_idx.tail) < 0)
 				goto return_bad_req;
 			t->flags |= SN_CONN_CLOSED;
 		}
@@ -2587,14 +2613,8 @@ int process_srv(struct session *t)
 
 			/* add response headers from the rule sets in the same order */
 			for (cur_idx = 0; cur_idx < rule_set->nb_rspadd; cur_idx++) {
-				int len;
+				if (unlikely(http_header_add_tail(rep, &txn->rsp, &txn->hdr_idx,
-
+								  rule_set->rsp_add[cur_idx])) < 0)
-				len = sprintf(trash, "%s\r\n", rule_set->rsp_add[cur_idx]);
-				len = buffer_replace2(rep, rep->data + txn->rsp.eoh,
-				                      rep->data + txn->rsp.eoh, trash, len);
-				txn->rsp.eoh += len;
-				
-				if (unlikely(hdr_idx_add(len - 2, 1, &txn->hdr_idx, txn->hdr_idx.tail) < 0))
 					goto return_bad_resp;
 			}
 
@@ -2621,16 +2641,13 @@ int process_srv(struct session *t)
 			 * requests and this one isn't. Note that servers which don't have cookies
 			 * (eg: some backup servers) will return a full cookie removal request.
 			 */
-			len = sprintf(trash, "Set-Cookie: %s=%s; path=/\r\n",
+			len = sprintf(trash, "Set-Cookie: %s=%s; path=/",
 				      t->be->beprm->cookie_name,
 				      t->srv->cookie ? t->srv->cookie : "; Expires=Thu, 01-Jan-1970 00:00:01 GMT");
 
-			len = buffer_replace2(rep, rep->data + txn->rsp.eoh, rep->data + txn->rsp.eoh, trash, len);
+			if (unlikely(http_header_add_tail2(rep, &txn->rsp, &txn->hdr_idx,
-			txn->rsp.eoh += len;
+							   trash, len)) < 0)
-
-			if (hdr_idx_add(len - 2, 1, &txn->hdr_idx, txn->hdr_idx.tail) < 0)
 				goto return_bad_resp;
-
 			txn->flags |= TX_SCK_INSERTED;
 
 			/* Here, we will tell an eventual cache on the client side that we don't
@@ -2639,13 +2656,8 @@ int process_srv(struct session *t)
 			 * others don't (eg: apache <= 1.3.26). So we use 'private' instead.
 			 */
 			if (t->be->beprm->options & PR_O_COOK_NOC) {
-				//len += sprintf(newhdr + len, "Cache-control: no-cache=\"set-cookie\"\r\n");
+				if (unlikely(http_header_add_tail2(rep, &txn->rsp, &txn->hdr_idx,
-				len = sprintf(trash, "Cache-control: private\r\n");
+								   "Cache-control: private", 22)) < 0)
-
-				len = buffer_replace2(rep, rep->data + txn->rsp.eoh,
-				                     rep->data + txn->rsp.eoh, trash, len);
-				txn->rsp.eoh += len;
-				if (hdr_idx_add(len - 2, 1, &txn->hdr_idx, txn->hdr_idx.tail) < 0)
 					goto return_bad_resp;
 			}
 		}
@@ -2689,12 +2701,8 @@ int process_srv(struct session *t)
 		 */
 		if (((t->fe->options | t->be->beprm->options) & PR_O_HTTP_CLOSE) &&
 		    !(t->flags & SN_CONN_CLOSED)) {
-			int len;
+			if (unlikely(http_header_add_tail2(rep, &txn->rsp, &txn->hdr_idx,
-			len = buffer_replace2(rep, rep->data + txn->rsp.eoh,
+							   "Connection: close", 17)) < 0)
-					      rep->data + txn->rsp.eoh, "Connection: close\r\n", 19);
-			txn->rsp.eoh += len;
-
-			if (hdr_idx_add(17, 1, &txn->hdr_idx, txn->hdr_idx.tail) < 0)
 				goto return_bad_resp;
 			t->flags |= SN_CONN_CLOSED;
 		}