Le 26/03/2020 à 08:36, Willy Tarreau a écrit :
Hi Christopher,

On Thu, Mar 26, 2020 at 07:40:06AM +0100, Christopher Faulet wrote:
Thanks ! However there are many problems with your patch. First there is no
commit message. You must explain what is exactly the problem and how you fix
it. Then, the documentation must be updated. You introduced 2 new options
without any explanation. Without these info, it is really hard to figure out
what the patch do.

I noticed these as well, thanks for pointing out before me :-)

But reading how headers and body buffers are inserted in
the request, I suspect some bugs. A reg-test is probably necessary to
validate such feature.

Yes that would indeed be useful.

But, don't bother for now submitting a new patch. I'm currently refactoring
the tcp-checks. But on my todo list, the next big step is the http-check
refactoring. It is painful but I hope to finish the refactoring of the
checks for the 2.2.0.

My main concern is the backward compatibility. Kiran's patch is small
enough to be backported, so that might constitute a possible solution
for existing setups. However we cannot afford to do that if it works
differently than what we'll have in the final release. As such I think
it's important to quickly figure how we want such a separate body part
to be *configured* and make sure that it's done the same in your new
version and in Kiran's patch. If so, then we could imagine merging it
early to have it backported so that it's ultimately replaced in 2.2 by
your work once ready.

What do you think ?


It is a good idea. For now, I have only few idea though. For the header part, it must not be a raw block. Because it will be really hard to keep the same syntax with the HTX. I propose to keep same syntax than http-request rules :

http-check add-header <name> <value>

And later :

http-check set-header <name> <value>
http-check del-header <name>
http-check replace-header <name> <regex> <value>
http-check replace-value <name> <regex> <value>

Another idea could be to have a syntax similar to the HTTP return action :

http-check hdr <name> <value> hdr <name> <value> string <body-str>

or

http-check hdr <name> <value> hdr <name> <value> file <body-file>

The request URI is passed on the "option httpchk" line. But, maybe the method, the path and the version may also be defined on such line.

There are many other problems to deal with. But it is a first step. The content-length must be automatically deduced when a body is defined. For current versions, the connection header must also be added the same way it is done today.

During my refactoring, I can try to first work on a way to support such syntax on current versions. But we must be sure to have the right syntax first. It is the harder part :)

--
Christopher Faulet

Reply via email to