From 9a641b6e59abfd322331af4aa7d891001f9d2c8c Mon Sep 17 00:00:00 2001 From: Ruei-Bang Chen Date: Thu, 26 Oct 2023 18:40:47 -0700 Subject: [PATCH 2/2] MINOR: sample: Get one cookie name per Set-Cookie response header Address the following 2 things based on the review feedback: 1. Treat each Set-Cookie response header as a single cookie, and the Cookie request header as a semi-colon separated list of cookies and extract the cookie names accordingly. 2. Remove deprecated fetchers like cook_names and scook_names, which might bring confusions --- doc/configuration.txt | 2 +- include/haproxy/http.h | 2 +- reg-tests/sample_fetches/cook.vtc | 4 ++-- src/http.c | 18 +++++++++++++----- src/http_fetch.c | 21 ++++++++++----------- 5 files changed, 27 insertions(+), 20 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index 95e0d8809..20e000eb5 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -22641,7 +22641,7 @@ scook_val([]) : integer (deprecated) res.cook_names([]) : string This builds a string made from the concatenation of all cookie names as they - appear in the response (Set-Cookie header) when the rule is evaluated. The + appear in the response (Set-Cookie headers) when the rule is evaluated. The default delimiter is the comma (',') but it may be overridden as an optional argument . In this case, only the first character of is considered. diff --git a/include/haproxy/http.h b/include/haproxy/http.h index 654eba874..299264051 100644 --- a/include/haproxy/http.h +++ b/include/haproxy/http.h @@ -51,7 +51,7 @@ char *http_find_cookie_value_end(char *s, const char *e); char *http_extract_cookie_value(char *hdr, const char *hdr_end, char *cookie_name, size_t cookie_name_l, int list, char **value, size_t *value_l); -char *http_extract_next_cookie_name(char *hdr_beg, const char *hdr_end, +char *http_extract_next_cookie_name(char *hdr_beg, char *hdr_end, int is_req, char **ptr, size_t *len); int http_parse_qvalue(const char *qvalue, const char **end); const char *http_find_url_param_pos(const char **chunks, diff --git a/reg-tests/sample_fetches/cook.vtc b/reg-tests/sample_fetches/cook.vtc index 371dd0731..b0f547215 100644 --- a/reg-tests/sample_fetches/cook.vtc +++ b/reg-tests/sample_fetches/cook.vtc @@ -67,7 +67,7 @@ client c2 -connect ${h2_fe_sock} { txreq -url "/" rxresp expect resp.status == 200 - expect resp.http.cook_names == "cook1,cook2,cook3" + expect resp.http.cook_names == "cook1" } -run # TEST - 3 @@ -128,5 +128,5 @@ client c4 -connect ${h4_fe_sock} { txreq -url "/" rxresp expect resp.status == 200 - expect resp.http.cook_names == "cook1,cook2,cook3,cook4,cook5,cook6" + expect resp.http.cook_names == "cook1,cook4" } -run diff --git a/src/http.c b/src/http.c index 18e15bb02..bacd182d0 100644 --- a/src/http.c +++ b/src/http.c @@ -976,15 +976,17 @@ char *http_extract_cookie_value(char *hdr, const char *hdr_end, * The lookup begins at , which is assumed to be in * Cookie / Set-Cookie header, and the function returns a * pointer to the next position to search from if a valid - * cookie k-v pair is found. When the next cookie name is found, - * will be pointing to the start of the cookie name, - * and will be the length of the cookie name. + * cookie k-v pair is found for Cookie request header and + * for Set-Cookie response header. When the next + * cookie name is found, will be pointing to the start + * of the cookie name, and will be the length of the + * cookie name. * Otherwise if there is no valid cookie k-v pair, * NULL is returned. * The pointer must point to the first character * not part of the Cookie / Set-Cookie header. */ -char *http_extract_next_cookie_name(char *hdr_beg, const char *hdr_end, +char *http_extract_next_cookie_name(char *hdr_beg, char *hdr_end, int is_req, char **ptr, size_t *len) { char *equal, *att_end, *att_beg, *val_beg; @@ -1046,7 +1048,13 @@ char *http_extract_next_cookie_name(char *hdr_beg, const char *hdr_end, */ *ptr = att_beg; *len = att_end - att_beg; - return next + 1; + /* Return next position for Cookie request header and for + * Set-Cookie response header as each Set-Cookie header is assumed to + * contain only 1 cookie + */ + if (is_req) + return next + 1; + return hdr_end; } return NULL; diff --git a/src/http_fetch.c b/src/http_fetch.c index 76c60183e..c18ac9d71 100644 --- a/src/http_fetch.c +++ b/src/http_fetch.c @@ -1833,9 +1833,9 @@ static int smp_fetch_cookie_val(const struct arg *args, struct sample *smp, cons */ static int smp_fetch_cookie_names(const struct arg *args, struct sample *smp, const char *kw, void *private) { - /* possible keywords: req.cook_names / cook_names, res.cook_names / scook_names */ - struct channel *chn = ((kw[0] == 'c' || kw[2] == 'q') ? SMP_REQ_CHN(smp) : SMP_RES_CHN(smp)); - struct check *check = ((kw[0] == 's' || kw[2] == 's') ? objt_check(smp->sess->origin) : NULL); + /* possible keywords: req.cook_names, res.cook_names */ + struct channel *chn = ((kw[2] == 'q') ? SMP_REQ_CHN(smp) : SMP_RES_CHN(smp)); + struct check *check = ((kw[2] == 's') ? objt_check(smp->sess->origin) : NULL); struct htx *htx = smp_prefetch_htx(smp, chn, check, 1); struct http_hdr_ctx ctx; struct ist hdr; @@ -1843,6 +1843,7 @@ static int smp_fetch_cookie_names(const struct arg *args, struct sample *smp, co char del = ','; char *ptr, *attr_beg, *attr_end; size_t len = 0; + int is_req = !(check || (chn && chn->flags & CF_ISRESP)); if (!htx) return 0; @@ -1850,7 +1851,7 @@ static int smp_fetch_cookie_names(const struct arg *args, struct sample *smp, co if (args->type == ARGT_STR) del = *args[0].data.str.area; - hdr = (!(check || (chn && chn->flags & CF_ISRESP)) ? ist("Cookie") : ist("Set-Cookie")); + hdr = (is_req ? ist("Cookie") : ist("Set-Cookie")); temp = get_trash_chunk(); smp->flags |= SMP_F_VOL_HDR; @@ -1863,15 +1864,16 @@ static int smp_fetch_cookie_names(const struct arg *args, struct sample *smp, co while (1) { /* Note: attr_beg == NULL every time we need to fetch a new header */ if (!attr_beg) { - if (!http_find_header(htx, hdr, &ctx, 0)) - goto out; - + /* For Set-Cookie, we need to fetch the entire header line (set flag to 1) */ + if ((is_req && !http_find_header(htx, hdr, &ctx, 0)) || + (!is_req && !http_find_header(htx, hdr, &ctx, 1))) + break; attr_beg = ctx.value.ptr; attr_end = attr_beg + ctx.value.len; } while (1) { - attr_beg = http_extract_next_cookie_name(attr_beg, attr_end, &ptr, &len); + attr_beg = http_extract_next_cookie_name(attr_beg, attr_end, is_req, &ptr, &len); if (!attr_beg) break; @@ -1884,7 +1886,6 @@ static int smp_fetch_cookie_names(const struct arg *args, struct sample *smp, co return 0; } } - out: smp->data.type = SMP_T_STR; smp->data.u.str = *temp; return 1; @@ -2221,7 +2222,6 @@ static struct sample_fetch_kw_list sample_fetch_keywords = {ILH, { { "cookie", smp_fetch_chn_cookie, ARG1(0,STR), NULL, SMP_T_STR, SMP_USE_HRQHV|SMP_USE_HRSHV }, { "cook_cnt", smp_fetch_cookie_cnt, ARG1(0,STR), NULL, SMP_T_SINT, SMP_USE_HRQHV }, { "cook_val", smp_fetch_cookie_val, ARG1(0,STR), NULL, SMP_T_SINT, SMP_USE_HRQHV }, - { "cook_names", smp_fetch_cookie_names, ARG1(0,STR), NULL, SMP_T_STR, SMP_USE_HRQHV }, /* hdr is valid in both directions (eg: for "stick ...") but hdr_* are * only here to match the ACL's name, are request-only and are used for @@ -2303,7 +2303,6 @@ static struct sample_fetch_kw_list sample_fetch_keywords = {ILH, { { "scook", smp_fetch_cookie, ARG1(0,STR), NULL, SMP_T_STR, SMP_USE_HRSHV }, { "scook_cnt", smp_fetch_cookie_cnt, ARG1(0,STR), NULL, SMP_T_SINT, SMP_USE_HRSHV }, { "scook_val", smp_fetch_cookie_val, ARG1(0,STR), NULL, SMP_T_SINT, SMP_USE_HRSHV }, - { "scook_names", smp_fetch_cookie_names, ARG1(0,STR), NULL, SMP_T_STR, SMP_USE_HRSHV }, /* shdr is valid only on the response and is used for ACL compatibility */ { "shdr", smp_fetch_hdr, ARG2(0,STR,SINT), val_hdr, SMP_T_STR, SMP_USE_HRSHV }, -- 2.27.0