Thank you for the bug report;
I catch it. I join a temporary fix, I suspect that the problem to be more
tricky.
Thierry
On Fri, 27 May 2016 16:22:41 -0400
Michael Ezzell <mich...@ezzell.net> wrote:
>
> On Fri, May 27, 2016 at 10:41 AM, Thierry FOURNIER <tfourn...@arpalert.org>
> wrote:
>
> > Hi,
> >
> > Thank you for the bug repport. It was perfect. I found and fixed the
> > bug. You can try the patch in attachement.
> >
> >
> Thanks, Thierry.
>
> The converter is now able to be called, and returns a value as expected...
> however, there does appear to be another bug that this fix has uncovered,
> and perhaps this is something more serious.
>
> Somehow, I am able to corrupt the *name* of the header I was trying to add,
> if I call core.Alert() within the converter that is processing the header's
> value.
>
> First, a working example of a very simple converter:
>
> Lua:
>
> testfn = function(str)
> return ("original value was '" .. str .."'");
> end
>
> core.register_converters('testconv',testfn);
>
> Configuration uses this to process a value for a header:
>
> http-request set-header X-Empty-Header %[str("test"),lua.testconv]
>
> This now works exactly as expected, and I see this header in the request:
>
> X-Empty-Header: original value was 'test'
>
> However, if I call core.Alert() within the converter, the alert string
> actually overwrites part of the header *name* that I was trying to set. (!?)
>
> testfn = function(str)
> core.Alert("Stomp");
> return ("original value was '" .. str .."'");
> end
>
> core.register_converters('testconv',testfn);
>
> Now, after adding the alert message to the Lua converter, with everything
> else identical, here is the problem: in the HTTP request, the 5 byte
> "Alert" message I sent ("Stomp") has actually overwritten 6 bytes
> ("X-Empt") in the header name I was trying to add.
>
> Config:
>
> http-request set-header X-Empty-Header %[str("test"),lua.testconv]
>
> Request:
>
> Stompy-Header: original value was 'test'
>
> Clearly, that shouldn't be possible.
>
> HAProxy also kills the request with "PH--" (doc: "One possible cause for
> this error is an invalid syntax in an HTTP header name containing
> unauthorized characters." ... such as the \0 that I assume may be hiding at
> the end of "Stomp," which would explain why 5 characters appear to be
> replacing 6.)
>
> Please advise.
--
>From 45c7d8ea7aa75ace764d475f491c92fb390ad56b Mon Sep 17 00:00:00 2001
From: Thierry Fournier <thierry.fourn...@ozon.io>
Date: Mon, 30 May 2016 16:20:49 +0200
Subject: [PATCH] BUG/MEDIUM: lua/http: add-header corruption
The common buffer "trash" is used by the "set-header" action. This action can
call a lot of fetches and converters. One of these converters can be a lua
function. The Lua function can use the core.log() collection functions. The
core.log functions uses the common buffer "trash".
So, the value was overwritten in the common "trash" buffer, and the buffer added
is xorrupted.
This patch replace the usage of the trash buffer by the result of the function
"get_trash_chunk()". I suppose that it will be better to replace the usage if
trash in the "add header" action.
---
src/hlua.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/hlua.c b/src/hlua.c
index 953c219..82f183f 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -779,11 +779,14 @@ static inline void hlua_sendlog(struct proxy *px, int level, const char *msg)
{
struct tm tm;
char *p;
+ struct chunk *tmp;
+
+ tmp = get_trash_chunk();
/* Cleanup the log message. */
- p = trash.str;
+ p = tmp->str;
for (; *msg != '\0'; msg++, p++) {
- if (p >= trash.str + trash.size - 1) {
+ if (p >= tmp->str + tmp->size - 1) {
/* Break the message if exceed the buffer size. */
*(p-4) = ' ';
*(p-3) = '.';
@@ -798,12 +801,12 @@ static inline void hlua_sendlog(struct proxy *px, int level, const char *msg)
}
*p = '\0';
- send_log(px, level, "%s\n", trash.str);
+ send_log(px, level, "%s\n", tmp->str);
if (!(global.mode & MODE_QUIET) || (global.mode & (MODE_VERBOSE | MODE_STARTING))) {
get_localtime(date.tv_sec, &tm);
fprintf(stderr, "[%s] %03d/%02d%02d%02d (%d) : %s\n",
log_levels[level], tm.tm_yday, tm.tm_hour, tm.tm_min, tm.tm_sec,
- (int)getpid(), trash.str);
+ (int)getpid(), tmp->str);
fflush(stderr);
}
}
--
2.1.4
