Rework LOOLProtocol::tokenize() to return a StringVector object

The bulk of this commit just changes std::vector<std::string> to
StringVector when we deal with tokens from a websocket message.

The less boring part of it is the new StringVector class, which is a
wrapper around std::vector<std::string>, and provides the same API,
except that operator[] returns a string, not a string&, and this allows
returning an empty string in case that prevents reading past the end of
the underlying array.

This means in case client code forgets to check size() before invoking
operator[], we don't crash. (See the ~3 previous commits which fixed
such crashes.)

Later the ctor could be changed to take a single underlying string to
avoid lots of tiny allocations, that's not yet done in this commit.

Change-Id: I8a6082143a8ac0b65824f574b32104d7889c184f
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/89687
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

12 files changed, 127 insertions, 30 deletions

diff --git a/common/Message.hpp b/common/Message.hpp
index 3fdbc5d363..9320037c7b 100644
--- a/common/Message.hpp
+++ b/common/Message.hpp
@@ -80,11 +80,11 @@ public:
     size_t size() const { return _data.size(); }
     const std::vector<char>& data() const { return _data; }
 
-    const std::vector<std::string>& tokens() const { return _tokens; }
+    const StringVector& tokens() const { return _tokens; }
     const std::string& forwardToken() const { return _forwardToken; }
-    const std::string& firstToken() const { return _tokens[0]; }
+    std::string firstToken() const { return _tokens[0]; }
     const std::string& firstLine() const { return _firstLine; }
