Hi Samuel,
On Thu, Sep 08, 2022 at 09:04:22AM +0200, Samuel Maftoul wrote:
> Hi,
>
> This is the continuation of a discussion that happened on github (
> https://github.com/haproxy/haproxy/pull/1853).
Thanks for this.
> Here, I applied Willy's 3rd proposal: adding a keyword to the forwardfor
> option to disable the header. I used 'none' as the keyword, as I looked in
> the documentation to see if I should use 'disable' or 'disabled', it looked
> to me 'disable' is more consistent, but 'none' is even more consistent (we
> already have options using 'none' as keyword, but no option uses 'disable'
> as keyword).
>
> Also, the implementation is simpler than what was proposed in the PR,
> Christopher, I think I don't need to free anything, and also , I added the
> edge case you cited as a test.
>
> Is this approach right ?
I think the approach is right, however I suspect that it won't work if
the option is enabled in the frontend, which I understood was the initial
concern. In your case it will only disable it if it was present in the
defaults section, which a "no option ..." should handle right.
If we want to cover the cancellation of a frontend option, I suspect that
we need to set a flag on the backend to indicate that the header must not
be added when the request leaves, so that it can cancel a possible
forwardfor in the frontend.
So there are two possibilities:
- "no option ..." => revert to default settings for the option, i.e.
do not force it enabled, which means that it can revert what appears
in the defaults section, but that frontend presence remains sufficient
to enable itx
- "option forwardfor none" => explicitly disable XFF addition for requests
flowing through this backend, even if they passed through a frontend
which had the option explicitly enabled.
We can even make sure to support both, by the way, if "no option..." does
not work right now.
Hoping this clarifies it a little bit,
Willy
