From: Christopher Faulet Date: Mon, 16 Dec 2019 16:18:42 +0000 (+0100) Subject: MINOR: http-rules: Handle all message rewrites the same way X-Git-Tag: v2.2-dev1~52 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e00d06c99fa734beeb84642aa52d9bc75f33719b;p=thirdparty%2Fhaproxy.git MINOR: http-rules: Handle all message rewrites the same way In HTTP rules, error handling during a rewrite is now handle the same way for all rules. First, allocation errors are reported as internal errors. Then, if soft rewrites are allowed, rewrite errors are ignored and only the failed_rewrites counter is incremented. Otherwise, when strict rewrites are mandatory, interanl errors are returned. For now, only soft rewrites are supported. Note also that the warning sent to notify a rewrite failure was removed. It will be useless once the strict rewrites will be possible. --- diff --git a/include/proto/http_ana.h b/include/proto/http_ana.h index 376d4b150b..b324ea9046 100644 --- a/include/proto/http_ana.h +++ b/include/proto/http_ana.h @@ -44,7 +44,7 @@ int http_transform_header_str(struct stream* s, struct channel *chn, struct htx struct ist name, const char *str, struct my_regex *re, int action); int http_req_replace_stline(int action, const char *replace, int len, struct proxy *px, struct stream *s); -void http_res_set_status(unsigned int status, const char *reason, struct stream *s); +int http_res_set_status(unsigned int status, const char *reason, struct stream *s); void http_check_request_for_cacheability(struct stream *s, struct channel *req); void http_check_response_for_cacheability(struct stream *s, struct channel *res); void http_perform_server_redirect(struct stream *s, struct stream_interface *si); diff --git a/src/http_act.c b/src/http_act.c index 823907c1ee..f199e3b6b3 100644 --- a/src/http_act.c +++ b/src/http_act.c @@ -44,8 +44,9 @@ * builds a string in the trash from the specified format string. It finds * the action to be performed in , previously filled by function * parse_set_req_line(). The replacement action is excuted by the function - * http_action_set_req_line(). It always returns ACT_RET_CONT. If an error - * occurs the action is canceled, but the rule processing continue. + * http_action_set_req_line(). On success, it returns ACT_RET_CONT. If an error + * occurs while soft rewrites are enabled, the action is canceled, but the rule + * processing continue. Otherwsize ACT_RET_ERR is returned. */ static enum act_return http_action_set_req_line(struct act_rule *rule, struct proxy *px, struct session *sess, struct stream *s, int flags) @@ -55,7 +56,7 @@ static enum act_return http_action_set_req_line(struct act_rule *rule, struct pr replace = alloc_trash_chunk(); if (!replace) - goto leave; + goto fail_alloc; /* If we have to create a query string, prepare a '?'. */ if (rule->arg.http.action == 2) @@ -64,12 +65,32 @@ static enum act_return http_action_set_req_line(struct act_rule *rule, struct pr replace->size - replace->data, &rule->arg.http.logfmt); - http_req_replace_stline(rule->arg.http.action, replace->area, - replace->data, px, s); + if (http_req_replace_stline(rule->arg.http.action, replace->area, + replace->data, px, s) == -1) + goto fail_rewrite; -leave: + leave: free_trash_chunk(replace); return ret; + + fail_alloc: + if (!(s->flags & SF_ERR_MASK)) + s->flags |= SF_ERR_RESOURCE; + ret = ACT_RET_ERR; + goto leave; + + fail_rewrite: + _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_rewrites, 1); + if (s->flags & SF_BE_ASSIGNED) + _HA_ATOMIC_ADD(&s->be->be_counters.failed_rewrites, 1); + if (sess->listener->counters) + _HA_ATOMIC_ADD(&sess->listener->counters->failed_rewrites, 1); + if (objt_server(s->target)) + _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.failed_rewrites, 1); + + if (!(s->txn->req.flags & HTTP_MSGF_SOFT_RW)) + ret = ACT_RET_ERR; + goto leave; } /* parse an http-request action among : @@ -135,8 +156,9 @@ static enum act_parse_ret parse_set_req_line(const char **args, int *orig_arg, s * in p[1] to replace the URI. It uses the format string present in act.p[2..3]. * The component to act on (path/uri) is taken from act.p[0] which contains 1 * for the path or 3 for the URI (values used by http_req_replace_stline()). - * It always returns ACT_RET_CONT. If an error occurs, the action is canceled, - * but the rule processing continues. + * On success, it returns ACT_RET_CONT. If an error occurs while soft rewrites + * are enabled, the action is canceled, but the rule processing continue. + * Otherwsize ACT_RET_ERR is returned. */ static enum act_return http_action_replace_uri(struct act_rule *rule, struct proxy *px, struct session *sess, struct stream *s, int flags) @@ -149,7 +171,7 @@ static enum act_return http_action_replace_uri(struct act_rule *rule, struct pro replace = alloc_trash_chunk(); output = alloc_trash_chunk(); if (!replace || !output) - goto leave; + goto fail_alloc; uri = htx_sl_req_uri(http_get_stline(htxbuf(&s->req.buf))); if (rule->arg.act.p[0] == (void *)1) @@ -165,14 +187,34 @@ static enum act_return http_action_replace_uri(struct act_rule *rule, struct pro */ len = exp_replace(output->area, output->size, uri.ptr, replace->area, pmatch); if (len == -1) - goto leave; + goto fail_rewrite; - http_req_replace_stline((long)rule->arg.act.p[0], output->area, len, px, s); + if (http_req_replace_stline((long)rule->arg.act.p[0], output->area, len, px, s) == -1) + goto fail_rewrite; -leave: + leave: free_trash_chunk(output); free_trash_chunk(replace); return ret; + + fail_alloc: + if (!(s->flags & SF_ERR_MASK)) + s->flags |= SF_ERR_RESOURCE; + ret = ACT_RET_ERR; + goto leave; + + fail_rewrite: + _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_rewrites, 1); + if (s->flags & SF_BE_ASSIGNED) + _HA_ATOMIC_ADD(&s->be->be_counters.failed_rewrites, 1); + if (sess->listener->counters) + _HA_ATOMIC_ADD(&sess->listener->counters->failed_rewrites, 1); + if (objt_server(s->target)) + _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.failed_rewrites, 1); + + if (!(s->txn->req.flags & HTTP_MSGF_SOFT_RW)) + ret = ACT_RET_ERR; + goto leave; } /* parse a "replace-uri" or "replace-path" http-request action. @@ -222,7 +264,19 @@ static enum act_parse_ret parse_replace_uri(const char **args, int *orig_arg, st static enum act_return action_http_set_status(struct act_rule *rule, struct proxy *px, struct session *sess, struct stream *s, int flags) { - http_res_set_status(rule->arg.status.code, rule->arg.status.reason, s); + if (http_res_set_status(rule->arg.status.code, rule->arg.status.reason, s) == -1) { + _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_rewrites, 1); + if (s->flags & SF_BE_ASSIGNED) + _HA_ATOMIC_ADD(&s->be->be_counters.failed_rewrites, 1); + if (sess->listener->counters) + _HA_ATOMIC_ADD(&sess->listener->counters->failed_rewrites, 1); + if (objt_server(s->target)) + _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.failed_rewrites, 1); + + if (!(s->txn->req.flags & HTTP_MSGF_SOFT_RW)) + return ACT_RET_ERR; + } + return ACT_RET_CONT; } diff --git a/src/http_ana.c b/src/http_ana.c index 5b49e3438d..f80da9a983 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -2729,24 +2729,41 @@ static int http_transform_header(struct stream* s, struct channel *chn, struct h const struct ist name, struct list *fmt, struct my_regex *re, int action) { struct buffer *replace; - int ret = -1; + int ret = 0; replace = alloc_trash_chunk(); - if (!replace) { - if (!(s->flags & SF_ERR_MASK)) - s->flags |= SF_ERR_RESOURCE; - goto leave; - } + if (!replace) + goto fail_alloc; replace->data = build_logline(s, replace->area, replace->size, fmt); if (replace->data >= replace->size - 1) - goto leave; + goto fail_rewrite; - ret = http_transform_header_str(s, chn, htx, name, replace->area, re, action); + if (http_transform_header_str(s, chn, htx, name, replace->area, re, action) == -1) + goto fail_rewrite; leave: free_trash_chunk(replace); return ret; + + fail_alloc: + if (!(s->flags & SF_ERR_MASK)) + s->flags |= SF_ERR_RESOURCE; + ret = -1; + goto leave; + + fail_rewrite: + _HA_ATOMIC_ADD(&s->sess->fe->fe_counters.failed_rewrites, 1); + if (s->flags & SF_BE_ASSIGNED) + _HA_ATOMIC_ADD(&s->be->be_counters.failed_rewrites, 1); + if (s->sess->listener->counters) + _HA_ATOMIC_ADD(&s->sess->listener->counters->failed_rewrites, 1); + if (objt_server(s->target)) + _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.failed_rewrites, 1); + + if (!(s->txn->req.flags & HTTP_MSGF_SOFT_RW)) + ret = -1; + goto leave; } @@ -2866,9 +2883,10 @@ int http_req_replace_stline(int action, const char *replace, int len, } /* This function replace the HTTP status code and the associated message. The - * variable contains the new status code. This function never fails. + * variable contains the new status code. This function never fails. It + * returns 0 in case of success, -1 in case of internal error. */ -void http_res_set_status(unsigned int status, const char *reason, struct stream *s) +int http_res_set_status(unsigned int status, const char *reason, struct stream *s) { struct htx *htx = htxbuf(&s->res.buf); char *res; @@ -2881,8 +2899,11 @@ void http_res_set_status(unsigned int status, const char *reason, struct stream if (reason == NULL) reason = http_get_reason(status); - if (http_replace_res_status(htx, ist2(trash.area, trash.data))) - http_replace_res_reason(htx, ist2(reason, strlen(reason))); + if (!http_replace_res_status(htx, ist2(trash.area, trash.data))) + return -1; + if (!http_replace_res_reason(htx, ist2(reason, strlen(reason)))) + return -1; + return 0; } /* Executes the http-request rules for stream , proxy and @@ -3055,12 +3076,6 @@ static enum rule_result http_req_get_intercept_rule(struct proxy *px, struct lis } if (!http_add_header(htx, n, v)) { - static unsigned char rate_limit = 0; - - if ((rate_limit++ & 255) == 0) { - send_log(px, LOG_WARNING, "Proxy %s failed to add or set the request header '%.*s' for request #%u. You might need to increase tune.maxrewrite.", px->id, (int)n.len, n.ptr, s->uniq_id); - } - _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_rewrites, 1); if (s->flags & SF_BE_ASSIGNED) _HA_ATOMIC_ADD(&s->be->be_counters.failed_rewrites, 1); @@ -3419,12 +3434,6 @@ resume_execution: } if (!http_add_header(htx, n, v)) { - static unsigned char rate_limit = 0; - - if ((rate_limit++ & 255) == 0) { - send_log(px, LOG_WARNING, "Proxy %s failed to add or set the response header '%.*s' for request #%u. You might need to increase tune.maxrewrite.", px->id, (int)n.len, n.ptr, s->uniq_id); - } - _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_rewrites, 1); _HA_ATOMIC_ADD(&s->be->be_counters.failed_rewrites, 1); if (sess->listener->counters)