Merge branch 'wip-4779' into next

Reviewed-by: Sage Weil <sage@inktank.com># Please enter a commit message to explain why this merge is necessary,

5 files changed, 85 insertions, 36 deletions

diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index 8a9addfb6c2..e96e6123aab 100644
--- a/src/crush/CrushWrapper.cc
+++ b/src/crush/CrushWrapper.cc
@@ -333,6 +333,9 @@ int CrushWrapper::insert_item(CephContext *cct, int item, float weight, string n
   ldout(cct, 5) << "insert_item item " << item << " weight " << weight
 		<< " name " << name << " loc " << loc << dendl;
 
+  if (!is_valid_crush_name(name))
+    return -EINVAL;
+
   if (name_exists(name)) {
     if (get_item_id(name) != item) {
       ldout(cct, 10) << "device name '" << name << "' already exists as id "
@@ -473,6 +476,10 @@ int CrushWrapper::create_or_move_item(CephContext *cct, int item, float weight,
 {
   int ret = 0;
   int old_iweight;
+
+  if (!is_valid_crush_name(name))
+    return -EINVAL;
+
   if (check_item_loc(cct, item, loc, &old_iweight)) {
     ldout(cct, 5) << "create_or_move_item " << item << " already at " << loc << dendl;
   } else {
@@ -497,6 +504,9 @@ int CrushWrapper::update_item(CephContext *cct, int item, float weight, string n
 		<< " name " << name << " loc " << loc << dendl;
   int ret = 0;
 
+  if (!is_valid_crush_name(name))
+    return -EINVAL;
+
   // compare quantized (fixed-point integer) weights!  
   int iweight = (int)(weight * (float)0x10000);
   int old_iweight;
@@ -1109,3 +1119,20 @@ void CrushWrapper::generate_test_instances(list<CrushWrapper*>& o)
   o.push_back(new CrushWrapper);
   // fixme
 }
+
+
+bool CrushWrapper::is_valid_crush_name(const string& s)
+{
+  if (s.empty())
+    return false;
+  for (string::const_iterator p = s.begin(); p != s.end(); ++p) {
+    if (!(*p == '-') &&
+	!(*p == '_') &&
+	!(*p == '.') &&
+	!(*p >= '0' && *p <= '9') &&
+	!(*p >= 'A' && *p <= 'Z') &&
+	!(*p >= 'a' && *p <= 'z'))
+      return false;
+  }
+  return true;
+}
diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h
index f5a88c8bdd5..3d07a281956 100644
--- a/src/crush/CrushWrapper.h
+++ b/src/crush/CrushWrapper.h
@@ -200,10 +200,13 @@ public:
       return p->second.c_str();
     return 0;
   }
-  void set_item_name(int i, const string& name) {
+  int set_item_name(int i, const string& name) {
+    if (!is_valid_crush_name(name))
+      return -EINVAL;
     name_map[i] = name;
     if (have_rmaps)
       name_rmap[name] = i;
+    return 0;
   }
 
   // rule names
@@ -790,6 +793,9 @@ public:
   void dump_rules(Formatter *f) const;
   void list_rules(Formatter *f) const;
   static void generate_test_instances(list<CrushWrapper*>& o);
+
+
+  static bool is_valid_crush_name(const string& s);
 };
 WRITE_CLASS_ENCODER(CrushWrapper)
 
diff --git a/src/mon/MonCommands.h b/src/mon/MonCommands.h
index 9236462edd9..90b9c7ee74a 100644
--- a/src/mon/MonCommands.h
+++ b/src/mon/MonCommands.h
@@ -357,55 +357,55 @@ COMMAND("osd setcrushmap", "set crush map from input file", \
 COMMAND("osd crush set", "set crush map from input file", \
 	"osd", "rw", "cli,rest")
 COMMAND("osd crush add-bucket " \
-	"name=name,type=CephString " \
+	"name=name,type=CephString,goodchars=[A-Za-z0-9-_.] " \
 	"name=type,type=CephString", \
 	"add no-parent (probably root) crush bucket <name> of type <type>", \
 	"osd", "rw", "cli,rest")
 COMMAND("osd crush set " \
 	"name=id,type=CephOsdName " \
 	"name=weight,type=CephFloat,range=0.0 " \
-	"name=args,type=CephString,n=N", \
+	"name=args,type=CephString,n=N,goodchars=[A-Za-z0-9-_.=]", \
 	"set crushmap entry for <name> to <weight> with location <args>", \
 	"osd", "rw", "cli,rest")
 COMMAND("osd crush add " \
 	"name=id,type=CephOsdName " \
 	"name=weight,type=CephFloat,range=0.0 " \
-	"name=args,type=CephString,n=N", \
+	"name=args,type=CephString,n=N,goodchars=[A-Za-z0-9-_.=]", \
 	"add crushmap entry for <name> with <weight> and location <args>", \
 	"osd", "rw", "cli,rest")
 COMMAND("osd crush create-or-move " \
 	"name=id,type=CephOsdName " \
 	"name=weight,type=CephFloat,range=0.0 " \
-	"name=args,type=CephString,n=N", \
+	"name=args,type=CephString,n=N,goodchars=[A-Za-z0-9-_.=]", \
 	"create entry or move existing entry for <name> <weight> at/to location <args>", \
 	"osd", "rw", "cli,rest")
 COMMAND("osd crush move " \
 	"name=id,type=CephOsdName " \
-	"name=args,type=CephString,n=N", \
+	"name=args,type=CephString,n=N,goodchars=[A-Za-z0-9-_.=]", \
 	"move existing entry for <name> to location <args>", \
 	"osd", "rw", "cli,rest")
 COMMAND("osd crush link " \
 	"name=name,type=CephString " \
-	"name=args,type=CephString,n=N", \
+	"name=args,type=CephString,n=N,goodchars=[A-Za-z0-9-_.=]", \
 	"link existing entry for <name> under location <args>", \
 	"osd", "rw", "cli,rest")
 COMMAND("osd crush rm " \
-	"name=name,type=CephString " \
-	"name=ancestor,type=CephString,req=false", \
+	"name=name,type=CephString,goodchars=[A-Za-z0-9-_.] " \
+	"name=ancestor,type=CephString,req=false,goodchars=[A-Za-z0-9-_.]", \
 	"remove <name> from crush map (everywhere, or just at <ancestor>",\
 	"osd", "rw", "cli,rest")
 COMMAND("osd crush remove " \
-	"name=name,type=CephString " \
-	"name=ancestor,type=CephString,req=false", \
+	"name=name,type=CephString,goodchars=[A-Za-z0-9-_.] " \
+	"name=ancestor,type=CephString,req=false,goodchars=[A-Za-z0-9-_.]", \
 	"remove <name> from crush map (everywhere, or just at <ancestor>", \
 	"osd", "rw", "cli,rest")
 COMMAND("osd crush unlink " \
-	"name=name,type=CephString " \
-	"name=ancestor,type=CephString,req=false", \
+	"name=name,type=CephString,goodchars=[A-Za-z0-9-_.] " \
+	"name=ancestor,type=CephString,req=false,goodchars=[A-Za-z0-9-_.]", \
 	"unlink <name> from crush map (everywhere, or just at <ancestor>", \
 	"osd", "rw", "cli,rest")
 COMMAND("osd crush reweight " \
-	"name=name,type=CephString " \
+	"name=name,type=CephString,goodchars=[A-Za-z0-9-_.] " \
 	"name=weight,type=CephFloat,range=0.0", \
 	"change <name>'s weight to <weight> in crush map", \
 	"osd", "rw", "cli,rest")
@@ -413,12 +413,13 @@ COMMAND("osd crush tunables " \
 	"name=profile,type=CephChoices,strings=legacy|argonaut|bobtail|optimal|default", \
 	"set crush tunables values to <profile>", "osd", "rw", "cli,rest")
 COMMAND("osd crush rule create-simple " \
-	"name=name,type=CephString " \
-	"name=root,type=CephString " \
-	"name=type,type=CephString", \
+	"name=name,type=CephString,goodchars=[A-Za-z0-9-_.] " \
+	"name=root,type=CephString,goodchars=[A-Za-z0-9-_.] " \
+	"name=type,type=CephString,goodchars=[A-Za-z0-9-_.]",
 	"create crush rule <name> in <root> of type <type>", \
 	"osd", "rw", "cli,rest")
 COMMAND("osd crush rule rm " \
+	"name=name,type=CephString,goodchars=[A-Za-z0-9-_.] " \
 	"name=name,type=CephString", \
 	"remove crush rule <name>", "osd", "rw", "cli,rest")
 COMMAND("osd setmaxosd " \
diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index eb674b7bc4c..0ba211c0fae 100644
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -2670,7 +2670,11 @@ bool OSDMonitor::prepare_command(MMonCommand *m)
     int bucketno = newcrush.add_bucket(0, CRUSH_BUCKET_STRAW,
 				       CRUSH_HASH_DEFAULT, type, 0, NULL,
 				       NULL);
-    newcrush.set_item_name(bucketno, name);
+    err = newcrush.set_item_name(bucketno, name);
+    if (err < 0) {
+      ss << "error setting bucket name to '" << name << "'";
+      goto reply;
+    }
 
     pending_inc.crush.clear();
     newcrush.encode(pending_inc.crush);
diff --git a/src/pybind/ceph_argparse.py b/src/pybind/ceph_argparse.py
index 15b04a8e06c..72b36dd50a5 100644
--- a/src/pybind/ceph_argparse.py
+++ b/src/pybind/ceph_argparse.py
@@ -15,6 +15,7 @@ Foundation.  See file COPYING.
 import copy
 import json
 import os
+import re
 import socket
 import stat
 import sys
@@ -175,21 +176,31 @@ class CephFloat(CephArgtype):
 
 class CephString(CephArgtype):
     """
-    String; pretty generic.
+    String; pretty generic.  goodchars is a RE char class of valid chars
     """
-    def __init__(self, badchars=''):
-        self.badchars = badchars
+    def __init__(self, goodchars=''):
+        from string import printable
+        try:
+            re.compile(goodchars)
+        except:
+            raise ValueError('CephString(): "{0}" is not a valid RE'.\
+                format(goodchars))
+        self.goodchars = goodchars
+        self.goodset = frozenset(
+            [c for c in printable if re.match(goodchars, c)]
+        )
 
     def valid(self, s, partial=False):
-        for c in self.badchars:
-            if c in s:
-                raise ArgumentFormat("bad char {0} in {1}".format(c, s))
+        sset = set(s)
+        if self.goodset and not sset <= self.goodset:
+            raise ArgumentFormat("invalid chars {0} in {1}".\
+                format(''.join(sset - self.goodset), s))
         self.val = s
 
     def __str__(self):
         b = ''
-        if len(self.badchars):
-            b = '(without chars in {0})'.format(self.badchars)
+        if self.goodchars:
+            b += '(goodchars {0})'.format(self.goodchars)
         return '<string{0}>'.format(b)
 
 class CephSocketpath(CephArgtype):
@@ -790,7 +801,7 @@ def validate(args, signature, partial=False):
                     # hm, but it was required, so quit
                     if partial:
                         return d
-                    raise ArgumentFormat('{0} not valid argument {1}: {2}'.format(str(myarg), desc, e))
+                    raise e
 
             # valid arg acquired.  Store in dict, as a list if multivalued
             if desc.N:
@@ -848,19 +859,19 @@ def validate_command(parsed_args, sigdict, args, verbose=False):
                 sig = cmd['sig']
                 helptext = cmd['help']
                 try:
-                    valid_dict = validate(args, sig, verbose)
+                    valid_dict = validate(args, sig)
                     found = cmd
                     break
+                except ArgumentPrefix:
+                    # ignore prefix mismatches; we just haven't found
+                    # the right command yet
+                    pass
                 except ArgumentError as e:
                     # prefixes matched, but some other arg didn't;
-                    # this is interesting information if verbose
-                    if verbose:
-                        print >> sys.stderr, '{0}: invalid command'.\
-                            format(' '.join(args))
-                        print >> sys.stderr, '{0}'.format(e)
-                        print >> sys.stderr, "did you mean {0}?\n\t{1}".\
-                            format(concise_sig(sig), helptext)
-                    pass
+                    # stop now, because we have the right command but
+                    # some other input is invalid
+                    print >> sys.stderr, "Invalid command: ", str(e)
+                    return {}
                 if found:
                     break