Escape relevant fields when rendering internal visitor status page

3 files changed, 33 insertions, 27 deletions

diff --git a/storage/src/vespa/storage/visiting/visitor.cpp b/storage/src/vespa/storage/visiting/visitor.cpp
index e8a217fc718..91f304ad9a0 100644
--- a/storage/src/vespa/storage/visiting/visitor.cpp
+++ b/storage/src/vespa/storage/visiting/visitor.cpp
@@ -2,15 +2,15 @@
 
 #include "visitor.h"
 #include "visitormetrics.h"
+#include <vespa/document/select/node.h>
+#include <vespa/document/fieldset/fieldsets.h>
+#include <vespa/documentapi/messagebus/messages/visitor.h>
 #include <vespa/persistence/spi/docentry.h>
-#include <vespa/storageframework/generic/clock/timer.h>
 #include <vespa/storageapi/message/datagram.h>
+#include <vespa/storageframework/generic/clock/timer.h>
 #include <vespa/storage/persistence/messages.h>
-#include <vespa/documentapi/messagebus/messages/visitor.h>
-#include <vespa/document/select/node.h>
-#include <vespa/document/fieldset/fieldsets.h>
-#include <vespa/vespalib/stllike/hash_map.hpp>
 #include <vespa/vespalib/stllike/asciistream.h>
+#include <vespa/vespalib/util/string_escape.h>
 #include <vespa/vespalib/util/stringfmt.h>
 #include <unordered_map>
 #include <sstream>