-    const std::string& operator[](size_t index) const { return _tokens[index]; }
+    std::string operator[](size_t index) const { return _tokens[index]; }
 
     bool getTokenInteger(const std::string& name, int& value)
     {
@@ -177,7 +177,7 @@ private:
 private:
     const std::string _forwardToken;
     std::vector<char> _data;
-    const std::vector<std::string> _tokens;
+    const StringVector _tokens;
     const std::string _id;
     const std::string _firstLine;
     const std::string _abbr;
diff --git a/common/MessageQueue.cpp b/common/MessageQueue.cpp
index dd8ea33b35..84987b278a 100644
--- a/common/MessageQueue.cpp
+++ b/common/MessageQueue.cpp
@@ -30,7 +30,7 @@ void TileQueue::put_impl(const Payload& value)
     {
         LOG_TRC("Processing [" << LOOLProtocol::getAbbreviatedMessage(msg) << "]. Before canceltiles have " << getQueue().size() << " in queue.");
         const std::string seqs = msg.substr(12);
-        std::vector<std::string> tokens(LOOLProtocol::tokenize(seqs, ','));
+        StringVector tokens(LOOLProtocol::tokenize(seqs, ','));
         getQueue().erase(std::remove_if(getQueue().begin(), getQueue().end(),
                 [&tokens](const Payload& v)
                 {
@@ -126,7 +126,7 @@ void TileQueue::removeTileDuplicate(const std::string& tileMsg)
 namespace {
 
 /// Read the viewId from the tokens.
-std::string extractViewId(const std::string& origMsg, const std::vector<std::string>& tokens)
+std::string extractViewId(const std::string& origMsg, const StringVector& tokens)
 {
     size_t nonJson = tokens[0].size() + tokens[1].size() + tokens[2].size() + 3; // including spaces
     std::string jsonString(origMsg.data() + nonJson, origMsg.size() - nonJson);
@@ -151,7 +151,7 @@ std::string extractUnoCommand(const std::string& command)
 }
 
 /// Extract rectangle from the invalidation callback
-bool extractRectangle(const std::vector<std::string>& tokens, int& x, int& y, int& w, int& h, int& part)
+bool extractRectangle(const StringVector& tokens, int& x, int& y, int& w, int& h, int& part)
 {
     x = 0;
     y = 0;
@@ -186,7 +186,7 @@ std::string TileQueue::removeCallbackDuplicate(const std::string& callbackMsg)
 {
     assert(LOOLProtocol::matchPrefix("callback", callbackMsg, /*ignoreWhitespace*/ true));
 
-    std::vector<std::string> tokens = LOOLProtocol::tokenize(callbackMsg);
+    StringVector tokens = LOOLProtocol::tokenize(callbackMsg);
 
     if (tokens.size() < 3)
         return std::string();
@@ -211,7 +211,7 @@ std::string TileQueue::removeCallbackDuplicate(const std::string& callbackMsg)
         {
             auto& it = getQueue()[i];
 
-            std::vector<std::string> queuedTokens = LOOLProtocol::tokenize(it.data(), it.size());
+            StringVector queuedTokens = LOOLProtocol::tokenize(it.data(), it.size());
             if (queuedTokens.size() < 3)
             {
                 ++i;
@@ -316,7 +316,7 @@ std::string TileQueue::removeCallbackDuplicate(const std::string& callbackMsg)
         {
             auto& it = getQueue()[i];
 
-            std::vector<std::string> queuedTokens = LOOLProtocol::tokenize(it.data(), it.size());
+            StringVector queuedTokens = LOOLProtocol::tokenize(it.data(), it.size());
             if (queuedTokens.size() < 4)
                 continue;
 
@@ -362,7 +362,7 @@ std::string TileQueue::removeCallbackDuplicate(const std::string& callbackMsg)
             if (!LOOLProtocol::matchPrefix("callback", it))
                 continue;
 
-            std::vector<std::string> queuedTokens = LOOLProtocol::tokenize(it.data(), it.size());
+            StringVector queuedTokens = LOOLProtocol::tokenize(it.data(), it.size());
             if (queuedTokens.size() < 3)
                 continue;
 
diff --git a/common/Protocol.cpp b/common/Protocol.cpp
index 4d8f51c50d..7e5208774a 100644
--- a/common/Protocol.cpp
+++ b/common/Protocol.cpp
@@ -27,12 +27,12 @@ namespace LOOLProtocol
         int minor = -1;
         std::string patch;
 
-        std::vector<std::string> firstTokens(tokenize(version, '.'));
+        StringVector firstTokens(tokenize(version, '.'));
         if (firstTokens.size() > 0)
         {
             major = std::stoi(firstTokens[0]);
 
-            std::vector<std::string> secondTokens;
+            StringVector secondTokens;
             if (firstTokens.size() > 1)
             {
                 secondTokens = tokenize(firstTokens[1], '-');
@@ -170,7 +170,7 @@ namespace LOOLProtocol
         return false;
     }
 
-    bool getTokenInteger(const std::vector<std::string>& tokens, const std::string& name, int& value)
+    bool getTokenInteger(const StringVector& tokens, const std::string& name, int& value)
     {
         for (size_t i = 0; i < tokens.size(); i++)
         {
@@ -180,7 +180,7 @@ namespace LOOLProtocol
         return false;
     }
 
-    bool getTokenKeyword(const std::vector<std::string>& tokens, const std::string& name, const std::map<std::string, int>& map, int& value)
+    bool getTokenKeyword(const StringVector& tokens, const std::string& name, const std::map<std::string, int>& map, int& value)
     {
         for (size_t i = 0; i < tokens.size(); i++)
         {
diff --git a/common/Protocol.hpp b/common/Protocol.hpp
index e1a5d426e7..787be3706c 100644
--- a/common/Protocol.hpp
+++ b/common/Protocol.hpp
@@ -75,11 +75,11 @@ namespace LOOLProtocol
     bool getTokenString(const std::string& token, const std::string& name, std::string& value);
     bool getTokenKeyword(const std::string& token, const std::string& name, const std::map<std::string, int>& map, int& value);
 
-    bool getTokenKeyword(const std::vector<std::string>& tokens, const std::string& name, const std::map<std::string, int>& map, int& value);
+    bool getTokenKeyword(const StringVector& tokens, const std::string& name, const std::map<std::string, int>& map, int& value);
 
-    bool getTokenInteger(const std::vector<std::string>& tokens, const std::string& name, int& value);
+    bool getTokenInteger(const StringVector& tokens, const std::string& name, int& value);
 
-    inline bool getTokenString(const std::vector<std::string>& tokens,
+    inline bool getTokenString(const StringVector& tokens,
                                const std::string& name,
                                std::string& value)
     {
@@ -99,12 +99,12 @@ namespace LOOLProtocol
 
     /// Tokenize space-delimited values until we hit new-line or the end.
     inline
-    std::vector<std::string> tokenize(const char* data, const size_t size, const char delimiter = ' ')
+    StringVector tokenize(const char* data, const size_t size, const char delimiter = ' ')
     {
         std::vector<std::string> tokens;
         if (size == 0 || data == nullptr)
         {
-            return tokens;
+            return StringVector(tokens);
         }
         tokens.reserve(8);
 
@@ -132,11 +132,11 @@ namespace LOOLProtocol
             tokens.emplace_back(start, end);
         }
 
-        return tokens;
+        return StringVector(tokens);
     }
 
     inline
-    std::vector<std::string> tokenize(const std::string& s, const char delimiter = ' ')
+    StringVector tokenize(const std::string& s, const char delimiter = ' ')
     {
         return tokenize(s.data(), s.size(), delimiter);
     }
diff --git a/common/Seccomp.cpp b/common/Seccomp.cpp
index c22937f9fa..1160568922 100644
--- a/common/Seccomp.cpp
+++ b/common/Seccomp.cpp
@@ -269,7 +269,7 @@ void setRLimit(rlim_t confLim, int resource, const std::string &resourceText, co
         LOG_INF("Ignored setting " << resourceText << " to " << limTextWithUnit << ".");
 }
 
-bool handleSetrlimitCommand(const std::vector<std::string>& tokens)
+bool handleSetrlimitCommand(const StringVector& tokens)
 {
     if (tokens.size() == 3 && tokens[0] == "setconfig")
     {
diff --git a/common/Seccomp.hpp b/common/Seccomp.hpp
index 5a09de7725..400745bfa4 100644
--- a/common/Seccomp.hpp
+++ b/common/Seccomp.hpp
@@ -9,8 +9,7 @@
 #ifndef INCLUDED_SECCOMP_HPP
 #define INCLUDED_SECCOMP_HPP
 
-#include <string>
-#include <vector>
+#include <Protocol.hpp>
 
 namespace Seccomp {
     enum Type { KIT, WSD };
@@ -22,7 +21,7 @@ namespace Seccomp {
 namespace Rlimit {
     /// Handles setconfig command with limit_... subcommands.
     /// Returns true iff it handled the command, regardless of success/failure.
-    bool handleSetrlimitCommand(const std::vector<std::string>& tokens);
+    bool handleSetrlimitCommand(const StringVector& tokens);
 };
 
 #endif
diff --git a/common/Session.cpp b/common/Session.cpp
index 2a51b0e89f..815fd53913 100644
--- a/common/Session.cpp
+++ b/common/Session.cpp
@@ -75,7 +75,7 @@ bool Session::sendBinaryFrame(const char *buffer, int length)
     return sendMessage(buffer, length, WSOpCode::Binary) >= length;
 }
 
-void Session::parseDocOptions(const std::vector<std::string>& tokens, int& part, std::string& timestamp, std::string& doctemplate)
+void Session::parseDocOptions(const StringVector& tokens, int& part, std::string& timestamp, std::string& doctemplate)
 {
     // First token is the "load" command itself.
     size_t offset = 1;
diff --git a/common/Session.hpp b/common/Session.hpp
index aa585a559f..8171fef64d 100644
--- a/common/Session.hpp
+++ b/common/Session.hpp
@@ -168,7 +168,7 @@ protected:
 
     /// Parses the options of the "load" command,
     /// shared between MasterProcessSession::loadDocument() and ChildProcessSession::loadDocument().
-    void parseDocOptions(const std::vector<std::string>& tokens, int& part, std::string& timestamp, std::string& doctemplate);
+    void parseDocOptions(const StringVector& tokens, int& part, std::string& timestamp, std::string& doctemplate);
 
     void updateLastActivityTime()
     {
diff --git a/common/StringVector.cpp b/common/StringVector.cpp
new file mode 100644
index 0000000000..a4bf781095
--- /dev/null
+++ b/common/StringVector.cpp
@@ -0,0 +1,45 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include "StringVector.hpp"
+
+StringVector::StringVector() = default;
+
+StringVector::StringVector(const std::vector<std::string>& vector) { _vector = vector; }
+
+std::string StringVector::operator[](size_t index) const
+{
+    if (index >= _vector.size())
+    {
+        return std::string();
+    }
+
+    return _vector[index];
+}
+
+size_t StringVector::size() const { return _vector.size(); }
+
+bool StringVector::empty() const { return _vector.empty(); }
+
+std::vector<std::string>::const_iterator StringVector::begin() const { return _vector.begin(); }
+
+std::vector<std::string>::iterator StringVector::begin() { return _vector.begin(); }
+
+std::vector<std::string>::const_iterator StringVector::end() const { return _vector.end(); }
+
+std::vector<std::string>::iterator StringVector::end() { return _vector.end(); }
+
+std::vector<std::string>::iterator StringVector::erase(std::vector<std::string>::const_iterator it)
+{
+    return _vector.erase(it);
+}
+
+void StringVector::push_back(const std::string& string) { _vector.push_back(string); }
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/common/StringVector.hpp b/common/StringVector.hpp
new file mode 100644
index 0000000000..346bfabfff
--- /dev/null
+++ b/common/StringVector.hpp
@@ -0,0 +1,51 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#ifndef INCLUDED_STRINGVECTOR_HPP
+#define INCLUDED_STRINGVECTOR_HPP
+
+#include <string>
+#include <vector>
+
+/**
+ * Safe wrapper around an std::vector<std::string>. Gives you an empty string if you would read past
+ * the ends of the vector.
+ */
+class StringVector
+{
+    std::vector<std::string> _vector;
+
+public:
+    explicit StringVector();
+
+    explicit StringVector(const std::vector<std::string>& vector);
+
+    /// Unlike std::vector, gives an empty string if index is unexpected.
+    std::string operator[](size_t index) const;
+
+    size_t size() const;
+
+    bool empty() const;
+
+    std::vector<std::string>::const_iterator begin() const;
+
+    std::vector<std::string>::iterator begin();
+
+    std::vector<std::string>::const_iterator end() const;
+
+    std::vector<std::string>::iterator end();
+
+    std::vector<std::string>::iterator erase(std::vector<std::string>::const_iterator it);
+
+    void push_back(const std::string& string);
+};
+
+#endif
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/common/Util.cpp b/common/Util.cpp
index f9df08f2f8..9a2273eaff 100644
--- a/common/Util.cpp
+++ b/common/Util.cpp
@@ -245,7 +245,7 @@ namespace Util
         }
     }
 
-    int spawnProcess(const std::string &cmd, const std::vector<std::string> &args, const std::vector<int>* fdsToKeep, int *stdInput)
+    int spawnProcess(const std::string &cmd, const StringVector &args, const std::vector<int>* fdsToKeep, int *stdInput)
     {
         int pipeFds[2] = { -1, -1 };
         if (stdInput)
diff --git a/common/Util.hpp b/common/Util.hpp
index 991b548ef8..52043692f3 100644
--- a/common/Util.hpp
+++ b/common/Util.hpp
@@ -38,6 +38,8 @@
 #define LOK_USE_UNSTABLE_API
 #include <LibreOfficeKit/LibreOfficeKitEnums.h>
 
+#include <StringVector.hpp>
+
 namespace Util
 {
     namespace rng
@@ -68,7 +70,7 @@ namespace Util
 
     /// Spawn a process if stdInput is non-NULL it contains a writable descriptor
     /// to send data to the child.
-    int spawnProcess(const std::string &cmd, const std::vector<std::string> &args,
+    int spawnProcess(const std::string &cmd, const StringVector &args,
                      const std::vector<int>* fdsToKeep = nullptr, int *stdInput = nullptr);
     
 #endif