[Libreoffice-commits] online.git: net/Socket.cpp net/Socket.hpp test/UnitWOPITemplate.cpp wsd/ClientSession.cpp wsd/FileServer.cpp wsd/LOOLWSD.cpp

Michael Meeks (via logerrit) logerrit at kemper.freedesktop.org
Wed Jun 3 16:15:54 UTC 2020


 net/Socket.cpp            |   38 ++++++++++++++++++++++----------------
 net/Socket.hpp            |    8 ++++----
 test/UnitWOPITemplate.cpp |    4 +---
 wsd/ClientSession.cpp     |    2 +-
 wsd/FileServer.cpp        |    3 ++-
 wsd/LOOLWSD.cpp           |    8 ++------
 6 files changed, 32 insertions(+), 31 deletions(-)

New commits:
commit 714640993bb09f34cef97e878fd8e8b13ea4fb2b
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Wed Jun 3 16:06:10 2020 +0100
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Wed Jun 3 18:15:33 2020 +0200

    Remember to shutdown the socket after serving files.
    
    re-factor to make it hard not to.
    
    Change-Id: I26ebc48b4660276ede64a22167ac4779cebf5cd4
    Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95440
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/net/Socket.cpp b/net/Socket.cpp
index e388d412a..ee76a5a1a 100644
--- a/net/Socket.cpp
+++ b/net/Socket.cpp
@@ -1022,14 +1022,19 @@ namespace HttpHelper
         }
     }
 
-    void sendFile(const std::shared_ptr<StreamSocket>& socket,
-                  const std::string& path,
-                  const std::string& mediaType,
-                  Poco::Net::HTTPResponse& response,
-                  const bool noCache,
-                  const bool deflate,
-                  const bool headerOnly)
+    void sendFileAndShutdown(const std::shared_ptr<StreamSocket>& socket,
+                             const std::string& path,
+                             const std::string& mediaType,
+                             Poco::Net::HTTPResponse *optResponse,
+                             const bool noCache,
+                             const bool deflate,
+                             const bool headerOnly)
     {
+        Poco::Net::HTTPResponse *response = optResponse;
+        Poco::Net::HTTPResponse  localResponse;
+        if (!response)
+            response = &localResponse;
+
         struct stat st;
         if (stat(path.c_str(), &st) != 0)
         {
@@ -1040,16 +1045,16 @@ namespace HttpHelper
         if (!noCache)
         {
             // 60 * 60 * 24 * 128 (days) = 11059200
-            response.set("Cache-Control", "max-age=11059200");
-            response.set("ETag", "\"" LOOLWSD_VERSION_HASH "\"");
+            response->set("Cache-Control", "max-age=11059200");
+            response->set("ETag", "\"" LOOLWSD_VERSION_HASH "\"");
         }
         else
         {
-            response.set("Cache-Control", "no-cache");
+            response->set("Cache-Control", "no-cache");
         }
 
-        response.setContentType(mediaType);
-        response.add("X-Content-Type-Options", "nosniff");
+        response->setContentType(mediaType);
+        response->add("X-Content-Type-Options", "nosniff");
 
         int bufferSize = std::min(st.st_size, (off_t)Socket::MaximumSendBufferSize);
         if (st.st_size >= socket->getSendBufferSize())
@@ -1063,24 +1068,25 @@ namespace HttpHelper
         // IE/Edge before enabling the deflate again
         if (!deflate || true)
         {
-            response.setContentLength(st.st_size);
+            response->setContentLength(st.st_size);
             LOG_TRC('#' << socket->getFD() << ": Sending " <<
                     (headerOnly ? "header for " : "") << " file [" << path << "].");
-            socket->send(response);
+            socket->send(*response);
 
             if (!headerOnly)
                 sendUncompressedFileContent(socket, path, bufferSize);
         }
         else
         {
-            response.set("Content-Encoding", "deflate");
+            response->set("Content-Encoding", "deflate");
             LOG_TRC('#' << socket->getFD() << ": Sending " <<
                     (headerOnly ? "header for " : "") << " file [" << path << "].");
-            socket->send(response);
+            socket->send(*response);
 
             if (!headerOnly)
                 sendDeflatedFileContent(socket, path, st.st_size);
         }
+        socket->shutdown();
     }
 }
 
