Hi Aleks,
Le 20/10/2026 à 11:48 AM, Aleksandar Lazic a écrit :
Thanks! I agree, implementing it is better than documenting the gap.
I would call it only for the response message (&txn->rsp), after
sc_reset_endp() succeeded and before the L7 buffer swap:
- sc_reset_endp() is the last step of do_l7_retry() that may fail. Once
it succeeded the retry is certain, so a filter never resets its
context for a retry that does not happen.
- Before the swap and before b_reset(&res->buf), the failed response is
still in the buffer and txn->status is set. That matches the 1xx case,
where http_reset() is called before channel_htx_fwd_headers(), while
the interim response is still visible. The request that is sent again
is seen by the filters in the new callback anyway (see below).
- Only the response is reset from the filters' point of view: the
request callbacks (http_headers/payload/end) are not replayed on a L7
retry, the request is sent again from the L7 buffer as is.
While testing with the trace filter I noticed that
channel_start_analyze() is called a second time on the response channel
after a L7 retry (the response analyzers are set again by
back_establish()), without a channel_end_analyze() in between. With
http_reset() called just before, a filter at least gets a hint. Should
this be documented too, or do you prefer to avoid the second call?
If that's fine for you, I will replace the DOC patch with one that calls
http_reset() on L7 retries and updates the filters documentation.
Yes, you can add http_reset() call into do_l7_retry() and update the doc. Thanks
! Good point about calls to channel_start_analyze() on each retry. I guess we
must keep it because it is performed after the connection is established. At
first glance, it can seem strange, but it is the place where the response
analyzers are set. And channel_start_analyze() is an analyzer. So it cannot
easily be changed.
OK, I will move it out of stconn.c. But I'm not sure stream_set_target()
fits a filter that must sign the request:
- It is also called during the request analysis (use-server and
force-persist in process_server_rules(), cookie and stick persistence,
RDP cookie), before AN_REQ_HTTP_INNER and the http_headers callbacks of
the other filters, so the request is not final yet at that point.
Well, it is called with NULL to just unset target already set on the stream. I
don't know yet if the filters must be called in that case. Maybe it could be
useful. In that case, we must be sure to call filters when the stream's target
was previously set (so on retries).
Then it is called with a target was selected, a server or an applet. In that
case, this unset the old target, if any, and set a new one. At this stage, the
request headers was already fully analyzed. And in fact, http_payload was
already called on it and the headers were marked to be forwarded. And this point
could be a bit tricky, the channel's output value must be updated accordingly if
the HTX message is altered.
- It returns void and is called from many places, so a filter cannot
report a failure there and stop the request before it is sent. For a
signing filter that's a requirement: without valid credentials the
request must not leave HAProxy.
Hum, that's a good point. Eventually we can move the callback elsewhere. For
instance in back_handle_st_req() after the call to srv_redispatch_connect(). In
that case the callback must be renamed to stream_set_server().
- It is also called with a NULL target to clear it, e.g. at the
beginning of assign_server() before the LB picks a server.
What about a callback at the beginning of connect_server() instead? The
target is known there, it's called once per connection attempt
(redispatch, retries, L7 retries), it's not on the sending path, and it
already returns SF_ERR_INTERNAL, which back_try_conn_req() handles
properly (no retry, error reported to the client, log line). A negative
return value from the filter would map to SF_ERR_INTERNAL.
At first glance, back_handle_st_req() is better because on retry, it will not be
called again, except on redispatch. And I prefer to not add more dependencies on
the stream in connect_server().
It only covers servers, not applets. If filters also be able to be
notified when an applet is selected, a separate informational
stream_set_target callback could be added for that.
We can leave the server applets aside for now.
Sure. Apart from OpenSSL it has no external dependency, but it relies on
several internal APIs (httpclient, HTX, filters, traces, JWT), so I would
prefer to keep it in the tree under addons/cloudauth, like promex. But
if you prefer a separate repository like
https://github.com/git001/haproxy-cloudauth, that works for me too. What do you
prefer?
I saw your repository. It is a good idea to work in a dedicated repository for
now indeed. This way, no reason to wait for review on our side to merge commits.
During the dev stage, this will be easier for you I guess.
--
Christopher Faulet
