MINOR: stats admin: allow unordered parameters in POST requests

Previously, the stats admin page required POST parameters to be provided
exactly in the same order as the HTML form.
This patch allows to handle those parameters in any orders.

Also, note that haproxy won't alter server states anymore if backend or server
names are ambiguous (duplicated names in the configuration) to prevent
unexpected results (the same should probably be applied to the stats socket).

diff --git a/include/proto/dumpstats.h b/include/proto/dumpstats.h
index eb44a363299b168ddc5bb2ee212f563bd3a299fa..4c3837b424e2272b31d1a91c0e5f0dfbceb46245 100644 (file)
--- a/include/proto/dumpstats.h
+++ b/include/proto/dumpstats.h
@@ -58,8 +58,10 @@
 /* status codes (strictly 4 chars) used in the URL to display a message */
 #define STAT_STATUS_UNKN "UNKN"        /* an unknown error occured, shouldn't happen */
 #define STAT_STATUS_DONE "DONE"        /* the action is successful */
+#define STAT_STATUS_PART "PART"        /* the action is partially successful */
 #define STAT_STATUS_NONE "NONE"        /* nothing happened (no action chosen or servers state didn't change) */
-#define STAT_STATUS_EXCD "EXCD"        /* an error occured becayse the buffer couldn't store all data */
+#define STAT_STATUS_ERRP "ERRP"        /* an error occured due to invalid values in parameters */
+#define STAT_STATUS_EXCD "EXCD"        /* an error occured because the buffer couldn't store all data */
 #define STAT_STATUS_DENY "DENY"        /* action denied */
 
 extern struct si_applet http_stats_applet;

diff --git a/include/types/proto_http.h b/include/types/proto_http.h
index f1b3eef524410a32329a7296e061062120d103b2..a7b16aa257d298904cb6bec71f85c6259b1f3fbb 100644 (file)
--- a/include/types/proto_http.h
+++ b/include/types/proto_http.h
@@ -251,6 +251,13 @@ enum {
        HTTP_ERR_SIZE
 };
 