@@ -932,10 +932,12 @@ Visitor::continueVisitor()
 void
 Visitor::getStatus(std::ostream& out, bool verbose) const
 {
+    using vespalib::xml_content_escaped;
+
     out << "<table border=\"1\"><tr><td>Property</td><td>Value</td></tr>\n";
 
     out << "<tr><td>Visitor id</td><td>" << _visitorId << "</td></tr>\n";
-    out << "<tr><td>Visitor name</td><td>" << _id << "</td></tr>\n";
+    out << "<tr><td>Visitor name</td><td>" << xml_content_escaped(_id) << "</td></tr>\n";
 
     out << "<tr><td>Number of buckets to visit</td><td>" << _buckets.size()
         << "</td></tr>\n";
@@ -953,7 +955,7 @@ Visitor::getStatus(std::ostream& out, bool verbose) const
         << "</td></tr>\n";
 
     out << "<tr><td>Current status</td><td>"
-        << _result << "</td></tr>\n";
+        << xml_content_escaped(_result.toString()) << "</td></tr>\n";
 
     out << "<tr><td>Failed</td><td>" << (failed() ? "true" : "false")
         << "</td></tr>\n";
@@ -973,28 +975,28 @@ Visitor::getStatus(std::ostream& out, bool verbose) const
         out << "<tr><td>Called completed visitor</td><td>"
             << (_calledCompletedVisitor ? "true" : "false") << "</td></tr>\n";
         out << "<tr><td>Visiting fields</td><td>"
-            << _visitorOptions._fieldSet
+            << xml_content_escaped(_visitorOptions._fieldSet)
             << "</td></tr>\n";
         out << "<tr><td>Visiting removes</td><td>"
             << (_visitorOptions._visitRemoves ? "true" : "false")
             << "</td></tr>\n";
         out << "<tr><td>Control destination</td><td>";
         if (_controlDestination.get()) {
-            out << _controlDestination->toString();
+            out << xml_content_escaped(_controlDestination->toString());
         } else {
             out << "nil";
         }
         out << "</td></tr>\n";
         out << "<tr><td>Data destination</td><td>";
         if (_dataDestination.get()) {
-            out << _dataDestination->toString();
+            out << xml_content_escaped(_dataDestination->toString());
         } else {
             out << "nil";
         }
         out << "</td></tr>\n";
         out << "<tr><td>Document selection</td><td>";
         if (_documentSelection.get()) {
-            out << *_documentSelection;
+            out << xml_content_escaped(_documentSelection->toString());
         } else {
             out << "nil";
         }
@@ -1052,7 +1054,7 @@ Visitor::getStatus(std::ostream& out, bool verbose) const
     for (auto& idAndMeta : _visitorTarget._messageMeta) {
         const VisitorTarget::MessageMeta& meta(idAndMeta.second);
         out << "Message #" << idAndMeta.first << " <b>"
-            << meta.messageText << "</b> ";
+            << xml_content_escaped(meta.messageText) << "</b> ";
         if (meta.retryCount > 0) {
             out << "Retried " << meta.retryCount << " times. ";
         }
diff --git a/storage/src/vespa/storage/visiting/visitormanager.cpp b/storage/src/vespa/storage/visiting/visitormanager.cpp
index c548ef9e20c..b305abae019 100644
--- a/storage/src/vespa/storage/visiting/visitormanager.cpp
+++ b/storage/src/vespa/storage/visiting/visitormanager.cpp
@@ -7,11 +7,12 @@
 #include "testvisitor.h"
 #include "recoveryvisitor.h"
 #include "reindexing_visitor.h"
-#include <vespa/storage/common/statusmessages.h>
 #include <vespa/config/subscription/configuri.h>
 #include <vespa/config/common/exceptions.h>
-#include <vespa/vespalib/util/stringfmt.h>
 #include <vespa/config/helper/configfetcher.hpp>
+#include <vespa/storage/common/statusmessages.h>
+#include <vespa/vespalib/util/string_escape.h>
+#include <vespa/vespalib/util/stringfmt.h>
 #include <cassert>
 
 #include <vespa/log/log.h>
@@ -557,6 +558,9 @@ void
 VisitorManager::reportHtmlStatus(std::ostream& out,
                                  const framework::HttpUrlPath& path) const
 {
+    using vespalib::xml_attribute_escaped;
+    using vespalib::xml_content_escaped;
+
     bool showStatus = !path.hasAttribute("visitor");
     bool verbose = path.hasAttribute("verbose");
     bool showAll = path.hasAttribute("allvisitors");
@@ -568,7 +572,7 @@ VisitorManager::reportHtmlStatus(std::ostream& out,
         << " | <a href=\"?allvisitors" << (verbose ? "&verbose" : "")
         << "\">Show all visitors</a>"
         << " | <a href=\"?" << (verbose ? "notverbose" : "verbose");
-    if (!showStatus) out << "&visitor=" << path.get("visitor", std::string(""));
+    if (!showStatus) out << "&visitor=" << xml_attribute_escaped(path.get("visitor", std::string("")));
     if (showAll) out << "&allvisitors";
     out << "\">" << (verbose ? "Less verbose" : "More verbose") << "</a>\n"
         << " ]</font><br><br>\n";
@@ -585,7 +589,8 @@ VisitorManager::reportHtmlStatus(std::ostream& out,
                     out << " none";
                 } else {
                     for (const auto& id_and_visitor : _visitorThread[i].second) {
-                        out << " " << id_and_visitor.second << " (" << id_and_visitor.first << ")";
+                        out << " " << xml_content_escaped(id_and_visitor.second)
+                            << " (" << id_and_visitor.first << ")";
                     }
                 }
                 out << "<br>\n";
@@ -596,7 +601,8 @@ VisitorManager::reportHtmlStatus(std::ostream& out,
             for (const auto& enqueued : _visitorQueue) {
                 auto& cmd = enqueued._command;
                 assert(cmd);
-                out << "<li>" << cmd->getInstanceId() << " - "
+                out << "<li>"
+                    << xml_content_escaped(cmd->getInstanceId()) << " - "
                     << vespalib::count_ms(cmd->getQueueTimeout()) << ", remaining timeout "
                     << vespalib::count_ms(enqueued._deadline - now) << " ms\n";
             }
@@ -619,7 +625,7 @@ VisitorManager::reportHtmlStatus(std::ostream& out,
                         << "<td>" << entry.second.id << "</td>"
                         << "<td>" << entry.second.timestamp << "</td>"
                         << "<td>" << vespalib::count_ms(entry.second.timeout) << "</td>"
-                        << "<td>" << entry.second.destination << "</td>"
+                        << "<td>" << xml_content_escaped(entry.second.destination) << "</td>"
                         << "</tr>\n";
                 }
                 out << "</table>\n";
@@ -632,13 +638,13 @@ VisitorManager::reportHtmlStatus(std::ostream& out,
             << ", variable = " << _maxVariableConcurrentVisitors
             << ", waiting visitors " << _visitorQueue.size() << "<br>\n";
     }
-        // Only one can access status at a time as _statusRequest only holds
-        // answers from one request at a time
+    // Only one can access status at a time as _statusRequest only holds
+    // answers from one request at a time
     std::unique_lock sync(_statusLock);
-        // Send all subrequests
+    // Send all sub-requests
     uint32_t parts = _visitorThread.size();
     for (uint32_t i=0; i<parts; ++i) {
-        std::shared_ptr<RequestStatusPage> cmd(new RequestStatusPage(path));
+        auto cmd = std::make_shared<RequestStatusPage>(path);
         std::ostringstream token;
         token << "Visitor thread " << i;
         cmd->setSortToken(token.str());
@@ -648,7 +654,7 @@ VisitorManager::reportHtmlStatus(std::ostream& out,
     _statusCond.wait(sync, [&]() { return (_statusRequest.size() >= parts);});
     std::sort(_statusRequest.begin(), _statusRequest.end(), StatusReqSorter());
 
-        // Create output
+    // Create output
     for (uint32_t i=0; i<_statusRequest.size(); ++i) {
         out << "<h2>" << _statusRequest[i]->getSortToken()
             << "</h2>\n" << _statusRequest[i]->getStatus() << "\n";
diff --git a/storage/src/vespa/storage/visiting/visitorthread.cpp b/storage/src/vespa/storage/visiting/visitorthread.cpp
index cc3e709a848..ba2a7584ff4 100644
--- a/storage/src/vespa/storage/visiting/visitorthread.cpp
+++ b/storage/src/vespa/storage/visiting/visitorthread.cpp
@@ -2,18 +2,16 @@
 
 #include "visitorthread.h"
 #include "messages.h"
+#include <vespa/document/base/exceptions.h>
 #include <vespa/document/select/bodyfielddetector.h>
 #include <vespa/document/select/parser.h>
 #include <vespa/messagebus/rpcmessagebus.h>
+#include <vespa/storageapi/message/datagram.h>
 #include <vespa/storage/common/statusmessages.h>
 #include <vespa/storage/config/config-stor-server.h>
-#include <vespa/storageapi/message/datagram.h>
 #include <vespa/vespalib/stllike/asciistream.h>
 #include <vespa/vespalib/util/exceptions.h>
-#include <vespa/document/base/exceptions.h>
-#include <vespa/vespalib/stllike/hash_map.hpp>
 #include <locale>
-#include <sstream>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".visitor.thread");