Hi David,

On Tue, Dec 28, 2010 at 11:27:02AM +0900, David Cournapeau wrote:
> On Mon, Dec 27, 2010 at 2:59 PM, Willy Tarreau <[email protected]> wrote:
> > Hi David,
> >
> > On Thu, Dec 23, 2010 at 08:55:41PM +0900, David Cournapeau wrote:
> >> The patch does work because I set data->str.len to url_param_value_l +
> >> 1, but I don't understand why this works. I have checked that
> >> url_param_value_l does correspond to the actual string length,
> >
> > It is possible that you uncovered a bug, I'll check that tomorrow.
> 
> Ok. Let me know if you need more information or different test to
> uncover the potential bug (I have a simple setup with dummy python
> servers to track sessions easily).

OK I found the bug. The string keys were not always zero-terminated
during lookups, but the zero was checked after the lookup, so once
you entered a key that had another one as a prefix, it would make
it unreachable.

It's fixed in this commit :

   http://git.1wt.eu/web?p=haproxy.git;a=commitdiff;h=035da6

I've checked your patch (without the +1) and overall it seems to work
as expected. However I've found a minor issue which once fixed will
result in simpler and faster code. Function find_query_string_pos()
searches for the last slash then the first question mark. This prevents
query strings containing slashes from being correctly parsed.

The fix is easy and will make the code look better (so you'll be able
to get rid of your comment about the ugliness). We'll just use a
memchr(path, '/', path_l) just as we do in backend.c:get_server_ph().

I can perform the change myself if you want, then commit the patch.
Please just tell me if you're OK with that.

Thanks,
Willy


Reply via email to