diff --git a/net/Socket.hpp b/net/Socket.hpp
index 7e011bb9d..55cfe05bb 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -1278,10 +1278,10 @@ enum class WSOpCode : unsigned char {
 
 namespace HttpHelper
 {
-    /// Sends file as HTTP response.
-    void sendFile(const std::shared_ptr<StreamSocket>& socket, const std::string& path, const std::string& mediaType,
-                  Poco::Net::HTTPResponse& response, bool noCache = false, bool deflate = false,
-                  const bool headerOnly = false);
+    /// Sends file as HTTP response and shutdown the socket.
+    void sendFileAndShutdown(const std::shared_ptr<StreamSocket>& socket, const std::string& path, const std::string& mediaType,
+                             Poco::Net::HTTPResponse *optResponse = nullptr, bool noCache = false, bool deflate = false,
+                             const bool headerOnly = false);
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/test/UnitWOPITemplate.cpp b/test/UnitWOPITemplate.cpp
index 2ebb1a28c..83e1bb64f 100644
--- a/test/UnitWOPITemplate.cpp
+++ b/test/UnitWOPITemplate.cpp
@@ -94,9 +94,7 @@ public:
         {
             LOG_INF("Fake wopi host request, handling template GetFile: " << uriReq.getPath());
 
-            Poco::Net::HTTPResponse response;
-            HttpHelper::sendFile(socket, TDOC "/test.ott", "", response);
-            socket->shutdown();
+            HttpHelper::sendFileAndShutdown(socket, TDOC "/test.ott", "");
 
             return true;
         }
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index d534a2656..904867b18 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -1271,7 +1271,7 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt
                 if (!fileName.empty())
                     response.set("Content-Disposition", "attachment; filename=\"" + fileName + '"');
 
-                HttpHelper::sendFile(_saveAsSocket, encodedFilePath, mimeType, response);
+                HttpHelper::sendFileAndShutdown(_saveAsSocket, encodedFilePath, mimeType, &response);
             }
 
             // Conversion is done, cleanup this fake session.
diff --git a/wsd/FileServer.cpp b/wsd/FileServer.cpp
index 1c47b0f8d..a298adb6c 100644
--- a/wsd/FileServer.cpp
+++ b/wsd/FileServer.cpp
@@ -442,7 +442,7 @@ void FileServerRequestHandler::handleRequest(const HTTPRequest& request,
                 // Useful to not serve from memory sometimes especially during loleaflet development
                 // Avoids having to restart loolwsd everytime you make a change in loleaflet
                 const std::string filePath = Poco::Path(LOOLWSD::FileServerRoot, relPath).absolute().toString();
-                HttpHelper::sendFile(socket, filePath, mimeType, response, noCache);
+                HttpHelper::sendFileAndShutdown(socket, filePath, mimeType, &response, noCache);
                 return;
             }
 #endif
@@ -470,6 +470,7 @@ void FileServerRequestHandler::handleRequest(const HTTPRequest& request,
                     (!gzip ? "un":"") << "compressed : file [" << relPath << "]: " << header);
             socket->send(header);
             socket->send(*content);
+            // shutdown by caller
         }
     }
     catch (const Poco::Net::NotAuthenticatedException& exc)
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index db7bfbaef..c672932f0 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2485,13 +2485,9 @@ private:
         std::string mimeType = "image/vnd.microsoft.icon";
         std::string faviconPath = Path(Application::instance().commandPath()).parent().toString() + "favicon.ico";
         if (!File(faviconPath).exists())
-        {
             faviconPath = LOOLWSD::FileServerRoot + "/favicon.ico";
-        }
 
-        Poco::Net::HTTPResponse response;
-        HttpHelper::sendFile(socket, faviconPath, mimeType, response);
-        socket->shutdown();
+        HttpHelper::sendFileAndShutdown(socket, faviconPath, mimeType);
     }
 
     void handleWopiDiscoveryRequest(const RequestDetails &requestDetails,
@@ -2900,7 +2896,7 @@ private:
 
                 try
                 {
-                    HttpHelper::sendFile(socket, filePath.toString(), contentType, response);
+                    HttpHelper::sendFileAndShutdown(socket, filePath.toString(), contentType, &response);
                 }
                 catch (const Exception& exc)
                 {


More information about the Libreoffice-commits mailing list