Hello Willy,


As per my findings, the existing "httpchk" directive contains header and body 
as single argument args[4] and there is no definite identifier to differentiate 
header from body.

Therefore, i have added 2 new options to http-check directive for header and 
body. Specifying these options would be mandatory if "http-check expect" option 
is configured in haproxy.cfg file. If "http-check expect" is configured but 
"http-check header" and/or "http-check body" not configured, it would result 
into an error logged in haproxy.log



Following is the pseudocode in src/checks.c file.



if check->type == PR_O2_HTTP_CHK

     /* prevent HTTP keep-alive when "http-check expect" is used */

     if s->proxy->options2 & PR_O2_EXP_TYPE

           if  "http-check header" and  "http-check body" present in the 
haproxy.cfg file

                b_putblk(&check->bo, s->proxy->header_check_req, 
s->proxy->header_check_len);

                b_putist(&check->bo, ist("Connection: close\r\n"));

                b_putblk(&check->bo, s->proxy->body_check_req, 
s->proxy->body_check_len); }

           else

                Error logged in haproxy.log ErrorMsg=Specify the header and body

           endif

     else

                b_putblk(&check->bo, s->proxy->check_req, s->proxy->check_len)

     endif

endif



Thanks And Regards

Kiran Gavali



-----Original Message-----
From: Willy Tarreau [mailto:w...@1wt.eu]
Sent: Monday, February 24, 2020 1:26 PM
To: Kiran Gavali <kiran.gav...@india.nec.com>
Subject: Re: HAProxy v2.1.0 health checks with POST and data corrupted by extra 
"connection: close"



Hello Kiran,



On Mon, Feb 24, 2020 at 07:00:08AM +0000, Kiran Gavali wrote:

> Hello Mr. Willy Tarreau ,

>

> Issue Link: https://github.com/haproxy/haproxy/issues/16



Oh I didn't notice that you've updated the report there a month ago, sorry 
about that, I get so many github notifications that I do miss quite some of 
them :-(



> I have reproduced and analyzed the above bug on HAProxy v2.1.0 (using 
> Wireshark).

> As per my findings, the "Connection: close" string is appended after

> the data block instead of being appended after the header. [Refer the

> attached Wireshark screenshot at the end]



Yes absolutely, that's the problem (and limitation) described there.



> As stated by @wtarreau<https://github.com/wtarreau> in [

> #https://www.mail-archive.com/haproxy@formilux.org/msg28200.html], it

> would be best to add another option "body" to http-check directive so

> as to correctly differentiate body from header, as shown below:-

>

> option httpchk POST / HTTP/1.1\r\n

> http-check header Host: 10.10.26.236\r\nContent-Type:

> application/json\r\nContent-Length: 38\r\n http-check body

> {"command":"getNodeInfo"} http-check expect rstatus (2|3)[0-9][0-9]

>

> To incorporate the change, we need to do changes in below code:-

> File: cfgparse-listen.c

> Function: int cfg_parse_listen(const char *file, int linenum, char

> **args, int kwm) Code Snippet: curproxy->check_len =

> snprintf(curproxy->check_req, reqlen, "%s %s %s\r\n", args[2],

> args[3], *args[4]?args[4]:"HTTP/1.0");

>

> Please let me know if my understanding of the above issue is correct,

> so that I can proceed further accordingly. Looking forward for your guidance.



Your analysis is correct. However this issue is more than two years old now and 
the check system is currently being reworked in order to support muxes for 
HTTP/1 and HTTP/2 so that we have better checks in 2.2. However I don't know if 
for this specific issue we can come up with something trivial enough to be 
backported to older releases with no risk at all.

I guess that it might be worth trying, but let me be clear about the fact that 
if the code becomes tricky, we may finally decide not to merge it and/or not to 
backport it.



Do not hesitate to share your findings on the mailing list so that others can 
follow the progress and possibly suggest adjustments.



Thanks!

Willy

________________________________
The contents of this e-mail and any attachment(s) are confidential and intended 
for the named recipient(s) only. It shall not attach any liability on the 
originator or NECTI or its affiliates. Any views or opinions presented in this 
email are solely those of the author and may not necessarily reflect the 
opinions of NECTI or its affiliates. Any form of reproduction, dissemination, 
copying, disclosure, modification, distribution and / or publication of this 
message without the prior written consent of the author of this e-mail is 
strictly prohibited. If you have received this email in error please delete it 
and notify the sender immediately.

Reply via email to