Don't interpret strings as format strings

Ludo reported this error:

  unexpected Nix daemon error: boost::too_few_args: format-string refered to more arguments than were passed

coming from this line:

  printMsg(lvlError, run.program + ": " + string(err, 0, p));

The problem here is that the string ends up implicitly converted to a
Boost format() object, so % characters are treated specially.  I
always assumed (wrongly) that strings are converted to a format object
that outputs the string as-is.

Since this assumption appears in several places that may be hard to
grep for, I've added some C++ type hackery to ensures that the right
thing happens.  So you don't have to worry about % in statements like

  printMsg(lvlError, "foo: " + s);

or

  throw Error("foo: " + s);

3 files changed, 32 insertions, 23 deletions

diff --git a/src/libutil/types.hh b/src/libutil/types.hh
index a2ab1cc0ee83..4b5ce9a78c48 100644
--- a/src/libutil/types.hh
+++ b/src/libutil/types.hh
@@ -20,28 +20,37 @@ using std::vector;
 using boost::format;
 
 
+struct FormatOrString
+{
+    string s;
+    FormatOrString(const string & s) : s(s) { };
+    FormatOrString(const format & f) : s(f.str()) { };
+    FormatOrString(const char * s) : s(s) { };
+};
+
+
 /* BaseError should generally not be caught, as it has Interrupted as
    a subclass. Catch Error instead. */
-class BaseError : public std::exception 
+class BaseError : public std::exception
 {
 protected:
     string prefix_; // used for location traces etc.
     string err;
 public:
     unsigned int status; // exit status
-    BaseError(const format & f, unsigned int status = 1);
+    BaseError(const FormatOrString & fs, unsigned int status = 1);
     ~BaseError() throw () { };
     const char * what() const throw () { return err.c_str(); }
     const string & msg() const throw () { return err; }
     const string & prefix() const throw () { return prefix_; }
-    BaseError & addPrefix(const format & f);
+    BaseError & addPrefix(const FormatOrString & fs);
 };
 
 #define MakeError(newClass, superClass) \
     class newClass : public superClass                  \
     {                                                   \
     public:                                             \
-        newClass(const format & f, unsigned int status = 1) : superClass(f, status) { }; \
+        newClass(const FormatOrString & fs, unsigned int status = 1) : superClass(fs, status) { }; \
     };
 
 MakeError(Error, BaseError)
@@ -50,7 +59,7 @@ class SysError : public Error
 {
 public:
     int errNo;
-    SysError(const format & f);
+    SysError(const FormatOrString & fs);
 };
 
 
@@ -63,8 +72,8 @@ typedef string Path;
 typedef list<Path> Paths;
 typedef set<Path> PathSet;
 
- 
-typedef enum { 
+
+typedef enum {
     lvlError = 0,
     lvlInfo,
     lvlTalkative,
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index b264fc5f3d13..15c462ce4e5b 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -25,22 +25,22 @@ extern char * * environ;
 namespace nix {
 
 
-BaseError::BaseError(const format & f, unsigned int status)
+BaseError::BaseError(const FormatOrString & fs, unsigned int status)
     : status(status)
 {
-    err = f.str();
+    err = fs.s;
 }
 
 
-BaseError & BaseError::addPrefix(const format & f)
+BaseError & BaseError::addPrefix(const FormatOrString & fs)
 {
-    prefix_ = f.str() + prefix_;
+    prefix_ = fs.s + prefix_;
     return *this;
 }
 
 
-SysError::SysError(const format & f)
-    : Error(format("%1%: %2%") % f.str() % strerror(errno))
+SysError::SysError(const FormatOrString & fs)
+    : Error(format("%1%: %2%") % fs.s % strerror(errno))
     , errNo(errno)
 {
 }
@@ -417,14 +417,14 @@ static string escVerbosity(Verbosity level)
 }
 
 
-void Nest::open(Verbosity level, const format & f)
+void Nest::open(Verbosity level, const FormatOrString & fs)
 {
     if (level <= verbosity) {
         if (logType == ltEscapes)
             std::cerr << "\033[" << escVerbosity(level) << "p"
-                      << f.str() << "\n";
+                      << fs.s << "\n";
         else
-            printMsg_(level, f);
+            printMsg_(level, fs);
         nest = true;
         nestingLevel++;
     }
@@ -442,7 +442,7 @@ void Nest::close()
 }
 
 
-void printMsg_(Verbosity level, const format & f)
+void printMsg_(Verbosity level, const FormatOrString & fs)
 {
     checkInterrupt();
     if (level > verbosity) return;
@@ -452,15 +452,15 @@ void printMsg_(Verbosity level, const format & f)
             prefix += "|   ";
     else if (logType == ltEscapes && level != lvlInfo)
         prefix = "\033[" + escVerbosity(level) + "s";
-    string s = (format("%1%%2%\n") % prefix % f.str()).str();
+    string s = (format("%1%%2%\n") % prefix % fs.s).str();
     writeToStderr(s);
 }
 
 
-void warnOnce(bool & haveWarned, const format & f)
+void warnOnce(bool & haveWarned, const FormatOrString & fs)
 {
     if (!haveWarned) {
-        printMsg(lvlError, format("warning: %1%") % f.str());
+        printMsg(lvlError, format("warning: %1%") % fs.s);
         haveWarned = true;
     }
 }
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index 5d0408f9b528..8bedfea9a0de 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -125,11 +125,11 @@ private:
 public:
     Nest();
     ~Nest();
-    void open(Verbosity level, const format & f);
+    void open(Verbosity level, const FormatOrString & fs);
     void close();
 };
 
-void printMsg_(Verbosity level, const format & f);
+void printMsg_(Verbosity level, const FormatOrString & fs);
 
 #define startNest(varName, level, f) \
     Nest varName; \
@@ -146,7 +146,7 @@ void printMsg_(Verbosity level, const format & f);
 
 #define debug(f) printMsg(lvlDebug, f)
 
-void warnOnce(bool & haveWarned, const format & f);
+void warnOnce(bool & haveWarned, const FormatOrString & fs);
 
 void writeToStderr(const string & s);