config: improve variable substitution

- allow ${foo_bar}
- prevent $foogarbage from substitution variable 'foo'
- use std::string throughout
- improve tests

Signed-off-by: Sage Weil <sage@inktank.com>

diff --git a/src/common/config.cc b/src/common/config.cc
index fcbf61a53bf406bdb034f541c1e8c5a81571c31c..ff39059b0c0d11be31a61121f12a8ebf5dddfec6 100644 (file)
--- a/src/common/config.cc
+++ b/src/common/config.cc
@@ -882,73 +882,91 @@ bool md_config_t::expand_meta(std::string &origval) const
 
   set<string> resolved;
 
-  string::size_type sz;
   string val = origval;
 
  restart:
-  sz = val.size();
   string out;
-  out.reserve(sz);
+  out.reserve(val.size());
 
-  for (string::size_type s = 0; s < sz; ) {
+  for (string::size_type s = 0; s < val.size(); ) {
     if (val[s] != '$') {
       out += val[s++];
       continue;
     }
-    string::size_type rem = sz - (s + 1);
-
-    // special metavariable?
-    for (int i = 0; i < NUM_CONF_METAVARIABLES; ++i) {
-      size_t clen = strlen(CONF_METAVARIABLES[i]);
-      if (rem < clen)
-       continue;
-      if (strncmp(val.c_str() + s + 1, CONF_METAVARIABLES[i], clen))
-       continue;
-      if (strcmp(CONF_METAVARIABLES[i], "type")==0)
-       out += name.get_type_name();
-      else if (strcmp(CONF_METAVARIABLES[i], "cluster")==0)
-       out += cluster;
-      else if (strcmp(CONF_METAVARIABLES[i], "name")==0)
-       out += name.to_cstr();
-      else if (strcmp(CONF_METAVARIABLES[i], "host")==0)
-       out += host;
-      else if (strcmp(CONF_METAVARIABLES[i], "num")==0)
-       out += name.get_id().c_str();
-      else if (strcmp(CONF_METAVARIABLES[i], "id")==0)
-       out += name.get_id().c_str();
+
+    // try to parse the variable name into var, either \$\{(.+)\} or
+    // \$([a-z\_]+)
+    const char *valid_chars = "abcdefghijklmnopqrstuvwxyz_";
+    string var;
+    size_t endpos = 0;
+    if (val[s+1] == '{') {
+      // ...${foo_bar}...
+      endpos = val.find_first_not_of(valid_chars, s+2);
+      if (endpos != std::string::npos &&
+         val[endpos] == '}') {
+       var = val.substr(s+2, endpos-s-2);
+       endpos++;
+      }
+    } else {
+      // ...$foo...
+      endpos = val.find_first_not_of(valid_chars, s+1);
+      if (endpos != std::string::npos)
+       var = val.substr(s+1, endpos-s-1);
       else
-       assert(0); // unreachable
-      found_meta = true;
-      s += strlen(CONF_METAVARIABLES[i]) + 1;
-      out += val.substr(s);
-      val = out;
-      goto restart;
-    }
-
-    // config option?
-    for (int i = 0; i < NUM_CONFIG_OPTIONS; i++) {
-      config_option *opt = &config_optionsp[i];
-      size_t clen = strlen(opt->name);
-      if (rem < clen)
-       continue;
-      if (strncmp(val.c_str() + s + 1, opt->name, clen))
-       continue;
-
-      // avoid loops
-      if (resolved.count(opt->name))
-       continue;       // loop; skip
-      resolved.insert(opt->name);
-
-      found_meta = true;
-      char *vv = NULL;
-      _get_val(opt->name, &vv, -1);
-      out += vv;
-      s += strlen(opt->name) + 1;
-      out += val.substr(s);
-      //cout << "val '" << val << "' s " << s << " out '" << out << "' after sub " << opt->name << " -> " << vv << std::endl;
-      val = out;
-      free(vv);
-      goto restart;
+       var = val.substr(s+1);
+    }
+    //cout << "var='" << var << "'" << std::endl;
+
+    if (var.length()) {
+      // special metavariable?
+      for (int i = 0; i < NUM_CONF_METAVARIABLES; ++i) {
+       if (var != CONF_METAVARIABLES[i])
+         continue;
+       //cout << "  meta match of " << var << " " << CONF_METAVARIABLES[i] << std::endl;
+       if (var == "type")
+         out += name.get_type_name();
+       else if (var == "cluster")
+         out += cluster;
+       else if (var == "name")
+         out += name.to_cstr();
+       else if (var == "host")
+         out += host;
+       else if (var == "num")
+         out += name.get_id().c_str();
+       else if (var == "id")
+         out += name.get_id().c_str();
+       else
+         assert(0); // unreachable
+       found_meta = true;
+       if (endpos != std::string::npos)
+         out += val.substr(endpos);
+       //cout << "val '" << val << "' s " << s << " out '" << out << "'"  << std::endl;
+       val = out;
+       goto restart;
+      }
+
+      // config option?
+      for (int i = 0; i < NUM_CONFIG_OPTIONS; i++) {
+       config_option *opt = &config_optionsp[i];
+       if (var != opt->name)
+         continue;
+       
+       // avoid loops
+       if (resolved.count(opt->name))
+         continue;     // loop; skip
+       resolved.insert(opt->name);
+
+       found_meta = true;
+       char *vv = NULL;
+       _get_val(opt->name, &vv, -1);
+       out += vv;
+       if (endpos != std::string::npos)
+         out += val.substr(endpos);
+       //cout << "val '" << val << "' s " << s << " out '" << out << "' after sub " << opt->name << " -> " << vv << std::endl;
+       val = out;
+       free(vv);
+       goto restart;
+      }
     }
 
     // pass it thru

diff --git a/src/test/daemon_config.cc b/src/test/daemon_config.cc
index 9eaf6614bcbbd351b9413d63d57aea697f6102a1..d643a6f49677dd4d34db8ea238b3fb17f5cdaab5 100644 (file)
--- a/src/test/daemon_config.cc
+++ b/src/test/daemon_config.cc
@@ -43,7 +43,39 @@ TEST(DaemonConfig, Substitution) {
   ret = g_ceph_context->_conf->set_val("internal_safe_to_start_threads", "false");
   ret = g_ceph_context->_conf->set_val("host", "foo");
   ASSERT_EQ(ret, 0);
-  ret = g_ceph_context->_conf->set_val("public_network", "bar$hostbaz", false);
+  ret = g_ceph_context->_conf->set_val("public_network", "bar$host.baz", false);
+  ASSERT_EQ(ret, 0);
+  g_ceph_context->_conf->apply_changes(NULL);
+  char buf[128];
+  memset(buf, 0, sizeof(buf));
+  char *tmp = buf;
+  ret = g_ceph_context->_conf->get_val("public_network", &tmp, sizeof(buf));
+  ASSERT_EQ(ret, 0);
+  ASSERT_EQ(string("barfoo.baz"), string(buf));
+}
+
+TEST(DaemonConfig, SubstitutionTrailing) {
+  int ret;
+  ret = g_ceph_context->_conf->set_val("internal_safe_to_start_threads", "false");
+  ret = g_ceph_context->_conf->set_val("host", "foo");
+  ASSERT_EQ(ret, 0);
+  ret = g_ceph_context->_conf->set_val("public_network", "bar$host", false);
+  ASSERT_EQ(ret, 0);
+  g_ceph_context->_conf->apply_changes(NULL);
+  char buf[128];
+  memset(buf, 0, sizeof(buf));
+  char *tmp = buf;
+  ret = g_ceph_context->_conf->get_val("public_network", &tmp, sizeof(buf));
+  ASSERT_EQ(ret, 0);
+  ASSERT_EQ(string("barfoo"), string(buf));
+}
+
+TEST(DaemonConfig, SubstitutionBraces) {
+  int ret;
+  ret = g_ceph_context->_conf->set_val("internal_safe_to_start_threads", "false");
+  ret = g_ceph_context->_conf->set_val("host", "foo");
+  ASSERT_EQ(ret, 0);
+  ret = g_ceph_context->_conf->set_val("public_network", "bar${host}baz", false);
   ASSERT_EQ(ret, 0);
   g_ceph_context->_conf->apply_changes(NULL);
   char buf[128];
@@ -53,6 +85,21 @@ TEST(DaemonConfig, Substitution) {
   ASSERT_EQ(ret, 0);
   ASSERT_EQ(string("barfoobaz"), string(buf));
 }
+TEST(DaemonConfig, SubstitutionBracesTrailing) {
+  int ret;
+  ret = g_ceph_context->_conf->set_val("internal_safe_to_start_threads", "false");
+  ret = g_ceph_context->_conf->set_val("host", "foo");
+  ASSERT_EQ(ret, 0);
+  ret = g_ceph_context->_conf->set_val("public_network", "bar${host}", false);
+  ASSERT_EQ(ret, 0);
+  g_ceph_context->_conf->apply_changes(NULL);
+  char buf[128];
+  memset(buf, 0, sizeof(buf));
+  char *tmp = buf;
+  ret = g_ceph_context->_conf->get_val("public_network", &tmp, sizeof(buf));
+  ASSERT_EQ(ret, 0);
+  ASSERT_EQ(string("barfoo"), string(buf));
+}
 
 TEST(DaemonConfig, SubstitutionLoop) {
   int ret;