Maciej, Am 20.11.20 um 18:45 schrieb Maciej Zdeb: >> +# This config tests various http request rules (set/replace) >> manipulating the >>> +# path, with or without the query-string. It also test path and pathq >> sample >>> +# fetches. >> >> I believe this comment is not correct. >> >>> + http-response del-header x-str2 -m str >>> + http-response del-header x-beg -m beg >>> + http-response del-header end1 -m end >> >> Indentation is messed up there, because you mixed tabs and spaces. >> > True, I was so excited that my code started to work that I didn't notice > this garbage. I'm very sorry for wasting your time.
Please do not apologize. You didn't waste anyone's time. You certainly didn't waste mine. Think about the time you saved for others, because you took the effort to implement this! My remarks all were very minor. >> I now also tested your patch. I noticed the following issues: >> >>> http-response del-header -m beg >> >> This should probably cause a configuration parsing failure, but is >> silently accepted. I am not sure what is does, delete headers called >> `-m`, ignoring the `beg`? >> > Yes, it deleted the `-m` header and ignored `beg`. I've added an > additional check, if an unsupported flag is set. > > Thank you again Tim! > You're welcome. I can't comment on whether you correctly integrated everything, but I can say that the patch now looks good to me based on my knowledge of HAProxy. I hope I saved the maintainers some reviewing effort and I'll leave the final review to them now (adding Willy to Cc). You earned this: Reviewed-by: Tim Duesterhus <[email protected]> Best regards Tim Düsterhus
