search

sapcc / octavia-f5-provider-driver

Apache License 2.0
15 stars 2 forks source link

HTTP Monitor generated receive string is not intended regex #219

Open notandy opened 1 year ago

notandy commented 1 year ago

If in octavia an expected_codes is defined as a range (e.g. 200-399), the generated regex is not correct. Currently, it looks like this:

HTTP/1.(0|1) [200-399]

But it should look like this:

HTTP\/1.(0|1) [2-3][0-9][0-9]

see https://github.com/sapcc/octavia-f5-provider-driver/blob/d3ccf2bebf9910c2a663a2d56681dcff9dd4c733/octavia_f5/restclient/as3objects/monitor.py#L156 for implementation details

BenjaminLudwigSAP commented 1 year ago

This value is used as the receive parameter of the monitor AS3 class. Since its default value is "HTTP/1." the forward slash probably does not have to be escaped. Both this question as well as the character class behavior (which you're most probably right about) depend on the regex engine being used for evaluating the receive parameter. @m-kratochvil can you please check which one it is?

m-kratochvil commented 1 year ago

Yes, this is documented in:https://my.f5.com/manage/s/article/K5917

With some exceptions, health monitors on BIG-IP systems support the use of POSIX Extended Regular Expressions (ERE). ERE syntax treats most characters as literals, meaning that they match only themselves, and defines metacharacters, which can be used to represent special characters, multiple characters, or a sequences of characters.

I also agree that the forward slash does not seem to require escape in the Big-IP monitor configuration, judging by the fact we normally use expression like HTTP/1.(0|1) 200 OK\r\n\r\n and it works. Testing in various online regex validators gets me varying results.

notandy commented 1 year ago

it's not about the forward slash....

m-kratochvil commented 1 year ago

Of course not. The regex [200-399] is wrong (even though it works most of the times), at least for the use case of matching HTTP response codes 200 - 399. AFAIK it breaks down to 20 0-3 99 within single character class which is not what we want, not only is the logic broken but it won't match for example any middle digits in the range 4-9 (not that there are any known codes like that but that's beside the point), and it will match for example 002.

m-kratochvil commented 1 year ago

Another use-case where this bug surfaced: https://convergedcloud.slack.com/archives/CSP5GMKD1/p1687332168839529