+/* Actions available for the stats admin forms */
+enum {
+       ST_ADM_ACTION_NONE = 0,
+       ST_ADM_ACTION_DISABLE,
+       ST_ADM_ACTION_ENABLE,
+};
+
 /* This is an HTTP message, as described in RFC2616. It can be either a request
  * message or a response message.
  *

diff --git a/src/dumpstats.c b/src/dumpstats.c
index 142429b8c34d634d55d2e07823cfeb195baa4a06..16d07ddb21f766cd72beb3b6aa8bda01fd16e832 100644 (file)
--- a/src/dumpstats.c
+++ b/src/dumpstats.c
@@ -2037,6 +2037,26 @@ static int stats_dump_http(struct stream_interface *si, struct uri_auth *uri)
                                                     "Nothing has changed."
                                                     "</div>\n", uri->uri_prefix);
                                }
+                               else if (strcmp(si->applet.ctx.stats.st_code, STAT_STATUS_PART) == 0) {
+                                       chunk_printf(&msg,
+                                                    "<p><div class=active2>"
+                                                    "<a class=lfsb href=\"%s\" title=\"Remove this message\">[X]</a> "
+                                                    "Action partially processed.<br>"
+                                                    "Some server names are probably unknown or ambiguous (duplicated names in the backend)."
+                                                    "</div>\n", uri->uri_prefix);
+                               }
+                               else if (strcmp(si->applet.ctx.stats.st_code, STAT_STATUS_ERRP) == 0) {
+                                       chunk_printf(&msg,
+                                                    "<p><div class=active0>"
+                                                    "<a class=lfsb href=\"%s\" title=\"Remove this message\">[X]</a> "
+                                                    "Action not processed because of invalid parameters."
+                                                    "<ul>"
+                                                    "<li>The action is maybe unknown.</li>"
+                                                    "<li>The backend name is probably unknown or ambiguous (duplicated names).</li>"
+                                                    "<li>Some server names are probably unknown or ambiguous (duplicated names in the backend).</li>"
+                                                    "</ul>"
+                                                    "</div>\n", uri->uri_prefix);
+                               }
                                else if (strcmp(si->applet.ctx.stats.st_code, STAT_STATUS_EXCD) == 0) {
                                        chunk_printf(&msg,
                                                     "<p><div class=active0>"

diff --git a/src/proto_http.c b/src/proto_http.c
index ed8e018420d554bff0da1e0e57ae575a78042455..bf91328fe08db902fa3e28d533061ec0664d7846 100644 (file)
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -2545,13 +2545,18 @@ int http_wait_for_request(struct session *s, struct buffer *req, int an_bit)
  */
 int http_process_req_stat_post(struct stream_interface *si, struct http_txn *txn, struct buffer *req)
 {
-       struct proxy *px;
-       struct server *sv;
+       struct proxy *px = NULL;
+       struct server *sv = NULL;
 
-       char *backend = NULL;
-       int action = 0;
+       char key[LINESIZE];
+       int action = ST_ADM_ACTION_NONE;
+       int reprocess = 0;
+
+       int total_servers = 0;
+       int altered_servers = 0;
 
        char *first_param, *cur_param, *next_param, *end_params;
+       char *st_cur_param, *st_next_param;
 
        first_param = req->data + txn->req.eoh + 2;
        end_params  = first_param + txn->req.body_len;
@@ -2578,15 +2583,21 @@ int http_process_req_stat_post(struct stream_interface *si, struct http_txn *txn
         * From the html form, the backend and the action are at the end.
         */
        while (cur_param > first_param) {
-               char *key, *value;
+               char *value;
+               int poffset, plen;
 
                cur_param--;
                if ((*cur_param == '&') || (cur_param == first_param)) {
+ reprocess_servers:
                        /* Parse the key */
-                       key = cur_param;
-                       if (cur_param != first_param) {
-                               /* delimit the string for the next loop */
-                               *key++ = '\0';
+                       poffset = (cur_param != first_param ? 1 : 0);
+                       plen = next_param - cur_param + (cur_param == first_param ? 1 : 0);
+                       if ((plen > 0) && (plen <= sizeof(key))) {
+                               strncpy(key, cur_param + poffset, plen);
+                               key[plen - 1] = '\0';
+                       } else {
+                               si->applet.ctx.stats.st_code = STAT_STATUS_EXCD;
+                               goto out;
                        }
 
                        /* Parse the value */
@@ -2603,45 +2614,91 @@ int http_process_req_stat_post(struct stream_interface *si, struct http_txn *txn
                                break;
 
                        /* Now we can check the key to see what to do */
-                       if (!backend && strcmp(key, "b") == 0) {
-                               backend = value;
+                       if (!px && (strcmp(key, "b") == 0)) {
+                               if ((px = findproxy(value, PR_CAP_BE)) == NULL) {
+                                       /* the backend name is unknown or ambiguous (duplicate names) */
+                                       si->applet.ctx.stats.st_code = STAT_STATUS_ERRP;
+                                       goto out;
+                               }
                        }
-                       else if (!action && strcmp(key, "action") == 0) {
+                       else if (!action && (strcmp(key, "action") == 0)) {
                                if (strcmp(value, "disable") == 0) {
-                                       action = 1;
+                                       action = ST_ADM_ACTION_DISABLE;
                                }
                                else if (strcmp(value, "enable") == 0) {
-                                       action = 2;
-                               } else {
-                                       /* unknown action, no need to continue */
-                                       break;
+                                       action = ST_ADM_ACTION_ENABLE;
+                               }
+                               else {
+                                       si->applet.ctx.stats.st_code = STAT_STATUS_ERRP;
+                                       goto out;
                                }
                        }
                        else if (strcmp(key, "s") == 0) {
-                               if (backend && action && get_backend_server(backend, value, &px, &sv)) {
+                               if (!(px && action)) {
+                                       /*
+                                        * Indicates that we'll need to reprocess the parameters
+                                        * as soon as backend and action are known
+                                        */
+                                       if (!reprocess) {
+                                               st_cur_param  = cur_param;
+                                               st_next_param = next_param;
+                                       }
+                                       reprocess = 1;
+                               }
+                               else if ((sv = findserver(px, value)) != NULL) {
                                        switch (action) {
-                                       case 1:
+                                       case ST_ADM_ACTION_DISABLE:
                                                if ((px->state != PR_STSTOPPED) && !(sv->state & SRV_MAINTAIN)) {
                                                        /* Not already in maintenance, we can change the server state */
                                                        sv->state |= SRV_MAINTAIN;
                                                        set_server_down(sv);
-                                                       si->applet.ctx.stats.st_code = STAT_STATUS_DONE;
+                                                       altered_servers++;
+                                                       total_servers++;
                                                }
                                                break;
-                                       case 2:
+                                       case ST_ADM_ACTION_ENABLE:
                                                if ((px->state != PR_STSTOPPED) && (sv->state & SRV_MAINTAIN)) {
                                                        /* Already in maintenance, we can change the server state */
                                                        set_server_up(sv);
                                                        sv->health = sv->rise;  /* up, but will fall down at first failure */
-                                                       si->applet.ctx.stats.st_code = STAT_STATUS_DONE;
+                                                       altered_servers++;
+                                                       total_servers++;
                                                }
                                                break;
                                        }
+                               } else {
+                                       /* the server name is unknown or ambiguous (duplicate names) */
+                                       total_servers++;
                                }
                        }
+                       if (reprocess && px && action) {
+                               /* Now, we know the backend and the action chosen by the user.
+                                * We can safely restart from the first server parameter
+                                * to reprocess them
+                                */
+                               cur_param  = st_cur_param;
+                               next_param = st_next_param;
+                               reprocess = 0;
+                               goto reprocess_servers;
+                       }
+
                        next_param = cur_param;
                }
        }
+
+       if (total_servers == 0) {
+               si->applet.ctx.stats.st_code = STAT_STATUS_NONE;
+       }
+       else if (altered_servers == 0) {
+               si->applet.ctx.stats.st_code = STAT_STATUS_ERRP;
+       }
+       else if (altered_servers == total_servers) {
+               si->applet.ctx.stats.st_code = STAT_STATUS_DONE;
+       }
+       else {
+               si->applet.ctx.stats.st_code = STAT_STATUS_PART;
+       }
+ out:
        return 1;
 }
 
@@ -7124,6 +7181,10 @@ int stats_check_uri(struct stream_interface *si, struct http_txn *txn, struct pr
                                si->applet.ctx.stats.st_code = STAT_STATUS_DONE;
                        else if (memcmp(h, STAT_STATUS_NONE, 4) == 0)
                                si->applet.ctx.stats.st_code = STAT_STATUS_NONE;
+                       else if (memcmp(h, STAT_STATUS_PART, 4) == 0)
+                               si->applet.ctx.stats.st_code = STAT_STATUS_PART;
+                       else if (memcmp(h, STAT_STATUS_ERRP, 4) == 0)
+                               si->applet.ctx.stats.st_code = STAT_STATUS_ERRP;
                        else if (memcmp(h, STAT_STATUS_EXCD, 4) == 0)
                                si->applet.ctx.stats.st_code = STAT_STATUS_EXCD;
                        else if (memcmp(h, STAT_STATUS_DENY, 4) == 0)