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 376d4b150b9b6b1bd380472ad6a22a1f788de69d..b324ea9046ca6b0efb6c7438dde7f4bd64dcfa6d 100644 (file)
--- 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 823907c1eeed73c30d04c27655fd3b46274e6708..f199e3b6b36d7f3c76270a5aa2c73210887b69b2 100644 (file)
--- 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 <http.action>, 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 5b49e3438d4ca5077146707cfd3b5ce45e0cef69..f80da9a98353fd8a6ec8a23211321f9af2aaac04 100644 (file)
--- 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 <status> contains the new status code. This function never fails.
+ * variable <status> 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 <rules> for stream <s>, proxy <px> 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)