From 21f90d0ea2c0887e41a0e9edecfc12ffe8375b66 Mon Sep 17 00:00:00 2001 From: Rashesh Date: Fri, 27 Dec 2024 17:17:34 +0530 Subject: [PATCH] wsd: make sure browsersetting.json is sent early as possible - async browsersetting might not be ready by the time we download the document - now we do sync request if async has not yet finished the execution to make sure we get browsersetting as early as possible Signed-off-by: Rashesh Change-Id: I4b01b8bcbd0fb2397888495b982aba366b99c3b8 --- wsd/ClientSession.cpp | 3 +- wsd/ClientSession.hpp | 10 ++++ wsd/DocumentBroker.cpp | 111 +++++++++++++++++++++++++++++++---------- wsd/DocumentBroker.hpp | 4 +- 4 files changed, 99 insertions(+), 29 deletions(-) diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index 6b4332328a6a..5b980785eaae 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -93,7 +93,8 @@ ClientSession::ClientSession( _isTextDocument(false), _thumbnailSession(false), _canonicalViewId(0), - _sentAudit(false) + _sentAudit(false), + _sentBrowserSetting(false) { const std::size_t curConnections = ++COOLWSD::NumConnections; LOG_INF("ClientSession ctor [" << getName() << "] for URI: [" << _uriPublic.toString() diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp index 6f897af1f6c1..5ac7a88f8da8 100644 --- a/wsd/ClientSession.hpp +++ b/wsd/ClientSession.hpp @@ -272,6 +272,13 @@ class ClientSession final : public Session int getCanonicalViewId() const { return _canonicalViewId; } + bool getSentBrowserSetting() const { return _sentBrowserSetting; } + + void setSentBrowserSetting(const bool sentBrowserSetting) + { + _sentBrowserSetting = sentBrowserSetting; + } + private: std::shared_ptr client_from_this() { @@ -429,6 +436,9 @@ class ClientSession final : public Session /// If server audit was already sent bool _sentAudit; + + /// If browser setting was already sent + bool _sentBrowserSetting; }; /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 53f83f7bd624..3998ddd4ae37 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -1019,6 +1019,8 @@ bool DocumentBroker::download( // Call the storage specific fileinfo functions std::string templateSource; + std::string browserSettingUri; + #if !MOBILEAPP std::chrono::milliseconds checkFileInfoCallDurationMs = std::chrono::milliseconds::zero(); WopiStorage* wopiStorage = dynamic_cast(_storage.get()); @@ -1049,6 +1051,7 @@ bool DocumentBroker::download( if (session) { + browserSettingUri = wopiFileInfo->getBrowserSettingsUri(); templateSource = updateSessionWithWopiInfo(session, wopiStorage, std::move(wopiFileInfo)); } @@ -1170,6 +1173,27 @@ bool DocumentBroker::download( session->sendTextFrame(msg); } } + + // if async browsersetting json request is not downlaoded even after document download is complete + // we do sync request to make sure the browser setting json sent before document starts to load + if (session && !browserSettingUri.empty()) + { + LOG_DBG("BrowserSetting for docKey [" + << _docKey << "] for session #" << session->getId() + << (session->getSentBrowserSetting() ? " already exists" : " is missing")); + if (!session->getSentBrowserSetting()) + { + sendBrowserSetting(session, browserSettingUri, /** async **/ false); + if (!session->getSentBrowserSetting()) + { + const std::string uriAnonym = COOLWSD::anonymizeUrl(browserSettingUri); + LOG_ERR("Request to uri[" + << uriAnonym + << "] failed or timedout while adding session #" + session->getId()); + } + } + } + #endif return true; } @@ -1452,7 +1476,7 @@ DocumentBroker::updateSessionWithWopiInfo(const std::shared_ptr& std::string browserSettingUri = wopiFileInfo->getBrowserSettingsUri(); if (_sessions.empty() && !browserSettingUri.empty()) { - asyncSendBrowserSetting(session, browserSettingUri); + sendBrowserSetting(session, browserSettingUri); } // Pass the ownership to the client session. @@ -1635,8 +1659,8 @@ void DocumentBroker::asyncInstallPresets(const std::shared_ptr se _asyncInstallTask->appendCallback([this](bool){ _asyncInstallTask.reset(); }); } -void DocumentBroker::asyncSendBrowserSetting(const std::shared_ptr& session, - const std::string& browserSettingUri) +void DocumentBroker::sendBrowserSetting(const std::shared_ptr& session, + const std::string& browserSettingUri, bool async) { // Download the json for browser settings const Poco::URI settingsUri{ browserSettingUri }; @@ -1646,30 +1670,14 @@ void DocumentBroker::asyncSendBrowserSetting(const std::shared_ptr& configSession) + auto parseResponse = + [session, uriAnonym](const std::shared_ptr& httpResponse) { - if (SigUtil::getShutdownRequestFlag()) - { - LOG_DBG("Shutdown flagged, giving up on in-flight requests"); + if (session->getSentBrowserSetting()) return; - } - - const std::shared_ptr httpResponse = configSession->response(); - LOG_TRC("DocumentBroker::asyncSendBrowserSetting returned " - << httpResponse->statusLine().statusCode()); - - const bool failed = (httpResponse->statusLine().statusCode() != http::StatusCode::OK); - if (failed) - { - if (httpResponse->statusLine().statusCode() == http::StatusCode::Forbidden) - LOG_ERR("Access denied to [" << uriAnonym << ']'); - else - LOG_ERR("Invalid URI or access denied to [" << uriAnonym << ']'); - return; - } const std::string& body = httpResponse->getBody(); Poco::JSON::Object::Ptr settings; @@ -1683,10 +1691,61 @@ void DocumentBroker::asyncSendBrowserSetting(const std::shared_ptrsetSentBrowserSetting(true); }; - httpSession->setFinishedHandler(std::move(finishedCallback)); - httpSession->asyncRequest(request, *_poll); + if (async) + { + http::Session::FinishedCallback finishedCallback = + [uriAnonym, session, requestType, + parseResponse](const std::shared_ptr& configSession) + { + if (SigUtil::getShutdownRequestFlag()) + { + LOG_DBG("Shutdown flagged, giving up on in-flight requests"); + return; + } + + const std::shared_ptr httpResponse = configSession->response(); + LOG_TRC("DocumentBroker::sendBrowserSetting with " + << requestType << " returned " << httpResponse->statusLine().statusCode()); + + const bool failed = (httpResponse->statusLine().statusCode() != http::StatusCode::OK); + if (failed) + { + if (httpResponse->statusLine().statusCode() == http::StatusCode::Forbidden) + LOG_ERR("Access denied to [" << uriAnonym << ']'); + else + LOG_ERR("Invalid URI or access denied to [" << uriAnonym << ']'); + return; + } + + parseResponse(httpResponse); + }; + + httpSession->setFinishedHandler(std::move(finishedCallback)); + httpSession->asyncRequest(request, *_poll); + } + else + { + const std::shared_ptr httpResponse = + httpSession->syncRequest(request, *_poll); + + LOG_TRC("DocumentBroker::sendBrowserSetting with " + << requestType << " returned " << httpResponse->statusLine().statusCode()); + + const bool failed = (httpResponse->statusLine().statusCode() != http::StatusCode::OK); + if (failed) + { + if (httpResponse->statusLine().statusCode() == http::StatusCode::Forbidden) + LOG_ERR("Access denied to [" << uriAnonym << ']'); + else + LOG_ERR("Invalid URI or access denied to [" << uriAnonym << ']'); + return; + } + + parseResponse(httpResponse); + } } struct PresetRequest diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index 7ecef6f7b333..c2b373ef9370 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -548,8 +548,8 @@ class DocumentBroker : public std::enable_shared_from_this const std::string& userSettingsUri, const std::string& presetsPath); - void asyncSendBrowserSetting(const std::shared_ptr& session, - const std::string& browserSettingUri); + void sendBrowserSetting(const std::shared_ptr& session, + const std::string& settingUri, bool async = true); /// Start an asynchronous Installation of the user presets, e.g. autotexts etc, as /// described at userSettingsUri for installation into presetsPath