Hi Lokesh,
On Sun, Sep 17, 2023 at 09:45:31PM -0700, Lokesh Jindal wrote:
> Hello Willy
>
> Thanks for the detailed explanations and the feedback.
> PFA the two patches based on your feedback. My apologies for the delay.
Thanks, I've applied them now.
> > - maybe a combination of both, i.e. return 0 as long as the caller does
> > not pass SMP_OPT_FINAL, or return a truncated response.
>
>
> Sounds good. The revised patch uses option 3 above.
OK I also think it's the most versatile choice.
> > > /* Try to decode a variable. */
> > > - if (vars_check_arg(&args[0], NULL))
> > > + if (vars_check_arg(&args[0], err))
> > > return 1;
>
>
> > I can't say if this change was intentional or not, nor if the NULL had a
> > particular reason or not, but at least this is unrelated to your patch
> > and would need to be in its own patch, so that in case of regression we
> > can decide whether to fix or revert it. And if it really fixes an issue
> > maybe we'll want to backport it.
>
> This was intentional. I thought it was a "miss" in the existing code. This
> change is now part of the second patch.
Perfect, thanks.
> I have also fixed indentation/tabs, column width, etc. based on your
> feedback.
>
>
> I have a Q for you. Pasting code from the revised patch.
> + // arg1 value is greater than the remaining length
> + if (smp->opt & SMP_OPT_FINAL) {
> + // truncate to remaining length
> + length = smp->data.u.str.data - start_idx;
> + } else {
> + smp->data.u.str.data = 0;
> + return 0;
> + }
>
> I tried to write a test in the vtc file for the "else" clause above, but
> could not figure out when we will hit that condition. Here is what I tried.
> Added following the config section.
> + http-response set-header hdr_bytes_0_7
> "%[res.hdr(input),bytes(0,7)]"
> + http-response set-header hdr_bytes_0_7_concat
> "%[res.hdr(input),bytes(0,7),concat()]"
>
> Added following in the client section
> + # since "bytes" is the non-final operator, response would be empty
> + expect resp.http.hdr_bytes_0_7_concat == ""
>
> But the test failed:
> ---- c2 EXPECT resp.http.hdr_bytes_0_7_concat (012345) == "" failed
>
> Anyway, I can add this such a test in another patch if needed and time
> permits. Let me know what you think.
You can't do these using HTTP rules because they're only evaluated once
the message is complete. The only case these are used is in TCP rules,
because TCP rules can also process HTTP by decoding it on the fly. For
example you can have a TCP frontend that inspects protocol, and if it
detects HTTP with a certain header or content, routes it to an HTTP
backend, otherwise routes it to a TCP frontend. And more generally at
the TCP level it's more common to process bytes than at the HTTP level.
We could for instance imagine that you extract bytes from an SSL Client
Hello message to figure an offset and a length, then assign these to a
pair of variables, and reuse them with bytes() with your modification.
But raw TCP is less easy to manipulate in VTC, and stuff like this is
prone to time races, which regularly cause false positives, and the best
is the ennemy of good, so no worries :-)
Thanks!
Willy
