On Mon, Sep 24, 2012 at 08:42:31PM +0200, Willy Tarreau wrote:
> I'm looking into this, thanks for reporting.
OK got it. It was the use of an offset prior to its initialization
when the error lies on the first line. So it displays last known len
which was the one of last request. The response path suffered from
the same issue BTW.
The patch is attached.
Thanks,
Willy
>From e92693af2690e735d429d723c1dbc300c7a48637 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w...@1wt.eu>
Date: Mon, 24 Sep 2012 21:13:39 +0200
Subject: [PATCH] BUG: http: do not print garbage on invalid requests in debug
mode
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Cyril Bonté reported a mangled debug output when an invalid request
was sent with a faulty request line. The reason was the use of the
msg->sl.rq.l offset which was not yet initialized in this case. So
we change the way to report such an error so that first we initialize
it to zero before parsing a message, then we use that to know whether
we can trust it or not. If it's still zero, then we display the whole
buffer, truncated by debug_hdr() to the first CR or LF character, which
results in the first line only.
The same operation was performed for the response, which was wrong too.
---
src/proto_http.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/src/proto_http.c b/src/proto_http.c
index 8a43e8b..983d394 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -1319,6 +1319,7 @@ void http_msg_analyzer(struct http_msg *msg, struct
hdr_idx *idx)
bi_fast_delete(&buf->buf, ptr - buf->buf.p);
}
msg->sol = 0;
+ msg->sl.st.l = 0; /* used in debug mode */
hdr_idx_init(idx);
state = HTTP_MSG_RPVER;
goto http_msg_rpver;
@@ -1384,9 +1385,7 @@ void http_msg_analyzer(struct http_msg *msg, struct
hdr_idx *idx)
bi_fast_delete(&buf->buf, ptr - buf->buf.p);
}
msg->sol = 0;
- /* we will need this when keep-alive will be supported
- hdr_idx_init(idx);
- */
+ msg->sl.rq.l = 0; /* used in debug mode */
state = HTTP_MSG_RQMETH;
goto http_msg_rqmeth;
}
@@ -2068,7 +2067,11 @@ int http_wait_for_request(struct session *s, struct
channel *req, int an_bit)
char *eol, *sol;
sol = req->buf.p;
- eol = sol + msg->sl.rq.l;
+ /* this is a bit complex : in case of error on the request line,
+ * we know that rq.l is still zero, so we display only the part
+ * up to the end of the line (truncated by debug_hdr).
+ */
+ eol = sol + (msg->sl.rq.l ? msg->sl.rq.l : req->buf.i);
debug_hdr("clireq", s, sol, eol);
sol += hdr_idx_first_pos(&txn->hdr_idx);
@@ -4598,7 +4601,7 @@ int http_wait_for_response(struct session *s, struct
channel *rep, int an_bit)
char *eol, *sol;
sol = rep->buf.p;
- eol = sol + msg->sl.st.l;
+ eol = sol + (msg->sl.st.l ? msg->sl.st.l : rep->buf.i);
debug_hdr("srvrep", s, sol, eol);
sol += hdr_idx_first_pos(&txn->hdr_idx);
@@ -7421,15 +7424,21 @@ unsigned int http_get_hdr(const struct http_msg *msg,
const char *hname, int hle
}
/*
- * Print a debug line with a header
+ * Print a debug line with a header. Always stop at the first CR or LF char,
+ * so it is safe to pass it a full buffer if needed. If <err> is not NULL, an
+ * arrow is printed after the line which contains the pointer.
*/
void debug_hdr(const char *dir, struct session *t, const char *start, const
char *end)
{
int len, max;
len = sprintf(trash, "%08x:%s.%s[%04x:%04x]: ", t->uniq_id, t->be->id,
dir, (unsigned short)si_fd(t->req->prod), (unsigned
short)si_fd(t->req->cons));
- max = end - start;
- UBOUND(max, trashlen - len - 1);
+
+ for (max = 0; start + max < end; max++)
+ if (start[max] == '\r' || start[max] == '\n')
+ break;
+
+ UBOUND(max, trashlen - len - 3);
len += strlcpy2(trash + len, start, max + 1);
trash[len++] = '\n';
if (write(1, trash, len) < 0) /* shut gcc warning */;
--
1.7.9.3.2.g8f78a.dirty
