Bug 1475641

+++ This bug was initially created as a clone of Bug #1396327 +++

Description of problem:
coredump when incomplete gluster CLI command.
all with brick-name command has this issue, such like add-brick, remove-brick

Version-Release number of selected component (if applicable):
3.6.9 and mainline

How reproducible:


Steps to Reproduce:
1. gluster volume add-brick export  replica 2 force
2.
3.

Actual results:
coredump


Expected results:
prompt wrong parameter used.

Additional info:

--- Additional comment from George on 2016-11-17 21:57:21 EST ---

suggest fix on function cli_cmd_bricks_parse in cli-cmd-parser.c

        GF_ASSERT (brick_index > 0);
+        if(strcmp(words[brick_index],"force") == 0)
+        {
+               cli_err ("Wrong brick name used.");
+               ret = -1;
+               goto out;
+        }
        GF_ASSERT (brick_index < wordcount);

--- Additional comment from George on 2016-11-17 23:24:55 EST ---

Sorry, the last fix is not work.
Now we have fix it on 3.6.9 version, main branch should also have the same issue.
the changes patch on 3.6.9 FYI:

--- a/cli/src/cli-cmd-parser.c
+++ b/cli/src/cli-cmd-parser.c
@@ -544,6 +544,12 @@
 
         brick_index = index;
 
+        if (strcmp (words[brick_index], "force") == 0) {
+               cli_err ("Wrong brick name.");
+               ret = -1;
+               goto out;
+        }
+
         if (strcmp (words[wordcount - 1], "force") == 0) {
                 is_force = _gf_true;
                 wc = wordcount - 1;
@@ -1301,6 +1307,12 @@
         brick_index = index;
 
 parse_bricks:
+
+        if (strcmp (words[brick_index], "force") == 0) {
+               cli_err ("Wrong brick name.");
+               ret = -1;
+               goto out;
+        }
 
         if (strcmp (words[wordcount - 1], "force") == 0) {
                 is_force = _gf_true;

--- Additional comment from Worker Ant on 2016-11-18 00:30:56 EST ---

REVIEW: http://review.gluster.org/15874 (cli: error out incomplete CLI commands) posted (#1) for review on master by Atin Mukherjee (amukherj)

--- Additional comment from Atin Mukherjee on 2016-11-18 00:35:51 EST ---

(In reply to George from comment #1)
> suggest fix on function cli_cmd_bricks_parse in cli-cmd-parser.c
> 
>         GF_ASSERT (brick_index > 0);
> +        if(strcmp(words[brick_index],"force") == 0)
> +        {
> +               cli_err ("Wrong brick name used.");
> +               ret = -1;
> +               goto out;
> +        }
>         GF_ASSERT (brick_index < wordcount);

Why wouldn't it work? I just sent a patch and tested the same?

--- Additional comment from George on 2016-11-18 00:40:21 EST ---

Sorry, it could work.
but because bric_index == wordcount  in this case,
this strcmp statement somehow is confused and maybe has risk.

I would like suggest use the second fix. though this fix also work for this defect.

--- Additional comment from Worker Ant on 2017-07-17 01:18:09 EDT ---

REVIEW: https://review.gluster.org/15874 (cli: error out incomplete CLI commands) posted (#2) for review on master by Atin Mukherjee (amukherj)

--- Additional comment from Worker Ant on 2017-07-17 05:31:23 EDT ---

REVIEW: https://review.gluster.org/15874 (cli: error out incomplete CLI commands) posted (#3) for review on master by Atin Mukherjee (amukherj)

--- Additional comment from Worker Ant on 2017-07-26 03:17:38 EDT ---

REVIEW: https://review.gluster.org/17870 (cli: error out incomplete CLI commands) posted (#1) for review on master by Atin Mukherjee (amukherj)

--- Additional comment from Worker Ant on 2017-07-26 16:37:32 EDT ---

COMMIT: https://review.gluster.org/17870 committed in master by Jeff Darcy (jeff.us) 
------
commit c136024613c697fec87aaff3a070862b92c57977
Author: Atin Mukherjee <amukherj>
Date:   Wed Jul 26 12:46:42 2017 +0530

    cli: error out incomplete CLI commands
    
    cli_cmd_bricks_parse () & cli_cmd_volume_remove_brick_parse () were not
    handling the the error cases where the command is incomplete with
    missing brick details which could lead to glusterd crashes.
    
    Credit : george.lian
    
    Change-Id: Ia6303457a2aa279465aa75d4e1cfcc948893d5de
    BUG: 1396327
    Signed-off-by: Atin Mukherjee <amukherj>
    Reviewed-on: https://review.gluster.org/17870
    Smoke: Gluster Build System <jenkins.org>
    CentOS-regression: Gluster Build System <jenkins.org>
    Reviewed-by: Jeff Darcy <jeff.us>