Hi!
Phew, I was following this one with some concern, fearing it could be something
more serious just waiting to hit us, too ;-)
Great that the issue was found, thanks for that!
There is just one thing I wanted to note regarding
> […] It can be backported in 1.7, 1.6 and 1.5. I finally marked this patch as
> a bug fix.
If I read the patch correctly, even though it is classified as “MINOR” it will
fail with an error on startup, when the configuration has a value outside the
range.
When backporting into the stable branches, this can lead to updates failing
with existing configuration files — which may or may not be what you expect
when doing minor upgrades.
Granted, when you currently use an out-of-range value, you probably _want_ this
fix, but still might hit you unexpectedly.
It should be made very prominent in the release notes.
Cheers,
Daniel
--
Daniel Schneller
Principal Cloud Engineer
CenterDevice GmbH | Hochstraße 11
| 42697 Solingen
tel: +49 1754155711 | Deutschland
[email protected] | www.centerdevice.de
Geschäftsführung: Dr. Patrick Peschlow, Dr. Lukas Pustina,
Michael Rosbach, Handelsregister-Nr.: HRB 18655,
HR-Gericht: Bonn, USt-IdNr.: DE-815299431
> On 21. Jun. 2017, at 17:00, Christopher Faulet <[email protected]> wrote:
>
> Le 13/06/2017 à 14:16, Christopher Faulet a écrit :
>> Le 13/06/2017 à 10:31, siclesang a écrit :
>>> haproxy balances by host,but often captures a part of request header
>>> host or null, and requests balance to default server.
>>>
>>> how to debug it ,
>>>
>> Hi,
>> I'll try to help you. Can you share your configuration please ? It could
>> help to find a potential bug.
>> Could you also provide the tcpdump of a buggy request ?
>> And finally, could you upgrade your HAProxy to the last 1.6 version
>> (1.6.12) to be sure ?
>
> Hi,
>
> Just for the record. After some exchanges in private with siclesang, we found
> the bug in the configuration parser, because of a too high value for
> tune.http.maxhdr. Here is the explanation:
>
> Well, I think I found the problem. This is not a bug (not really). There
> is something I missed in your configuration. You set tune.http.maxhdr to
> 64000. I guess you keep this parameter during all your tests. This is an
> invalid value. It needs to be in the range [0, 32767]. This is mandatory
> to avoid integer overflow. the size of the array where headers offsets
> are stored is a signed short.
>
> To be fair, there is no check on this value during the configuration
> parsing. And the documentation does not specify any range for this
> parameter. I will post a fix very quickly to avoid errors.
>
> BTW, this is a really huge value. The default one is 101. You can
> legitimately increase this value. But there is no reason to have 64000
> headers in an HTTP message. IMHO, 1000/2000 is already an very huge limit.
>
> I attached a patch to improve the configuration parsing and to update the
> documentation. It can be backported in 1.7, 1.6 and 1.5. I finally marked
> this patch as a bug fix.
>
> Thanks siclesang for your help,
> --
> Christopher Faulet
> <0001-BUG-MINOR-cfgparse-Check-if-tune.http.maxhdr-is-in-t.patch>
