libgfapi: return correct errno on invalid volume name

glfs_init when called with volume name prefixed by '/'
sets errno to 0. Setting errno to EINVAL to resolve the issue.
Also volname is a parameter to glfs_new.
Thus, validating volname in glfs_new itself and
returning EINVAL from that function

fixes: bz#1507896

Change-Id: I0d4d2423e26cc07644d50ec8cce788ecc639203d
Signed-off-by: Sheetal Pamecha <spamecha@redhat.com>

Author: Sheetalpamecha

api/src/glfs.c

@@ -817,17 +817,30 @@ struct glfs *
 pub_glfs_new(const char *volname)
 {
     struct glfs *fs = NULL;
+    int i = 0;
     int ret = -1;
     glusterfs_ctx_t *ctx = NULL;
     xlator_t *old_THIS = NULL;
     char pname[16] = "";
     char msg[32] = "";
 
-    if (!volname) {
+    if (!volname || volname[0] == '/' || volname[0] == '-') {
+        if (strncmp(volname, "/snaps/", 7) == 0) {
+            goto label;
+        }
         errno = EINVAL;
         return NULL;
     }
 
+    for (i = 0; i < strlen(volname); i++) {
+        if (!isalnum(volname[i]) && (volname[i] != '_') &&
+            (volname[i] != '-')) {
+            errno = EINVAL;
+            return NULL;
+        }
+    }
+
+label:
     /*
      * Do this as soon as possible in case something else depends on
      * pool allocations.

libglusterfs/src/graph.c

@@ -568,14 +568,13 @@ glusterfs_graph_prepare(glusterfs_graph_t *graph, glusterfs_ctx_t *ctx,
     } else {
         ret = glusterfs_graph_settop(graph, volume_name, _gf_false);
     }
-    if (!ret) {
-        goto ok;
-    }
 
-    gf_msg("graph", GF_LOG_ERROR, 0, LG_MSG_GRAPH_ERROR,
-           "glusterfs graph settop failed");
-    return -1;
-ok:
+    if (ret) {
+        gf_msg("graph", GF_LOG_ERROR, EINVAL, LG_MSG_GRAPH_ERROR,
+               "glusterfs graph settop failed");
+        errno = EINVAL;
+        return -1;
+    }
 
     /* XXX: WORM VOLUME */
     ret = glusterfs_graph_worm(graph, ctx);

tests/basic/gfapi/bug-1507896.c

@@ -0,0 +1,49 @@
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <glusterfs/api/glfs.h>
+#include <glusterfs/api/glfs-handles.h>
+
+#define VALIDATE_AND_GOTO_LABEL_ON_ERROR(func, ret, label)                     \
+    do {                                                                       \
+        if (ret < 0) {                                                         \
+            fprintf(stderr, "%s : returned error %d (%s)\n", func, ret,        \
+                    strerror(errno));                                          \
+            goto label;                                                        \
+        }                                                                      \
+    } while (0)
+
+int
+main(int argc, char *argv[])
+{
+    int ret = -1;
+    glfs_t *fs = NULL;
+    char *volname = NULL;
+    char *logfile = NULL;
+    char *hostname = NULL;
+
+    hostname = argv[1];
+    volname = argv[2];
+    logfile = argv[3];
+
+    fs = glfs_new(volname);
+    if (!fs)
+        VALIDATE_AND_GOTO_LABEL_ON_ERROR("glfs_new(fs)", ret, out);
+
+    ret = glfs_set_volfile_server(fs, "tcp", hostname, 24007);
+    VALIDATE_AND_GOTO_LABEL_ON_ERROR("glfs_set_volfile_server(fs)", ret, out);
+
+    ret = glfs_set_logging(fs, logfile, 7);
+    VALIDATE_AND_GOTO_LABEL_ON_ERROR("glfs_set_logging(fs)", ret, out);
+
+    ret = glfs_init(fs);
+    VALIDATE_AND_GOTO_LABEL_ON_ERROR("glfs_init(fs)", ret, out);
+
+out:
+    if (fs) {
+        ret = glfs_fini(fs);
+        if (ret)
+            fprintf(stderr, "glfs_fini(fs) returned %d\n", ret);
+    }
+    return ret;
+}

tests/basic/gfapi/bug-1507896.t

@@ -0,0 +1,33 @@
+#!/bin/bash
+
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+
+cleanup;
+
+TEST glusterd
+
+TEST $CLI volume create $V0 $H0:$B0/brick1;
+EXPECT 'Created' volinfo_field $V0 'Status';
+
+TEST $CLI volume start $V0;
+EXPECT 'Started' volinfo_field $V0 'Status';
+
+logdir=`gluster --print-logdir`
+
+TEST build_tester $(dirname $0)/bug-1507896.c -lgfapi
+
+TEST ./$(dirname $0)/bug-1507896 $H0 $V0 $logdir/bug-1507896.log
+
+#volume name precedding with '/'
+TEST ! ./$(dirname $0)/bug-1507896 $H0 /$V0 $logdir/bug-1507896.log
+
+#volume name passed with any special characters
+TEST ! ./$(dirname $0)/bug-1507896 $H0 test@_$V0 $logdir/bug-1507896.log
+
+cleanup_tester $(dirname $0)/bug-1507896
+
+TEST $CLI volume stop $V0
+TEST $CLI volume delete $V0
+
+cleanup;

tests/basic/gfapi/glfsxmp-coverage.c

@@ -1843,20 +1843,25 @@ main(int argc, char *argv[])
 
     sleep(2);
 
-    fs2 = glfs_new(argv[1]);
-    if (!fs2) {
-        fprintf(stderr, "glfs_new: returned NULL\n");
-        return 1;
-    }
     if (argc == 2) {
+        /* Generally glfs_new() requires volume name as an argument */
+        fs2 = glfs_new("test_only_volume");
+        if (!fs2) {
+            fprintf(stderr, "glfs_new(fs2): returned NULL\n");
+            return 1;
+        }
         ret = glfs_set_volfile(fs2, argv[1]);
         if (ret)
-            fprintf(stderr, "glfs_set_volfile failed\n");
+            fprintf(stderr, "glfs_set_volfile failed(fs2)\n");
     } else {
-        // ret = glfs_set_volfile_server (fs2, "unix", "/tmp/gluster.sock", 0);
+        fs2 = glfs_new(argv[1]);
+        if (!fs2) {
+            fprintf(stderr, "glfs_new(fs2): returned NULL\n");
+            return 1;
+        }
         ret = glfs_set_volfile_server(fs2, "tcp", argv[2], 24007);
         if (ret)
-            fprintf(stderr, "glfs_set_volfile_server failed\n");
+            fprintf(stderr, "glfs_set_volfile_server failed(fs2)\n");
     }
 
     ret = glfs_set_statedump_path(fs2, "/tmp");