Clean up code that replaces secrets with asterisks

modules/ripple_basics/utility/ripple_Log.cpp

@@ -35,6 +35,60 @@ std::vector< std::pair<std::string, std::string> > LogPartition::getSeverities()
 	return sevs;
 }
 
+//------------------------------------------------------------------------------
+
+// VFALCO: TODO, remove original code once we know the replacement is correct.
+// Original code
+/*
+std::string ls = oss.str();
+size_t s = ls.find("\"secret\"");
+if (s != std::string::npos)
+{
+	s += 8;
+	size_t sEnd = ls.size() - 1;
+	if (sEnd > (s + 35))
+		sEnd = s + 35;
+	for (int i = s; i < sEnd; ++i)
+		ls[i] = '*';
+}
+logMsg += ls;
+*/
+
+//------------------------------------------------------------------------------
+
+std::string Log::replaceFirstSecretWithAsterisks (std::string s)
+{
+    using namespace std;
+
+    char const* secretToken = "\"secret\"";
+
+    // Look for the first occurrence of "secret" in the string.
+    //
+    size_t startingPosition = s.find (secretToken);
+
+    if (startingPosition != string::npos)
+    {
+        // Found it, advance past the token.
+        //
+        startingPosition += strlen (secretToken);
+
+        // VFALCO: NOTE, are base58 encoded secrets always exactly 35 characters?
+        //         I thought that the secret was variable length. Or is that the
+        //         public / private keys?
+        //
+        // Replace the next 35 characters at most, without overwriting the end.
+        // 
+        size_t endingPosition = std::min (startingPosition + 35, s.size () - 1);
+
+        for (size_t i = startingPosition; i < endingPosition; ++i)
+            s [i] = '*';
+    }
+
+    return s;
+}
+
+//------------------------------------------------------------------------------
+
 Log::~Log()
 {
 	std::string logMsg = boost::posix_time::to_simple_string(boost::posix_time::second_clock::universal_time());
@@ -54,18 +108,7 @@ Log::~Log()
 		case lsINVALID:	assert(false); return;
 	}
 
-	std::string ls = oss.str();
+    logMsg += replaceFirstSecretWithAsterisks (oss.str ());
-	size_t s = ls.find("\"secret\"");
-	if (s != std::string::npos)
-	{
-		s += 8;
-		size_t sEnd = ls.size() - 1;
-		if (sEnd > (s + 35))
-			sEnd = s + 35;
-		for (int i = s; i < sEnd; ++i)
-			ls[i] = '*';
-	}
-	logMsg += ls;
 
 	if (logMsg.size() > LOG_MAX_MESSAGE)
 	{

modules/ripple_basics/utility/ripple_Log.h

@@ -63,48 +63,62 @@ public:
 
 class Log
 {
-private:
-	Log(const Log&);			// no implementation
-	Log& operator=(const Log&);	// no implementation
-
-protected:
-	static boost::recursive_mutex sLock;
-	static LogSeverity sMinSeverity;
-	static std::ofstream* outStream;
-
-	mutable std::ostringstream	oss;
-	LogSeverity					mSeverity;
-	std::string					mPartitionName;
-
-	static boost::filesystem::path *pathToLog;
-	static uint32 logRotateCounter;
-
 public:
-	Log(LogSeverity s) : mSeverity(s)
+	explicit Log (LogSeverity s) : mSeverity(s)
-	{ ; }
+	{
+    }
 
-	Log(LogSeverity s, const LogPartition& p) : mSeverity(s), mPartitionName(p.getName())
+	Log (LogSeverity s, LogPartition const& p)
-	{ ; }
+        : mSeverity (s)
+        , mPartitionName (p.getName())
+	{
+    }
 
-	~Log();
+	~Log ();
 
-	template<typename T> std::ostream& operator<<(const T& t) const
+	template <class T>
+    std::ostream& operator<< (const T& t) const
 	{
 		return oss << t;
 	}
 
-	std::ostringstream& ref(void) const
+	std::ostringstream& ref () const
 	{
 		return oss;
 	}
 
-	static std::string severityToString(LogSeverity);
+	static std::string severityToString (LogSeverity);
-	static LogSeverity stringToSeverity(const std::string&);
 
-	static LogSeverity getMinSeverity();
+    static LogSeverity stringToSeverity (std::string const&);
-	static void setMinSeverity(LogSeverity, bool all);
+
-	static void setLogFile(boost::filesystem::path const&);
+	static LogSeverity getMinSeverity ();
-	static std::string rotateLog(void);
+
+    static void setMinSeverity (LogSeverity, bool all);
+	
+    static void setLogFile (boost::filesystem::path const& pathToLogFile);
+	
+    static std::string rotateLog ();
+
+private:
+    // VFALCO: TODO, derive from beast::Uncopyable
+	Log (const Log&);            // no implementation
+	Log& operator= (const Log&); // no implementation
+
+    // VFALCO: TODO, looks like there are really TWO classes in here.
+    //         One is a stream target for '<<' operator and the other
+    //         is a singleton. Split the singleton out to a new class.
+    //
+    static boost::recursive_mutex sLock;
+	static LogSeverity sMinSeverity;
+	static std::ofstream* outStream;
+	static boost::filesystem::path *pathToLog;
+	static uint32 logRotateCounter;
+
+    static std::string replaceFirstSecretWithAsterisks (std::string s);
+
+    mutable std::ostringstream	oss;
+	LogSeverity					mSeverity;
+	std::string					mPartitionName;
 };
 
 // Manually test for whether we should log