Makefile.am | 1 common/Authorization.cpp | 2 common/Crypto.cpp | 8 common/FileUtil.cpp | 4 common/JsonUtil.hpp | 4 common/LOOLWebSocket.hpp | 6 common/Log.cpp | 4 common/Message.hpp | 8 common/MessageQueue.cpp | 29 - common/Protocol.cpp | 6 common/Protocol.hpp | 88 --- common/Rectangle.hpp | 1 common/Seccomp.cpp | 12 common/Session.cpp | 6 common/SigUtil.cpp | 2 common/StringVector.cpp | 90 --- common/StringVector.hpp | 89 ++- common/Util.cpp | 8 common/Util.hpp | 85 +++ fuzzer/ClientSession.cpp | 7 kit/ChildSession.cpp | 31 - kit/Delta.hpp | 6 kit/ForKit.cpp | 14 kit/Kit.cpp | 58 +- kit/KitHelper.hpp | 4 loleaflet/js/global.js | 11 loleaflet/src/control/Control.JSDialogBuilder.js | 10 loleaflet/src/control/Ruler.js | 97 ++- net/DelaySocket.cpp | 28 - net/FakeSocket.cpp | 24 net/ServerSocket.hpp | 2 net/Socket.cpp | 74 +-- net/Socket.hpp | 44 - net/Ssl.cpp | 6 net/WebSocketHandler.hpp | 48 - net/clientnb.cpp | 10 test/DeltaTests.cpp | 12 test/Makefile.am | 1 test/TileCacheTests.cpp | 24 test/UnitAdmin.cpp | 18 test/UnitBadDocLoad.cpp | 2 test/UnitClient.cpp | 2 test/UnitClose.cpp | 2 test/UnitConvert.cpp | 2 test/UnitCopyPaste.cpp | 18 test/UnitCursor.cpp | 2 test/UnitHTTP.cpp | 7 test/UnitInsertDelete.cpp | 6 test/UnitLoad.cpp | 2 test/UnitPasswordProtected.cpp | 4 test/UnitRenderingOptions.cpp | 2 test/UnitSession.cpp | 4 test/UnitTyping.cpp | 12 test/UnitUNOCommand.cpp | 2 test/UnitWOPITemplate.cpp | 4 test/UnitWOPIWatermark.cpp | 2 test/WhiteBoxTests.cpp | 558 ++++++++++++++++++++++- test/WopiTestServer.hpp | 2 test/countloolkits.hpp | 2 test/fakesockettest.cpp | 2 test/helpers.hpp | 14 test/lokassert.hpp | 2 tools/Config.cpp | 10 tools/Connect.cpp | 4 tools/KitClient.cpp | 4 tools/Replay.hpp | 4 tools/Tool.cpp | 6 tools/WebSocketDump.cpp | 12 tools/map.cpp | 2 wsd/Admin.cpp | 31 - wsd/Admin.hpp | 3 wsd/AdminModel.cpp | 65 +- wsd/AdminModel.hpp | 15 wsd/Auth.cpp | 2 wsd/ClientSession.cpp | 26 - wsd/ClientSession.hpp | 2 wsd/DocumentBroker.cpp | 60 +- wsd/FileServer.cpp | 20 wsd/LOOLWSD.cpp | 169 +++--- wsd/LOOLWSD.hpp | 7 wsd/ProxyProtocol.cpp | 27 - wsd/ProxyProtocol.hpp | 13 wsd/QueueHandler.hpp | 70 -- wsd/RequestDetails.cpp | 165 +++++- wsd/RequestDetails.hpp | 151 +++++- wsd/SenderQueue.hpp | 6 wsd/ServerURL.hpp | 10 wsd/Storage.cpp | 35 - wsd/TileCache.cpp | 12 wsd/TileDesc.hpp | 20 wsd/TraceFile.hpp | 2 91 files changed, 1685 insertions(+), 903 deletions(-)
New commits: commit cc428e99a7aca82d67f97c5efd36ab4d631e9730 Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Wed Jun 3 17:14:03 2020 +0100 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Jun 8 22:19:53 2020 +0200 Proxy: dump ProxyProtocolHandler state separately Somewhat inelegant - nasty extra header & dynamic cast. Change-Id: Id18b2f7831ece3b971290e799c5df182429aa2a0 Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95448 Tested-by: Jenkins Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Michael Meeks <michael.me...@collabora.com> diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index d13a5d1a3..55e921295 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -32,6 +32,7 @@ #include "SenderQueue.hpp" #include "Storage.hpp" #include "TileCache.hpp" +#include "ProxyProtocol.hpp" #include <common/Log.hpp> #include <common/Message.hpp> #include <common/Clipboard.hpp> @@ -216,7 +217,7 @@ void DocumentBroker::setupPriorities() int prio = LOOLWSD::getConfigValue<int>("per_document.batch_priority", 5); Util::setProcessAndThreadPriorities(_childProcess->getPid(), prio); } -#endif // !MOBILE +#endif } void DocumentBroker::startThread() @@ -2420,6 +2421,17 @@ void DocumentBroker::dumpState(std::ostream& os) _tileCache->dumpState(os); _poll->dumpState(os); + +#if !MOBILEAPP + // Bit nasty - need a cleaner way to dump state. + for (auto &it : _sessions) + { + auto proto = it.second->getProtocol(); + auto proxy = dynamic_cast<ProxyProtocolHandler *>(proto.get()); + if (proxy) + proxy->dumpProxyState(os); + } +#endif } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/wsd/ProxyProtocol.cpp b/wsd/ProxyProtocol.cpp index 4b6950a10..8e7b8a631 100644 --- a/wsd/ProxyProtocol.cpp +++ b/wsd/ProxyProtocol.cpp @@ -277,7 +277,7 @@ void ProxyProtocolHandler::getIOStats(uint64_t &sent, uint64_t &recv) sent = recv = 0; } -void ProxyProtocolHandler::dumpState(std::ostream& os) +void ProxyProtocolHandler::dumpProxyState(std::ostream& os) { os << "proxy protocol sockets: " << _outSockets.size() << " writeQueue: " << _writeQueue.size() << ":\n"; os << '\t'; diff --git a/wsd/ProxyProtocol.hpp b/wsd/ProxyProtocol.hpp index 7a342912b..d20377b30 100644 --- a/wsd/ProxyProtocol.hpp +++ b/wsd/ProxyProtocol.hpp @@ -57,7 +57,10 @@ public: int sendBinaryMessage(const char *data, const size_t len, bool flush = false) const override; void shutdown(bool goingAway = false, const std::string &statusMessage = "") override; void getIOStats(uint64_t &sent, uint64_t &recv) override; - void dumpState(std::ostream& os) override; + // don't duplicate ourselves for every socket + void dumpState(std::ostream&) override {} + // instead do it centrally. + void dumpProxyState(std::ostream& os); bool parseEmitIncoming(const std::shared_ptr<StreamSocket> &socket); void handleRequest(bool isWaiting, const std::shared_ptr<Socket> &socket); commit 35848702e8c5e74c950ed959d55311c68d4d1abb Author: Gabriel Masei <gabriel.ma...@1and1.ro> AuthorDate: Fri May 29 14:32:04 2020 +0300 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Jun 8 22:19:53 2020 +0200 admin: notify subscribers that doc memory has changed Change-Id: I139c7d49a2cd1b86c3a281613f5628f6af8b3365 Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95133 Tested-by: Jenkins Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Michael Meeks <michael.me...@collabora.com> diff --git a/wsd/Admin.cpp b/wsd/Admin.cpp index 8d85dfbc7..78956b222 100644 --- a/wsd/Admin.cpp +++ b/wsd/Admin.cpp @@ -443,6 +443,8 @@ void Admin::pollingThread() std::chrono::duration_cast<std::chrono::milliseconds>(now - lastMem).count(); if (memWait <= MinStatsIntervalMs / 2) // Close enough { + _model.UpdateMemoryDirty(); + const size_t totalMem = getTotalMemoryUsage(); _model.addMemStats(totalMem); @@ -454,6 +456,8 @@ void Admin::pollingThread() _lastTotalMemory = totalMem; } + notifyDocsMemDirtyChanged(); + memWait += _memStatsTaskIntervalMs; lastMem = now; } @@ -764,6 +768,11 @@ void Admin::triggerMemoryCleanup(const size_t totalMem) } } +void Admin::notifyDocsMemDirtyChanged() +{ + _model.notifyDocsMemDirtyChanged(); +} + void Admin::dumpState(std::ostream& os) { // FIXME: be more helpful ... diff --git a/wsd/Admin.hpp b/wsd/Admin.hpp index 42e7a247a..5a3feee61 100644 --- a/wsd/Admin.hpp +++ b/wsd/Admin.hpp @@ -148,6 +148,7 @@ private: /// Memory consumption has increased, start killing kits etc. till memory consumption gets back /// under @hardModeLimit void triggerMemoryCleanup(size_t hardModeLimit); + void notifyDocsMemDirtyChanged(); /// Round the interval up to multiples of MinStatsIntervalMs. /// This is to avoid arbitrarily small intervals that hammer the server. diff --git a/wsd/AdminModel.cpp b/wsd/AdminModel.cpp index ab2d81df7..387796452 100644 --- a/wsd/AdminModel.cpp +++ b/wsd/AdminModel.cpp @@ -134,16 +134,18 @@ std::string Document::to_string() const return oss.str(); } -int Document::getMemoryDirty() const +void Document::updateMemoryDirty() { // Avoid accessing smaps too often const time_t now = std::time(nullptr); if (now - _lastTimeSMapsRead >= 5) { + int lastMemDirty = _memoryDirty; _memoryDirty = _procSMaps ? Util::getPssAndDirtyFromSMaps(_procSMaps).second : 0; _lastTimeSMapsRead = now; + if (lastMemDirty != _memoryDirty) + _hasMemDirtyChanged = true; } - return _memoryDirty; } bool Subscriber::notify(const std::string& message) @@ -1041,4 +1043,25 @@ std::set<pid_t> AdminModel::getDocumentPids() const return pids; } +void AdminModel::UpdateMemoryDirty() +{ + for (const auto& it: _documents) + { + it.second->updateMemoryDirty(); + } +} + +void AdminModel::notifyDocsMemDirtyChanged() +{ + for (const auto& it: _documents) + { + int memoryDirty = it.second->getMemoryDirty(); + if (it.second->hasMemDirtyChanged()) + { + notify("propchange " + std::to_string(it.second->getPid()) + " mem " + std::to_string(memoryDirty)); + it.second->setMemDirtyChanged(false); + } + } +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/wsd/AdminModel.hpp b/wsd/AdminModel.hpp index ae88ef8cd..98bd01495 100644 --- a/wsd/AdminModel.hpp +++ b/wsd/AdminModel.hpp @@ -121,7 +121,8 @@ public: _wopiUploadDuration(0), _procSMaps(nullptr), _lastTimeSMapsRead(0), - _isModified(false) + _isModified(false), + _hasMemDirtyChanged(true) { } @@ -155,7 +156,8 @@ public: const std::map<std::string, View>& getViews() const { return _views; } void updateLastActivityTime() { _lastActivity = std::time(nullptr); } - int getMemoryDirty() const; + void updateMemoryDirty(); + int getMemoryDirty() const { return _memoryDirty; } std::pair<std::time_t, std::string> getSnapshot() const; const std::string getHistory() const; @@ -182,6 +184,8 @@ public: void setWopiUploadDuration(const std::chrono::milliseconds wopiUploadDuration) { _wopiUploadDuration = wopiUploadDuration; } std::chrono::milliseconds getWopiUploadDuration() const { return _wopiUploadDuration; } void setProcSMapsFD(const int smapsFD) { _procSMaps = fdopen(smapsFD, "r"); } + bool hasMemDirtyChanged() const { return _hasMemDirtyChanged; } + void setMemDirtyChanged(bool changeStatus) { _hasMemDirtyChanged = changeStatus; } std::string to_string() const; @@ -195,7 +199,7 @@ private: /// Hosted filename std::string _filename; /// The dirty (ie. un-shared) memory of the document's Kit process. - mutable int _memoryDirty; + int _memoryDirty; /// Last noted Jiffy count unsigned _lastJiffy; @@ -212,11 +216,12 @@ private: std::chrono::milliseconds _wopiUploadDuration; FILE* _procSMaps; - mutable std::time_t _lastTimeSMapsRead; + std::time_t _lastTimeSMapsRead; /// Per-doc kit process settings. DocProcSettings _docProcSettings; bool _isModified; + bool _hasMemDirtyChanged; }; /// An Admin session subscriber. @@ -337,6 +342,8 @@ public: void getMetrics(std::ostringstream &oss); std::set<pid_t> getDocumentPids() const; + void UpdateMemoryDirty(); + void notifyDocsMemDirtyChanged(); static int getPidsFromProcName(const std::regex& procNameRegEx, std::vector<int> *pids); commit 7ab56d396b3fc6247e1bd56898596ed445c0cad8 Author: Tomaž Vajngerl <tomaz.vajng...@collabora.co.uk> AuthorDate: Thu Jun 4 12:13:17 2020 +0200 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Jun 8 22:19:53 2020 +0200 ruler: delete tabstop on long-press when over a specific tabstop When a long-press event occurs over a tabstop, it will now delete it. If no specific tabstop is at the long-press position, then insert a new tabstop. Change-Id: I3af2847db3367c1f76d28696f4fa3d0a8017011c diff --git a/loleaflet/src/control/Ruler.js b/loleaflet/src/control/Ruler.js index 040ec3442..cca4e46ed 100644 --- a/loleaflet/src/control/Ruler.js +++ b/loleaflet/src/control/Ruler.js @@ -300,9 +300,7 @@ L.Control.Ruler = L.Control.extend({ // least in the US. (The ruler unit to use doesn't seem to be stored in the document // at least for .odt?) for (var num = 0; num <= (this.options.pageWidth / 1000) + 1; num++) { - var marker = L.DomUtil.create('div', 'loleaflet-ruler-maj', this._rBPContainer); - // The - 1 is to compensate for the left and right .5px borders of // loleaflet-ruler-maj in leaflet.css. marker.style.width = this.options.DraggableConvertRatio*1000 - 1 + 'px'; @@ -355,12 +353,9 @@ L.Control.Ruler = L.Control.extend({ } if (!this.options.marginSet) { - this.options.marginSet = true; - this._lMarginMarker = L.DomUtil.create('div', 'loleaflet-ruler-margin loleaflet-ruler-left', this._rFace); this._rMarginMarker = L.DomUtil.create('div', 'loleaflet-ruler-margin loleaflet-ruler-right', this._rFace); - this._lMarginDrag = L.DomUtil.create('div', 'loleaflet-ruler-drag loleaflet-ruler-left', this._rMarginWrapper); this._lToolTip = L.DomUtil.create('div', 'loleaflet-ruler-ltooltip', this._lMarginDrag); this._rMarginDrag = L.DomUtil.create('div', 'loleaflet-ruler-drag loleaflet-ruler-right', this._rMarginWrapper); @@ -372,7 +367,6 @@ L.Control.Ruler = L.Control.extend({ this.options.interactive = true; L.DomEvent.on(this._rMarginDrag, 'touchstart', this._initiateDrag, this); L.DomEvent.on(this._lMarginDrag, 'touchstart', this._initiateDrag, this); - } } @@ -630,12 +624,36 @@ L.Control.Ruler = L.Control.extend({ } return tabstop; }, + + _showTabstopContextMenu: function(position, tabstopNumber) { + var self = this; + this.currentPositionInTwips = position; + this.currentTabStopIndex = tabstopNumber; + $.contextMenu({ + selector: '.loleaflet-ruler-tabstopcontainer', + className: 'loleaflet-font', + items: { + inserttabstop: { + name: _('Insert tabstop'), + callback: (this._insertTabstop).bind(this), + visible: function() { + return self.currentPositionInTwips != null; + } + }, + removetabstop: { + name: _('Delete tabstop'), + callback: (this._deleteTabstop).bind(this), + visible: function() { + return self.currentTabStopIndex != null; + } + } + } + }); + }, + _initiateTabstopDrag: function(event) { // console.log('===> _initiateTabstopDrag ' + event.type); - this.currentPositionInTwips = null; - this.currentTabStopIndex = null; - var tabstopContainer = null; var pointX = null; @@ -657,32 +675,12 @@ L.Control.Ruler = L.Control.extend({ // right-click inside tabstop container if (event.button === 2) { if (tabstop == null) { - this.currentPositionInTwips = this._map._docLayer._pixelsToTwips({x: pointX, y:0}).x; + var position = this._map._docLayer._pixelsToTwips({x: pointX, y:0}).x; + this._showTabstopContextMenu(position, null); } else { - this.currentTabStopIndex = tabstop.tabStopNumber; + this._showTabstopContextMenu(null, tabstop.tabStopNumber); } - var self = this; - $.contextMenu({ - selector: '.loleaflet-ruler-tabstopcontainer', - className: 'loleaflet-font', - items: { - inserttabstop: { - name: _('Insert tabstop'), - callback: (this._insertTabstop).bind(this), - visible: function() { - return self.currentPositionInTwips != null; - } - }, - removetabstop: { - name: _('Delete tabstop'), - callback: (this._deleteTabstop).bind(this), - visible: function() { - return self.currentTabStopIndex != null; - } - } - } - }); event.stopPropagation(); return; } @@ -778,22 +776,30 @@ L.Control.Ruler = L.Control.extend({ }, _onTabstopContainerLongPress: function(event) { - var pointX = event.center.x - event.target.getBoundingClientRect().left; - this.currentPositionInTwips = this._map._docLayer._pixelsToTwips({x: pointX, y:0}).x; + var tabstopContainer = event.target; + var pointX = event.center.x - tabstopContainer.getBoundingClientRect().left; + var pointXTwip = this._map._docLayer._pixelsToTwips({x: pointX, y:0}).x; + var tabstop = this._getTabStopHit(tabstopContainer, pointX); + if (window.mode.isMobile() || window.mode.isTablet()) { - this._insertTabstop(); + if (tabstop == null) { + this.currentPositionInTwips = pointXTwip; + this.currentTabStopIndex = null; + this._insertTabstop(); + } + else { + this.currentPositionInTwips = null; + this.currentTabStopIndex = tabstop.tabStopNumber; + this._deleteTabstop(); + } } else { - $.contextMenu({ - selector: '.loleaflet-ruler-tabstopcontainer', - className: 'loleaflet-font', - items: { - inserttabstop: { - name: _('Insert tabstop'), - callback: (this._insertTabstop).bind(this) - } - } - }); + var tabstopNumber = null; + if (tabstop != null) { + tabstopNumber = tabstop.tabstopNumber; + pointXTwip = null; + } + this._showTabstopContextMenu(pointXTwip, tabstopNumber); } }, @@ -841,7 +847,6 @@ L.Control.Ruler = L.Control.extend({ }); - L.control.ruler = function (options) { return new L.Control.Ruler(options); }; commit b87203d88eb73421fa9791b071458dfe812b4748 Author: Pranam Lashkari <lpra...@collabora.com> AuthorDate: Wed Jun 3 16:45:24 2020 +0530 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Jun 8 22:19:53 2020 +0200 leaflet: Can't apply text color or highlight color on text shape (impress, mobile) selecting a shape and trying to change the char color or hightlight won't work Change-Id: Ie48cdb6e276df7260c945d519d51244a32e59c28 Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95409 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Andras Timar <andras.ti...@collabora.com> diff --git a/loleaflet/src/control/Control.JSDialogBuilder.js b/loleaflet/src/control/Control.JSDialogBuilder.js index 71bf67726..debd6739a 100644 --- a/loleaflet/src/control/Control.JSDialogBuilder.js +++ b/loleaflet/src/control/Control.JSDialogBuilder.js @@ -1729,6 +1729,16 @@ L.Control.JSDialogBuilder = L.Control.extend({ gradientItem.endcolor = color; builder.map.sendUnoCommand('.uno:FillPageGradient?FillPageGradientJSON:string=' + JSON.stringify(gradientItem)); return; + } else if (data.id === 'Color' || data.id === 'CharBackColor') { + var params = {}; + params[data.id] = { + type : 'long', + value : parseInt('0x' + color) + }; + + builder.map['stateChangeHandler'].setItemValue(data.command, params[data.id].value); + builder.map.sendUnoCommand(data.command, params); + return; } var command = data.command + '?Color:string=' + color; commit 764374ba956d9b6852b51358187806a19006511f Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Wed Jun 3 17:13:40 2020 +0100 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Jun 8 22:19:53 2020 +0200 dumpState should dump much more loolwsd state. Change-Id: I0e59d56b2b735aea013a59850ff3f37fd72bc8b9 Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95447 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Michael Meeks <michael.me...@collabora.com> diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index c672932f0..39365cbe7 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -734,6 +734,8 @@ std::string LOOLWSD::OverrideWatermark; std::set<const Poco::Util::AbstractConfiguration*> LOOLWSD::PluginConfigurations; std::chrono::time_point<std::chrono::system_clock> LOOLWSD::StartTime; +// If you add global state please update dumpState below too + static std::string UnitTestLibrary; unsigned int LOOLWSD::NumPreSpawnedChildren = 0; @@ -3402,12 +3404,37 @@ public: << "\n SSL-Termination: " << (LOOLWSD::isSSLTermination() ? "yes" : "no") << "\n Security " << (LOOLWSD::NoCapsForKit ? "no" : "") << " chroot, " << (LOOLWSD::NoSeccomp ? "no" : "") << " api lockdown" + << "\n Admin: " << (LOOLWSD::AdminEnabled ? "enabled" : "disabled") #endif << "\n TerminationFlag: " << SigUtil::getTerminationFlag() << "\n isShuttingDown: " << SigUtil::getShutdownRequestFlag() << "\n NewChildren: " << NewChildren.size() << "\n OutstandingForks: " << OutstandingForks - << "\n NumPreSpawnedChildren: " << LOOLWSD::NumPreSpawnedChildren; + << "\n NumPreSpawnedChildren: " << LOOLWSD::NumPreSpawnedChildren + << "\n ChildSpawnTimeoutMs: " << ChildSpawnTimeoutMs + << "\n Document Brokers: " << DocBrokers.size() +#if !MOBILEAPP + << "\n of which ConvertTo: " << ConvertToBroker::getInstanceCount() +#endif + << "\n vs. MaxDocuments: " << LOOLWSD::MaxDocuments + << "\n NumConnections: " << LOOLWSD::NumConnections + << "\n vs. MaxConnections: " << LOOLWSD::MaxConnections + << "\n SysTemplate: " << LOOLWSD::SysTemplate + << "\n LoTemplate: " << LOOLWSD::LoTemplate + << "\n ChildRoot: " << LOOLWSD::ChildRoot + << "\n FileServerRoot: " << LOOLWSD::FileServerRoot + << "\n WelcomeFilesRoot: " << LOOLWSD::WelcomeFilesRoot + << "\n ServiceRoot: " << LOOLWSD::ServiceRoot + << "\n LOKitVersion: " << LOOLWSD::LOKitVersion + << "\n HostIdentifier: " << LOOLWSD::HostIdentifier + << "\n ConfigFile: " << LOOLWSD::ConfigFile + << "\n ConfigDir: " << LOOLWSD::ConfigDir + << "\n LogLevel: " << LOOLWSD::LogLevel + << "\n AnonymizeUserData: " << (LOOLWSD::AnonymizeUserData ? "yes" : "no") + << "\n CheckLoolUser: " << (LOOLWSD::CheckLoolUser ? "yes" : "no") + << "\n IsProxyPrefixEnabled: " << (LOOLWSD::IsProxyPrefixEnabled ? "yes" : "no") + << "\n OverrideWatermark: " << LOOLWSD::OverrideWatermark + ; os << "\nServer poll:\n"; _acceptPoll.dumpState(os); commit d55b8be08e268df298961418815b8ac6f48732be Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Wed Jun 3 16:06:10 2020 +0100 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Jun 8 22:19:53 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 <jenkinscollaboraoff...@gmail.com> Reviewed-by: Michael Meeks <michael.me...@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) { commit 89a8713b694a1b8b886ec374e67804d8752889da Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Wed Jun 3 16:06:52 2020 +0200 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Jun 8 22:19:53 2020 +0200 libfuzzer: fix build Also decrease the poll timeout to 0, otherwise testing each input would now take 5 sec, rather than ~3 ms. Change-Id: I1a4f347e5ec08a62d40131bfec3c504a19727323 Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95437 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Miklos Vajna <vmik...@collabora.com> diff --git a/fuzzer/ClientSession.cpp b/fuzzer/ClientSession.cpp index 06048e28d..31f8d07f8 100644 --- a/fuzzer/ClientSession.cpp +++ b/fuzzer/ClientSession.cpp @@ -22,7 +22,10 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) std::shared_ptr<ProtocolHandlerInterface> ws; std::string id; bool isReadOnly = false; - const RequestDetails requestDetails("fuzzer", LOOLWSD::ServiceRoot); + Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, uri, + Poco::Net::HTTPMessage::HTTP_1_1); + request.setHost("localhost:9980"); + const RequestDetails requestDetails(request, ""); auto session = std::make_shared<ClientSession>(ws, id, docBroker, uriPublic, isReadOnly, requestDetails); @@ -36,7 +39,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) } // Make sure SocketPoll::_newCallbacks does not grow forever, leading to OOM. - Admin::instance().poll(SocketPoll::DefaultPollTimeoutMicroS); + Admin::instance().poll(0); return 0; } commit 131cf8cdf50ad73d5182a6f12456d64796016c64 Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> AuthorDate: Mon Jun 1 08:18:13 2020 -0400 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Jun 8 22:19:53 2020 +0200 wsd: leaflet: fix reuse_cookies support reuse_cookies is now always encoded in the URL. And, there is no need for the WOPISrc in the three cases in this patch, and by passing the DocumentURI proper (without /ws?WOPISrc=...) ensures that all query-params in the DocumentURI are properly processed. This fixes the reuse_cookies regression where it wasn't passed to WOPI requests. Change-Id: I8dccfb09a7b4102d10c1aef24f43b699a07bfed8 Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95293 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Ashod Nakashian <ashnak...@gmail.com> diff --git a/loleaflet/js/global.js b/loleaflet/js/global.js index 20757e2a5..1c951b089 100644 --- a/loleaflet/js/global.js +++ b/loleaflet/js/global.js @@ -653,9 +653,16 @@ else if (global.accessHeader !== '') { wopiParams = { 'access_header': global.accessHeader }; } - else if (global.reuseCookies !== '') { - wopiParams = { 'reuse_cookies': global.reuseCookies }; + + if (global.reuseCookies !== '') { + if (wopiParams) { + wopiParams['reuse_cookies'] = global.reuseCookies; + } + else { + wopiParams = { 'reuse_cookies': global.reuseCookies }; + } } + if (wopiParams) { docParams = Object.keys(wopiParams).map(function(key) { return encodeURIComponent(key) + '=' + encodeURIComponent(wopiParams[key]); diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 51bd22e38..db7bfbaef 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -2810,7 +2810,7 @@ private: const std::string formName(form.get("name")); // Validate the docKey - const std::string decodedUri = requestDetails.getLegacyDocumentURI(); + const std::string decodedUri = requestDetails.getDocumentURI(); const std::string docKey = DocumentBroker::getDocKey(DocumentBroker::sanitizeURI(decodedUri)); std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex); @@ -2846,7 +2846,7 @@ private: // TODO: Check that the user in question has access to this file! // 1. Validate the dockey - const std::string decodedUri = requestDetails.getLegacyDocumentURI(); + const std::string decodedUri = requestDetails.getDocumentURI(); const std::string docKey = DocumentBroker::getDocKey(DocumentBroker::sanitizeURI(decodedUri)); std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex); @@ -3025,7 +3025,7 @@ private: SocketDisposition& disposition, const std::shared_ptr<StreamSocket>& socket) { - const std::string url = requestDetails.getLegacyDocumentURI(); + const std::string url = requestDetails.getDocumentURI(); assert(socket && "Must have a valid socket"); // must be trace for anonymization commit add164bf9d4498a3ec9eaf53a4b5bb1745735d03 Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> AuthorDate: Mon May 25 10:52:37 2020 -0400 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Jun 8 22:19:53 2020 +0200 wsd: improved RequestDetails parsing and documentation ...with support for properly extracting the different fields with unit-test. URIs are quite complex and varied. For historic reasons they have all been treated without distinction, which makes support for all variants difficult. RequestDetails encapsulates this complexity, and now it is almost completely documented both descriptively and functionally (via extensive unit-tests). Parsing of the URIs is now more structured by having named fields instead of relying on knowing which token should contain which field, which is error-prone and very opaque. Change-Id: I68d07c2e00baf43f0ade97d20f62691ffb3bf576 Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95292 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Ashod Nakashian <ashnak...@gmail.com> diff --git a/test/WhiteBoxTests.cpp b/test/WhiteBoxTests.cpp index e8290cd2f..bb2f57e4c 100644 --- a/test/WhiteBoxTests.cpp +++ b/test/WhiteBoxTests.cpp @@ -879,8 +879,12 @@ void WhiteBoxTests::testRequestDetails_DownloadURI() RequestDetails details(request, ""); + // LOK_ASSERT_EQUAL(URI, details.getDocumentURI()); + LOK_ASSERT_EQUAL(static_cast<std::size_t>(5), details.size()); LOK_ASSERT_EQUAL(std::string("loleaflet"), details[0]); + LOK_ASSERT_EQUAL(std::string("loleaflet"), details.getField(RequestDetails::Field::Type)); + LOK_ASSERT(details.equals(RequestDetails::Field::Type, "loleaflet")); LOK_ASSERT(details.equals(0, "loleaflet")); LOK_ASSERT_EQUAL(std::string("49c225146"), details[1]); LOK_ASSERT_EQUAL(std::string("src"), details[2]); @@ -897,8 +901,12 @@ void WhiteBoxTests::testRequestDetails_DownloadURI() RequestDetails details(request, ""); + // LOK_ASSERT_EQUAL(URI, details.getDocumentURI()); + LOK_ASSERT_EQUAL(static_cast<std::size_t>(3), details.size()); LOK_ASSERT_EQUAL(std::string("loleaflet"), details[0]); + LOK_ASSERT_EQUAL(std::string("loleaflet"), details.getField(RequestDetails::Field::Type)); + LOK_ASSERT(details.equals(RequestDetails::Field::Type, "loleaflet")); LOK_ASSERT(details.equals(0, "loleaflet")); LOK_ASSERT_EQUAL(std::string("49c225146"), details[1]); LOK_ASSERT_EQUAL(std::string("select2.css"), details[2]); @@ -921,8 +929,15 @@ void WhiteBoxTests::testRequestDetails_loleafletURI() RequestDetails details(request, ""); + const std::string wopiSrc + = "http://localhost/nextcloud/index.php/apps/richdocuments/wopi/files/593_ocqiesh0cngs"; + + LOK_ASSERT_EQUAL(wopiSrc, details.getField(RequestDetails::Field::WOPISrc)); + LOK_ASSERT_EQUAL(static_cast<std::size_t>(4), details.size()); LOK_ASSERT_EQUAL(std::string("loleaflet"), details[0]); + LOK_ASSERT_EQUAL(std::string("loleaflet"), details.getField(RequestDetails::Field::Type)); + LOK_ASSERT(details.equals(RequestDetails::Field::Type, "loleaflet")); LOK_ASSERT(details.equals(0, "loleaflet")); LOK_ASSERT_EQUAL(std::string("49c225146"), details[1]); LOK_ASSERT_EQUAL(std::string("loleaflet.html"), details[2]); @@ -960,6 +975,7 @@ void WhiteBoxTests::testRequestDetails_local() const std::string docUri = "file:///home/ash/prj/lo/online/test/data/hello-world.odt"; + LOK_ASSERT_EQUAL(docUri, details.getLegacyDocumentURI()); LOK_ASSERT_EQUAL(docUri, details.getDocumentURI()); LOK_ASSERT_EQUAL(static_cast<std::size_t>(6), details.size()); @@ -973,9 +989,19 @@ void WhiteBoxTests::testRequestDetails_local() LOK_ASSERT_EQUAL(std::string("open"), details[3]); LOK_ASSERT_EQUAL(std::string("open"), details[4]); LOK_ASSERT_EQUAL(std::string("0"), details[5]); + + LOK_ASSERT_EQUAL(std::string("lool"), details.getField(RequestDetails::Field::Type)); + LOK_ASSERT(details.equals(RequestDetails::Field::Type, "lool")); + LOK_ASSERT_EQUAL(std::string("open"), details.getField(RequestDetails::Field::SessionId)); + LOK_ASSERT(details.equals(RequestDetails::Field::SessionId, "open")); + LOK_ASSERT_EQUAL(std::string("open"), details.getField(RequestDetails::Field::Command)); + LOK_ASSERT(details.equals(RequestDetails::Field::Command, "open")); + LOK_ASSERT_EQUAL(std::string("0"), details.getField(RequestDetails::Field::Serial)); + LOK_ASSERT(details.equals(RequestDetails::Field::Serial, "0")); } { + // Blank entries are skipped. static const std::string URI = "/lool/" "file%3A%2F%2F%2Fhome%2Fash%2Fprj%2Flo%2Fonline%2Ftest%" "2Fdata%2Fhello-world.odt/ws//write/2"; @@ -1006,8 +1032,62 @@ void WhiteBoxTests::testRequestDetails_local() "file%3A%2F%2F%2Fhome%2Fash%2Fprj%2Flo%2Fonline%2Ftest%2Fdata%2Fhello-world.odt"), details[1]); LOK_ASSERT_EQUAL(std::string("ws"), details[2]); - LOK_ASSERT_EQUAL(std::string("write"), details[3]); - LOK_ASSERT_EQUAL(std::string("2"), details[4]); + LOK_ASSERT_EQUAL(std::string("write"), details[3]); // SessionId, since the real SessionId is blank. + LOK_ASSERT_EQUAL(std::string("2"), details[4]); // Command, since SessionId was blank. + + LOK_ASSERT_EQUAL(std::string("lool"), details.getField(RequestDetails::Field::Type)); + LOK_ASSERT(details.equals(RequestDetails::Field::Type, "lool")); + LOK_ASSERT_EQUAL(std::string("write"), details.getField(RequestDetails::Field::SessionId)); + LOK_ASSERT(details.equals(RequestDetails::Field::SessionId, "write")); + LOK_ASSERT_EQUAL(std::string("2"), details.getField(RequestDetails::Field::Command)); + LOK_ASSERT(details.equals(RequestDetails::Field::Command, "2")); + LOK_ASSERT_EQUAL(std::string(""), details.getField(RequestDetails::Field::Serial)); + LOK_ASSERT(details.equals(RequestDetails::Field::Serial, "")); + } + + { + // Apparently, the initial / can be missing -- all the tests do that. + static const std::string URI = "lool/" + "file%3A%2F%2F%2Fhome%2Fash%2Fprj%2Flo%2Fonline%2Ftest%" + "2Fdata%2Fhello-world.odt/ws//write/2"; + + Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, URI, + Poco::Net::HTTPMessage::HTTP_1_1); + request.setHost(Root); + request.set("User-Agent", WOPI_AGENT_STRING); + request.set("ProxyPrefix", ProxyPrefix); + + RequestDetails details(request, ""); + LOK_ASSERT_EQUAL(true, details.isProxy()); + LOK_ASSERT_EQUAL(ProxyPrefix, details.getProxyPrefix()); + + LOK_ASSERT_EQUAL(Root, details.getHostUntrusted()); + LOK_ASSERT_EQUAL(false, details.isWebSocket()); + LOK_ASSERT_EQUAL(true, details.isGet()); + + const std::string docUri = "file:///home/ash/prj/lo/online/test/data/hello-world.odt"; + + LOK_ASSERT_EQUAL(docUri, details.getDocumentURI()); + + LOK_ASSERT_EQUAL(static_cast<std::size_t>(5), details.size()); + LOK_ASSERT_EQUAL(std::string("lool"), details[0]); + LOK_ASSERT(details.equals(0, "lool")); + LOK_ASSERT_EQUAL( + std::string( + "file%3A%2F%2F%2Fhome%2Fash%2Fprj%2Flo%2Fonline%2Ftest%2Fdata%2Fhello-world.odt"), + details[1]); + LOK_ASSERT_EQUAL(std::string("ws"), details[2]); + LOK_ASSERT_EQUAL(std::string("write"), details[3]); // SessionId, since the real SessionId is blank. + LOK_ASSERT_EQUAL(std::string("2"), details[4]); // Command, since SessionId was blank. + + LOK_ASSERT_EQUAL(std::string("lool"), details.getField(RequestDetails::Field::Type)); + LOK_ASSERT(details.equals(RequestDetails::Field::Type, "lool")); + LOK_ASSERT_EQUAL(std::string("write"), details.getField(RequestDetails::Field::SessionId)); + LOK_ASSERT(details.equals(RequestDetails::Field::SessionId, "write")); + LOK_ASSERT_EQUAL(std::string("2"), details.getField(RequestDetails::Field::Command)); + LOK_ASSERT(details.equals(RequestDetails::Field::Command, "2")); + LOK_ASSERT_EQUAL(std::string(""), details.getField(RequestDetails::Field::Serial)); + LOK_ASSERT(details.equals(RequestDetails::Field::Serial, "")); } } @@ -1051,7 +1131,11 @@ void WhiteBoxTests::testRequestDetails() LOK_ASSERT_EQUAL(false, details.isWebSocket()); LOK_ASSERT_EQUAL(true, details.isGet()); - const std::string docUri + LOK_ASSERT_EQUAL(std::string("b26112ab1b6f2ed98ce1329f0f344791"), details.getField(RequestDetails::Field::SessionId)); + LOK_ASSERT_EQUAL(std::string("close"), details.getField(RequestDetails::Field::Command)); + LOK_ASSERT_EQUAL(std::string("31"), details.getField(RequestDetails::Field::Serial)); + + const std::string docUri_WopiSrc = "http://localhost/nextcloud/index.php/apps/richdocuments/wopi/files/" "593_ocqiesh0cngs?access_token=MN0KXXDv9GJ1wCCLnQcjVQT2T7WrfYpA&access_token_ttl=0&" "reuse_" @@ -1064,10 +1148,31 @@ void WhiteBoxTests::testRequestDetails() "3DXCookieValue%3ASuperCookieName%3DBAZINGA/ws?WOPISrc=http://localhost/nextcloud/" "index.php/apps/richdocuments/wopi/files/593_ocqiesh0cngs&compat="; + LOK_ASSERT_EQUAL(docUri_WopiSrc, details.getLegacyDocumentURI()); + + const std::string docUri + = "http://localhost/nextcloud/index.php/apps/richdocuments/wopi/files/" + "593_ocqiesh0cngs?access_token=MN0KXXDv9GJ1wCCLnQcjVQT2T7WrfYpA&access_token_ttl=0&" + "reuse_" + "cookies=oc_sessionPassphrase%" + "3D8nFRqycbs7bP97yxCuJviBbVKdCXmuiXp6ZYH0DfUoy5UZDCTQgLwluvbgRbKrdKodJteG3uNE19KNUAoE" + "5typ" + "f4oBGwJdFY%252F5W9RNST8wEHWkUVIjZy7vmY0ZX38PlS%3Anc_sameSiteCookielax%3Dtrue%3Anc_" + "sameSiteCookiestrict%3Dtrue%3Aocqiesh0cngs%3Dr5ujg4tpvgu9paaf5bguiokgjl%" + "3AXCookieName%" + "3DXCookieValue%3ASuperCookieName%3DBAZINGA"; + LOK_ASSERT_EQUAL(docUri, details.getDocumentURI()); - LOK_ASSERT_EQUAL(static_cast<std::size_t>(6), details.size()); + const std::string wopiSrc + = "http://localhost/nextcloud/index.php/apps/richdocuments/wopi/files/593_ocqiesh0cngs"; + + LOK_ASSERT_EQUAL(wopiSrc, details.getField(RequestDetails::Field::WOPISrc)); + + LOK_ASSERT_EQUAL(static_cast<std::size_t>(8), details.size()); LOK_ASSERT_EQUAL(std::string("lool"), details[0]); + LOK_ASSERT_EQUAL(std::string("lool"), details.getField(RequestDetails::Field::Type)); + LOK_ASSERT(details.equals(RequestDetails::Field::Type, "lool")); LOK_ASSERT(details.equals(0, "lool")); LOK_ASSERT_EQUAL( std::string( @@ -1078,14 +1183,26 @@ void WhiteBoxTests::testRequestDetails() "19KNUAoE5typf4oBGwJdFY%25252F5W9RNST8wEHWkUVIjZy7vmY0ZX38PlS%253Anc_" "sameSiteCookielax%253Dtrue%253Anc_sameSiteCookiestrict%253Dtrue%" "253Aocqiesh0cngs%253Dr5ujg4tpvgu9paaf5bguiokgjl%253AXCookieName%" - "253DXCookieValue%253ASuperCookieName%253DBAZINGA/" - "ws?WOPISrc=http%3A%2F%2Flocalhost%2Fnextcloud%2Findex.php%2Fapps%" - "2Frichdocuments%2Fwopi%2Ffiles%2F593_ocqiesh0cngs&compat="), + "253DXCookieValue%253ASuperCookieName%253DBAZINGA"), details[1]); LOK_ASSERT_EQUAL(std::string("ws"), details[2]); - LOK_ASSERT_EQUAL(std::string("b26112ab1b6f2ed98ce1329f0f344791"), details[3]); - LOK_ASSERT_EQUAL(std::string("close"), details[4]); - LOK_ASSERT_EQUAL(std::string("31"), details[5]); + LOK_ASSERT_EQUAL( + std::string("WOPISrc=http%3A%2F%2Flocalhost%2Fnextcloud%2Findex.php%2Fapps%" + "2Frichdocuments%2Fwopi%2Ffiles%2F593_ocqiesh0cngs&compat="), + details[3]); + LOK_ASSERT_EQUAL(std::string("ws"), details[4]); + LOK_ASSERT_EQUAL(std::string("b26112ab1b6f2ed98ce1329f0f344791"), details[5]); + LOK_ASSERT_EQUAL(std::string("close"), details[6]); + LOK_ASSERT_EQUAL(std::string("31"), details[7]); + + LOK_ASSERT_EQUAL(std::string("lool"), details.getField(RequestDetails::Field::Type)); + LOK_ASSERT(details.equals(RequestDetails::Field::Type, "lool")); + LOK_ASSERT_EQUAL(std::string("b26112ab1b6f2ed98ce1329f0f344791"), details.getField(RequestDetails::Field::SessionId)); + LOK_ASSERT(details.equals(RequestDetails::Field::SessionId, "b26112ab1b6f2ed98ce1329f0f344791")); + LOK_ASSERT_EQUAL(std::string("close"), details.getField(RequestDetails::Field::Command)); + LOK_ASSERT(details.equals(RequestDetails::Field::Command, "close")); + LOK_ASSERT_EQUAL(std::string("31"), details.getField(RequestDetails::Field::Serial)); + LOK_ASSERT(details.equals(RequestDetails::Field::Serial, "31")); } { @@ -1113,30 +1230,139 @@ void WhiteBoxTests::testRequestDetails() LOK_ASSERT_EQUAL(false, details.isWebSocket()); LOK_ASSERT_EQUAL(true, details.isGet()); - const std::string docUri + const std::string docUri_WopiSrc = "http://localhost/owncloud/index.php/apps/richdocuments/wopi/files/" "165_ocgdpzbkm39u?access_token=ODhIXdJdbsVYQoKKCuaYofyzrovxD3MQ&access_token_ttl=0&" "reuse_cookies=XCookieName%3DXCookieValue%3ASuperCookieName%3DBAZINGA/" "ws?WOPISrc=http://localhost/owncloud/index.php/apps/richdocuments/wopi/files/" "165_ocgdpzbkm39u&compat="; + LOK_ASSERT_EQUAL(docUri_WopiSrc, details.getLegacyDocumentURI()); + + const std::string docUri + = "http://localhost/owncloud/index.php/apps/richdocuments/wopi/files/" + "165_ocgdpzbkm39u?access_token=ODhIXdJdbsVYQoKKCuaYofyzrovxD3MQ&access_token_ttl=0&" + "reuse_cookies=XCookieName%3DXCookieValue%3ASuperCookieName%3DBAZINGA"; + LOK_ASSERT_EQUAL(docUri, details.getDocumentURI()); - LOK_ASSERT_EQUAL(static_cast<std::size_t>(6), details.size()); + const std::string wopiSrc + = "http://localhost/owncloud/index.php/apps/richdocuments/wopi/files/" + "165_ocgdpzbkm39u"; + + LOK_ASSERT_EQUAL(wopiSrc, details.getField(RequestDetails::Field::WOPISrc)); + + LOK_ASSERT_EQUAL(static_cast<std::size_t>(8), details.size()); LOK_ASSERT_EQUAL(std::string("lool"), details[0]); LOK_ASSERT(details.equals(0, "lool")); LOK_ASSERT_EQUAL( std::string("http%3A%2F%2Flocalhost%2Fowncloud%2Findex.php%2Fapps%2Frichdocuments%" "2Fwopi%2Ffiles%2F165_ocgdpzbkm39u%3Faccess_token%" "3DODhIXdJdbsVYQoKKCuaYofyzrovxD3MQ%26access_token_ttl%3D0%26reuse_cookies%" - "3DXCookieName%253DXCookieValue%253ASuperCookieName%253DBAZINGA/" - "ws?WOPISrc=http%3A%2F%2Flocalhost%2Fowncloud%2Findex.php%2Fapps%" - "2Frichdocuments%2Fwopi%2Ffiles%2F165_ocgdpzbkm39u&compat="), + "3DXCookieName%253DXCookieValue%253ASuperCookieName%253DBAZINGA"), details[1]); LOK_ASSERT_EQUAL(std::string("ws"), details[2]); - LOK_ASSERT_EQUAL(std::string("1c99a7bcdbf3209782d7eb38512e6564"), details[3]); - LOK_ASSERT_EQUAL(std::string("write"), details[4]); - LOK_ASSERT_EQUAL(std::string("2"), details[5]); + LOK_ASSERT_EQUAL( + std::string("WOPISrc=http%3A%2F%2Flocalhost%2Fowncloud%2Findex.php%2Fapps%" + "2Frichdocuments%2Fwopi%2Ffiles%2F165_ocgdpzbkm39u&compat="), + details[3]); + LOK_ASSERT_EQUAL(std::string("ws"), details[4]); + LOK_ASSERT_EQUAL(std::string("1c99a7bcdbf3209782d7eb38512e6564"), details[5]); + LOK_ASSERT_EQUAL(std::string("write"), details[6]); + LOK_ASSERT_EQUAL(std::string("2"), details[7]); + + LOK_ASSERT_EQUAL(std::string("lool"), details.getField(RequestDetails::Field::Type)); + LOK_ASSERT(details.equals(RequestDetails::Field::Type, "lool")); + LOK_ASSERT_EQUAL(std::string("1c99a7bcdbf3209782d7eb38512e6564"), details.getField(RequestDetails::Field::SessionId)); + LOK_ASSERT(details.equals(RequestDetails::Field::SessionId, "1c99a7bcdbf3209782d7eb38512e6564")); + LOK_ASSERT_EQUAL(std::string("write"), details.getField(RequestDetails::Field::Command)); + LOK_ASSERT(details.equals(RequestDetails::Field::Command, "write")); + LOK_ASSERT_EQUAL(std::string("2"), details.getField(RequestDetails::Field::Serial)); + LOK_ASSERT(details.equals(RequestDetails::Field::Serial, "2")); + } + + { + static const std::string URI + = "/lool/%2Ftmp%2Fslideshow_b8c3225b_setclientpart.odp/Ar3M1X89mVaryYkh/" + "UjaCGP4cYHlU6TvUGdnFTPi8hjOS87uFym7ruWMq3F3jBr0kSPgVhbKz5CwUyV8R/slideshow.svg"; + + Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, URI, + Poco::Net::HTTPMessage::HTTP_1_1); + request.setHost(Root); + request.set("User-Agent", WOPI_AGENT_STRING); + request.set("ProxyPrefix", ProxyPrefix); + + RequestDetails details(request, ""); + LOK_ASSERT_EQUAL(true, details.isProxy()); + LOK_ASSERT_EQUAL(ProxyPrefix, details.getProxyPrefix()); + + LOK_ASSERT_EQUAL(Root, details.getHostUntrusted()); + LOK_ASSERT_EQUAL(false, details.isWebSocket()); + LOK_ASSERT_EQUAL(true, details.isGet()); + + const std::string docUri + = "/tmp/slideshow_b8c3225b_setclientpart.odp"; + + LOK_ASSERT_EQUAL(docUri, details.getLegacyDocumentURI()); + LOK_ASSERT_EQUAL(docUri, details.getDocumentURI()); + + LOK_ASSERT_EQUAL(std::string(), details.getField(RequestDetails::Field::WOPISrc)); + + LOK_ASSERT_EQUAL(static_cast<std::size_t>(5), details.size()); + LOK_ASSERT_EQUAL(std::string("lool"), details[0]); + LOK_ASSERT(details.equals(0, "lool")); + LOK_ASSERT_EQUAL(std::string("%2Ftmp%2Fslideshow_b8c3225b_setclientpart.odp"), details[1]); + LOK_ASSERT_EQUAL(std::string("Ar3M1X89mVaryYkh"), details[2]); + LOK_ASSERT_EQUAL(std::string("UjaCGP4cYHlU6TvUGdnFTPi8hjOS87uFym7ruWMq3F3jBr0kSPgVhbKz5CwUyV8R"), details[3]); + LOK_ASSERT_EQUAL(std::string("slideshow.svg"), details[4]); + + LOK_ASSERT_EQUAL(std::string("lool"), details.getField(RequestDetails::Field::Type)); + LOK_ASSERT(details.equals(RequestDetails::Field::Type, "lool")); + LOK_ASSERT_EQUAL(std::string(""), details.getField(RequestDetails::Field::SessionId)); + LOK_ASSERT(details.equals(RequestDetails::Field::SessionId, "")); + LOK_ASSERT_EQUAL(std::string(""), details.getField(RequestDetails::Field::Command)); + LOK_ASSERT(details.equals(RequestDetails::Field::Command, "")); + LOK_ASSERT_EQUAL(std::string(""), details.getField(RequestDetails::Field::Serial)); + LOK_ASSERT(details.equals(RequestDetails::Field::Serial, "")); + } + + { + static const std::string URI = "/lool/" + "clipboard?WOPISrc=file%3A%2F%2F%2Ftmp%2Fcopypasteef324307_" + "empty.ods&ServerId=7add98ed&ViewId=0&Tag=5f7972ce4e6a37dd"; + + Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, URI, + Poco::Net::HTTPMessage::HTTP_1_1); + request.setHost(Root); + request.set("User-Agent", WOPI_AGENT_STRING); + request.set("ProxyPrefix", ProxyPrefix); + + RequestDetails details(request, ""); + LOK_ASSERT_EQUAL(true, details.isProxy()); + LOK_ASSERT_EQUAL(ProxyPrefix, details.getProxyPrefix()); + + LOK_ASSERT_EQUAL(Root, details.getHostUntrusted()); + LOK_ASSERT_EQUAL(false, details.isWebSocket()); + LOK_ASSERT_EQUAL(true, details.isGet()); + + const std::string docUri = "clipboard"; + + LOK_ASSERT_EQUAL(docUri, details.getLegacyDocumentURI()); + LOK_ASSERT_EQUAL(docUri, details.getDocumentURI()); + + LOK_ASSERT_EQUAL(static_cast<std::size_t>(3), details.size()); + LOK_ASSERT_EQUAL(std::string("lool"), details[0]); + LOK_ASSERT(details.equals(0, "lool")); + LOK_ASSERT_EQUAL(std::string("clipboard"), details[1]); + + LOK_ASSERT_EQUAL(std::string("lool"), details.getField(RequestDetails::Field::Type)); + LOK_ASSERT(details.equals(RequestDetails::Field::Type, "lool")); + LOK_ASSERT_EQUAL(std::string(""), details.getField(RequestDetails::Field::SessionId)); + LOK_ASSERT(details.equals(RequestDetails::Field::SessionId, "")); + LOK_ASSERT_EQUAL(std::string(""), details.getField(RequestDetails::Field::Command)); + LOK_ASSERT(details.equals(RequestDetails::Field::Command, "")); + LOK_ASSERT_EQUAL(std::string(""), details.getField(RequestDetails::Field::Serial)); + LOK_ASSERT(details.equals(RequestDetails::Field::Serial, "")); } } diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 917ef33fb..51bd22e38 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -2277,7 +2277,7 @@ private: // re-write ServiceRoot and cache. RequestDetails requestDetails(request, LOOLWSD::ServiceRoot); -// LOG_TRC("Request details " << requestDetails.toString()); + // LOG_TRC("Request details " << requestDetails.toString()); // Config & security ... if (requestDetails.isProxy()) @@ -2293,14 +2293,14 @@ private: { // Unit testing, nothing to do here } - else if (requestDetails.equals(0, "loleaflet")) + else if (requestDetails.equals(RequestDetails::Field::Type, "loleaflet")) { // File server assert(socket && "Must have a valid socket"); FileServerRequestHandler::handleRequest(request, requestDetails, message, socket); socket->shutdown(); } - else if (requestDetails.equals(0, "lool") && + else if (requestDetails.equals(RequestDetails::Field::Type, "lool") && requestDetails.equals(1, "adminws")) { // Admin connections @@ -2314,7 +2314,7 @@ private: } } - else if (requestDetails.equals(0, "lool") && + else if (requestDetails.equals(RequestDetails::Field::Type, "lool") && requestDetails.equals(1, "getMetrics")) { // See metrics.txt @@ -2372,7 +2372,7 @@ private: else if (requestDetails.isGet("/robots.txt")) handleRobotsTxtRequest(request, socket); - else if (requestDetails.equals(0, "lool") && + else if (requestDetails.equals(RequestDetails::Field::Type, "lool") && requestDetails.equals(1, "clipboard")) { // Util::dumpHex(std::cerr, "clipboard:\n", "", socket->getInBuffer()); // lots of data ... @@ -2382,11 +2382,11 @@ private: else if (requestDetails.isProxy() && requestDetails.equals(2, "ws")) handleClientProxyRequest(request, requestDetails, message, disposition); - else if (requestDetails.equals(0, "lool") && + else if (requestDetails.equals(RequestDetails::Field::Type, "lool") && requestDetails.equals(2, "ws") && requestDetails.isWebSocket()) handleClientWsUpgrade(request, requestDetails, disposition, socket); - else if (!requestDetails.isWebSocket() && requestDetails.equals(0, "lool")) + else if (!requestDetails.isWebSocket() && requestDetails.equals(RequestDetails::Field::Type, "lool")) { // All post requests have url prefix 'lool'. handlePostRequest(requestDetails, request, message, disposition, socket); @@ -2810,9 +2810,10 @@ private: const std::string formName(form.get("name")); // Validate the docKey - std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex); - std::string decodedUri = requestDetails.getDocumentURI(); + const std::string decodedUri = requestDetails.getLegacyDocumentURI(); const std::string docKey = DocumentBroker::getDocKey(DocumentBroker::sanitizeURI(decodedUri)); + + std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex); auto docBrokerIt = DocBrokers.find(docKey); // Maybe just free the client from sending childid in form ? @@ -2845,8 +2846,9 @@ private: // TODO: Check that the user in question has access to this file! // 1. Validate the dockey - std::string decodedUri = requestDetails.getDocumentURI(); + const std::string decodedUri = requestDetails.getLegacyDocumentURI(); const std::string docKey = DocumentBroker::getDocKey(DocumentBroker::sanitizeURI(decodedUri)); + std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex); auto docBrokerIt = DocBrokers.find(docKey); if (docBrokerIt == DocBrokers.end()) @@ -2932,7 +2934,8 @@ private: Poco::MemoryInputStream& message, SocketDisposition &disposition) { - std::string url = requestDetails.getDocumentURI(); + //FIXME: The DocumentURI includes the WOPISrc, which makes it potentially invalid URI. + const std::string url = requestDetails.getLegacyDocumentURI(); LOG_INF("URL [" << url << "]."); const auto uriPublic = DocumentBroker::sanitizeURI(url); @@ -3022,7 +3025,7 @@ private: SocketDisposition& disposition, const std::shared_ptr<StreamSocket>& socket) { - std::string url = requestDetails.getDocumentURI(); + const std::string url = requestDetails.getLegacyDocumentURI(); assert(socket && "Must have a valid socket"); // must be trace for anonymization diff --git a/wsd/ProxyProtocol.cpp b/wsd/ProxyProtocol.cpp index a2b2c75b9..4b6950a10 100644 --- a/wsd/ProxyProtocol.cpp +++ b/wsd/ProxyProtocol.cpp @@ -35,12 +35,8 @@ void DocumentBroker::handleProxyRequest( const RequestDetails &requestDetails, const std::shared_ptr<StreamSocket> &socket) { - const size_t session = 3; - const size_t command = 4; - const size_t serial = 5; - std::shared_ptr<ClientSession> clientSession; - if (requestDetails.equals(command, "open")) + if (requestDetails.equals(RequestDetails::Field::Command, "open")) { bool isLocal = socket->isLocal(); LOG_TRC("proxy: validate that socket is from localhost: " << isLocal); @@ -73,7 +69,7 @@ void DocumentBroker::handleProxyRequest( } else { - std::string sessionId = requestDetails[session]; + const std::string sessionId = requestDetails.getField(RequestDetails::Field::SessionId); LOG_TRC("proxy: find session for " << _docKey << " with id " << sessionId); for (const auto &it : _sessions) { @@ -98,15 +94,14 @@ void DocumentBroker::handleProxyRequest( addSocketToPoll(socket); auto proxy = std::static_pointer_cast<ProxyProtocolHandler>(protocol); - if (requestDetails.equals(command, "close")) + if (requestDetails.equals(RequestDetails::Field::Command, "close")) { LOG_TRC("Close session"); proxy->notifyDisconnected(); return; } - (void)serial; // in URL for logging, debugging, and uniqueness. - bool isWaiting = requestDetails.equals(command, "wait"); + const bool isWaiting = requestDetails.equals(RequestDetails::Field::Command, "wait"); proxy->handleRequest(isWaiting, socket); } diff --git a/wsd/RequestDetails.cpp b/wsd/RequestDetails.cpp index 02482a9e1..f89745abb 100644 --- a/wsd/RequestDetails.cpp +++ b/wsd/RequestDetails.cpp @@ -10,10 +10,39 @@ #include <config.h> #include "RequestDetails.hpp" +#include "common/Log.hpp" #include <Poco/URI.h> #include "Exceptions.hpp" +/// Returns true iff the two containers are equal. +template <typename T> bool equal(const T& lhs, const T& rhs) +{ + if (lhs.size() != rhs.size()) + { + LOG_ERR("!!! Size mismatch: [" << lhs.size() << "] != [" << rhs.size() << "]."); + return false; + } + + const auto endLeft = std::end(lhs); + + auto itRight = std::begin(rhs); + + for (auto itLeft = std::begin(lhs); itLeft != endLeft; ++itLeft, ++itRight) + { + const auto subLeft = lhs.getParam(*itLeft); + const auto subRight = rhs.getParam(*itRight); + + if (subLeft != subRight) + { + LOG_ERR("!!! Data mismatch: [" << subLeft << "] != [" << subRight << "]"); + return false; + } + } + + return true; +} + RequestDetails::RequestDetails(Poco::Net::HTTPRequest &request, const std::string& serviceRoot) : _isMobile(false) { @@ -39,6 +68,19 @@ RequestDetails::RequestDetails(Poco::Net::HTTPRequest &request, const std::strin _hostUntrusted = request.getHost(); #endif + // Poco::SyntaxException is thrown when the syntax is invalid. + Poco::URI uri(_uriString); + for (const auto& param : uri.getQueryParameters()) + { + LOG_TRC("Decoding param [" << param.first << "] = [" << param.second << "]."); + + std::string value; + Poco::URI::decode(param.second, value); + + _params.emplace(param.first, value); + } + + // First tokenize by '/' then by '?'. std::vector<StringToken> tokens; const auto len = _uriString.size(); if (len > 0) @@ -48,21 +90,6 @@ RequestDetails::RequestDetails(Poco::Net::HTTPRequest &request, const std::strin { if (_uriString[i] == '/' || _uriString[i] == '?') { - if (_uriString[i] == '/') - { - // Wopi also uses /ws? in the URL, which - // we need to avoid confusing with the - // trailing /ws/<command>/<sessionId>/<serial>. - // E.g. /ws?WOPISrc= - if (i + 3 < len && _uriString[i + 1] == 'w' && _uriString[i + 2] == 's' - && _uriString[i + 3] == '?') - { - // Skip over '/ws?' - i += 4; - continue; - } - } - if (i - start > 0) // ignore empty tokens.emplace_back(start, i - start); start = i + 1; @@ -72,6 +99,84 @@ RequestDetails::RequestDetails(Poco::Net::HTTPRequest &request, const std::strin tokens.emplace_back(start, i - start); _pathSegs = StringVector(_uriString, std::move(tokens)); } + + + std::size_t off = 0; + std::size_t posDocUri = _uriString.find_first_of('/'); + if (posDocUri == 0) + { + off = 1; + posDocUri = _uriString.find_first_of('/', 1); + } + + _fields[Field::Type] = _uriString.substr(off, posDocUri - off); // The first is always the type. + std::string uriRes = _uriString.substr(posDocUri + 1); + + const auto posLastWS = uriRes.rfind("/ws"); + // DocumentURI is the second segment in lool URIs. + if (_pathSegs.equals(0, "lool")) + { + //FIXME: For historic reasons the DocumentURI includes the WOPISrc. + // This is problematic because decoding a URI that embedds not one, but + // *two* encoded URIs within it is bound to produce an invalid URI. + // Potentially three '?' might exist in the result (after decoding). + std::size_t end = uriRes.rfind("/ws?"); + if (end != std::string::npos) + { + // Until the end of the WOPISrc. + // e.g. <encoded-document-URI+options>/ws?WOPISrc=<encoded-document-URI>&compat= + end = uriRes.find_first_of("/?", end + 4, 2); // Start searching after '/ws?'. + } + else + { + end = (posLastWS != std::string::npos ? posLastWS : uriRes.find('/')); + if (end == std::string::npos) + end = uriRes.find('?'); // e.g. /lool/clipboard?WOPISrc=file%3A%2F%2F%2Ftmp%2Fcopypasteef324307_empty.ods... + } + + const std::string docUri = uriRes.substr(0, end); + + std::string decoded; + Poco::URI::decode(docUri, decoded); + _fields[Field::LegacyDocumentURI] = decoded; + + // Find the DocumentURI proper. + end = uriRes.find_first_of("/?", 0, 2); + decoded.clear(); + Poco::URI::decode(uriRes.substr(0, end), decoded); + _fields[Field::DocumentURI] = decoded; + } + else // Otherwise, it's the full URI. + { + _fields[Field::LegacyDocumentURI] = _uriString; + _fields[Field::DocumentURI] = _uriString; + } + + _fields[Field::WOPISrc] = getParam("WOPISrc"); + + // &compat= + const std::string compat = getParam("compat"); + if (!compat.empty()) + _fields[Field::Compat] = compat; + + // /ws[/<sessionId>/<command>/<serial>] + if (posLastWS != std::string::npos) + { + std::string lastWS = uriRes.substr(posLastWS); + const auto proxyTokens = Util::tokenize(lastWS, '/'); + if (proxyTokens.size() > 1) + { + _fields[Field::SessionId] = proxyTokens[1]; + if (proxyTokens.size() > 2) + { + _fields[Field::Command] = proxyTokens[2]; + if (proxyTokens.size() > 3) + { + _fields[Field::Serial] = proxyTokens[3]; + } + } + } + } } RequestDetails::RequestDetails(const std::string &mobileURI) @@ -87,15 +192,4 @@ RequestDetails::RequestDetails(const std::string &mobileURI) _fields[Field::DocumentURI] = _uriString; } -std::string RequestDetails::getDocumentURI() const -{ - if (_isMobile) - return _uriString; - - assert(equals(0, "lool")); - std::string docURI; - Poco::URI::decode(_pathSegs[1], docURI); - return docURI; -} - /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/wsd/RequestDetails.hpp b/wsd/RequestDetails.hpp index b931b98f8..695db11cc 100644 --- a/wsd/RequestDetails.hpp +++ b/wsd/RequestDetails.hpp @@ -13,13 +13,100 @@ #include <common/StringVector.hpp> #include <common/Util.hpp> +#include <common/Log.hpp> /** * A class to encapsulate various useful pieces from the request. * as well as path parsing goodness. + * + * The URI is complex and encapsulates multiple segments with + * different purposes and consumers. There are three URIs + * formats/modes that are supported. + * + * literal URI: used by ConvertToBroker. + * Origin: ConvertToBroker::startConversion + * Format: + * <anything> + * Identifier: special constructor that takes a string. + * Example: + * convert-to + * + * loleaflet URI: used to load loleaflet.html and other static files. + * Origin: the page where the document will be embedded. + * Format: + * /loleaflet/<loolwsd-version-hash>/[path/]<filename>.<ext>[?WOPISrc=<encoded-document-URI>] + * Identifier: /loleaflet/. + * Examples: + * /loleaflet/49c225146/src/map/Clipboard.js + * /loleaflet/49c225146/loleaflet.html?WOPISrc=http%3A%2F%2Flocalhost%2Fnextcloud%2Findex.php%2Fapps%2Frichdocuments%2Fwopi%2Ffiles%2F593_ocqiesh0cngs&title=empty.odt&lang=en-us&closebutton=1&revisionhistory=1 + * + * lool URI: used to load the document. + * Origin: loleaflet.html + * Format: + * /lool/<encoded-document-URI+options>/ws?WOPISrc=<encoded-document-URI>&compat=/ws[/<sessionId>/<command>/<serial>] + * Identifier: /lool/. + * + * The 'document-URI' is the original URL in the client that is used to load the document page. + * The optional section at the end, in square-brackets, is for richproxy. + * + * Example: + * /lool/http%3A%2F%2Flocalhost%2Fowncloud%2Findex.php%2Fapps%2Frichdocuments%2Fwopi%2Ffiles%2F165_ocgdpzbkm39u%3F + * access_token%3DODhIXdJdbsVYQoKKCuaYofyzrovxD3MQ%26access_token_ttl%3D0%26reuse_cookies%3DXCookieName%253DXCookieValue%253 + * ASuperCookieName%253DBAZINGA/ws?WOPISrc=http%3A%2F%2Flocalhost%2Fowncloud%2Findex.php%2Fapps%2Frichdocuments%2Fwopi%2F + * files%2F165_ocgdpzbkm39u&compat=/ws/1c99a7bcdbf3209782d7eb38512e6564/write/2 + * Where: + * encoded-document-URI+options: + * http%3A%2F%2Flocalhost%2Fowncloud%2Findex.php%2Fapps%2Frichdocuments%2Fwopi%2Ffiles%2F165_ocgdpzbkm39u%3F + * access_token%3DODhIXdJdbsVYQoKKCuaYofyzrovxD3MQ%26access_token_ttl%3D0%26reuse_cookies%3DXCookieName%253DXCookieValue%253 + * ASuperCookieName%253DBAZINGA + * encoded-document-URI: + * http%3A%2F%2Flocalhost%2Fowncloud%2Findex.php%2Fapps%2Frichdocuments%2Fwopi%2Ffiles%2F165_ocgdpzbkm39u + * sessionId: + * 1c99a7bcdbf3209782d7eb38512e6564 + * command: + * write + * serial: + * 2 + * In decoded form: + * document-URI+options: + * http://localhost/owncloud/index.php/apps/richdocuments/wopi/files/165_ocgdpzbkm39u?access_token= + * ODhIXdJdbsVYQoKKCuaYofyzrovxD3MQ&access_token_ttl=0&reuse_cookies=XCookieName%3DXCookieValue%3ASuperCookieName%3DBAZINGA + * document-URI: + * http://localhost/owncloud/index.php/apps/richdocuments/wopi/files/165_ocgdpzbkm39u + * + * Note that the options are still encoded and need decoding separately. + * + * Due to the multi-layer nature of the URI, it raises many difficulties, not least + * the fact that it has multiple query parameters ('?' sections). It also has foreslash + * delimiters after query parameters. + * + * The different sections are henceforth given names to help both in documenting and + * communicating them, and to facilitate parsing them. + * + * /lool/<encoded-document-URI+options>/ws?WOPISrc=<encoded-document-URI>&compat=/ws[/<sessionId>/<command>/<serial>] + * |--------documentURI---------| |-------WOPISrc------| |--------------compat--------------| + * |options| |sessionId| |command| |serial| + * |---------------------------LegacyDocumentURI---------------------------| */ class RequestDetails { +public: + + /// The fields of the URI. + enum class Field + { + Type, + DocumentURI, + LegacyDocumentURI, //< Legacy, to be removed. + WOPISrc, + Compat, + SessionId, + Command, + Serial + }; + +private: + bool _isGet : 1; bool _isHead : 1; bool _isProxy : 1; @@ -30,12 +117,21 @@ class RequestDetails std::string _hostUntrusted; std::string _documentURI; StringVector _pathSegs; + std::map<std::string, std::string> _params; + std::map<Field, std::string> _fields; + public: + RequestDetails(Poco::Net::HTTPRequest &request, const std::string& serviceRoot); RequestDetails(const std::string &mobileURI); + // matches the WOPISrc if used. For load balancing // must be 2nd element in the path after /lool/<here> - std::string getDocumentURI() const; + std::string getLegacyDocumentURI() const { return getField(Field::LegacyDocumentURI); } + + /// The DocumentURI, decoded. Doesn't contain WOPISrc or any other appendages. + std::string getDocumentURI() const { return getField(Field::DocumentURI); } + std::string getURI() const { return _uriString; @@ -68,32 +164,54 @@ public: { return (_isGet || _isHead) && _uriString == path; } - bool startsWith(const char *path) - { - return Util::startsWith(_uriString, path); - } - bool equals(size_t index, const char *string) const + + bool equals(std::size_t index, const char* string) const { return _pathSegs.equals(index, string); } - std::string operator[](size_t index) const + + /// Return the segment of the URI at index. + /// URI segments are delimited by '/'. + std::string operator[](std::size_t index) const { return _pathSegs[index]; } - size_t size() const + + /// Returns the number of segments in the URI. + std::size_t size() const { return _pathSegs.size(); } + + std::string getParam(const std::string& name) const + { + const auto it = _params.find(name); + return it != _params.end() ? it->second : std::string(); + } + + std::string getField(const Field field) const + { + const auto it = _fields.find(field); + return it != _fields.end() ? it->second : std::string(); + } + + bool equals(const Field field, const char* string) const + { + const auto it = _fields.find(field); + return it != _fields.end() ? it->second == string : (string == nullptr || *string == '\0'); + } + std::string toString() const { std::ostringstream oss; oss << _uriString << ' ' << (_isGet?"G":"") << (_isHead?"H":"") << (_isProxy?"Proxy":"") << (_isWebSocket?"WebSocket":""); - oss << " host: " << _hostUntrusted; - oss << " path: " << _pathSegs.size(); - for (size_t i = 0; i < _pathSegs.size(); ++i) - oss << " '" << _pathSegs[i] << "'"; + oss << ", host: " << _hostUntrusted; + oss << ", path: " << _pathSegs.size(); + for (std::size_t i = 0; i < _pathSegs.size(); ++i) + oss << "\n[" << i << "] '" << _pathSegs[i] << "'"; + oss << "\nfull URI: " << _uriString; return oss.str(); } }; commit bab2f6b355486b5752a2f0d252fe9ca6a57cee5b Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> AuthorDate: Mon May 25 07:51:04 2020 -0400 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Jun 8 22:19:53 2020 +0200 wsd: proxy: correctly parse single-char fields in the URI ...instead of skipping them. And add tests to defend the fix. Change-Id: I8585cc3592841c8ad16d3804dc09a2a3b3a3bb71 Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95291 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Ashod Nakashian <ashnak...@gmail.com> diff --git a/test/WhiteBoxTests.cpp b/test/WhiteBoxTests.cpp index a97451172..e8290cd2f 100644 --- a/test/WhiteBoxTests.cpp +++ b/test/WhiteBoxTests.cpp @@ -940,7 +940,9 @@ void WhiteBoxTests::testRequestDetails_local() = "http://localhost/nextcloud/apps/richdocuments/proxy.php?req="; { - static const std::string URI = "/lool/file%3A%2F%2F%2Fhome%2Fash%2Fprj%2Flo%2Fonline%2Ftest%2Fdata%2Fhello-world.odt/ws/open/open/0"; + static const std::string URI = "/lool/" + "file%3A%2F%2F%2Fhome%2Fash%2Fprj%2Flo%2Fonline%2Ftest%" + "2Fdata%2Fhello-world.odt/ws/open/open/0"; Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, URI, Poco::Net::HTTPMessage::HTTP_1_1); @@ -960,7 +962,7 @@ void WhiteBoxTests::testRequestDetails_local() LOK_ASSERT_EQUAL(docUri, details.getDocumentURI()); - LOK_ASSERT_EQUAL(static_cast<std::size_t>(5), details.size()); + LOK_ASSERT_EQUAL(static_cast<std::size_t>(6), details.size()); LOK_ASSERT_EQUAL(std::string("lool"), details[0]); LOK_ASSERT(details.equals(0, "lool")); LOK_ASSERT_EQUAL( @@ -970,18 +972,13 @@ void WhiteBoxTests::testRequestDetails_local() LOK_ASSERT_EQUAL(std::string("ws"), details[2]); LOK_ASSERT_EQUAL(std::string("open"), details[3]); LOK_ASSERT_EQUAL(std::string("open"), details[4]); + LOK_ASSERT_EQUAL(std::string("0"), details[5]); } { - static const std::string URI - = "/lool/" - "http%3A%2F%2Flocalhost%2Fowncloud%2Findex.php%2Fapps%2Frichdocuments%2Fwopi%2Ffiles%" - "2F165_ocgdpzbkm39u%3Faccess_token%3DODhIXdJdbsVYQoKKCuaYofyzrovxD3MQ%26access_token_" - "ttl%" - "3D0%26reuse_cookies%3DXCookieName%253DXCookieValue%253ASuperCookieName%253DBAZINGA/" - "ws?WOPISrc=http%3A%2F%2Flocalhost%2Fowncloud%2Findex.php%2Fapps%2Frichdocuments%" - "2Fwopi%" - "2Ffiles%2F165_ocgdpzbkm39u&compat=/ws/1c99a7bcdbf3209782d7eb38512e6564/write/2"; + static const std::string URI = "/lool/" + "file%3A%2F%2F%2Fhome%2Fash%2Fprj%2Flo%2Fonline%2Ftest%" + "2Fdata%2Fhello-world.odt/ws//write/2"; Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, URI, Poco::Net::HTTPMessage::HTTP_1_1); @@ -997,12 +994,7 @@ void WhiteBoxTests::testRequestDetails_local() LOK_ASSERT_EQUAL(false, details.isWebSocket()); LOK_ASSERT_EQUAL(true, details.isGet()); - const std::string docUri - = "http://localhost/owncloud/index.php/apps/richdocuments/wopi/files/" - "165_ocgdpzbkm39u?access_token=ODhIXdJdbsVYQoKKCuaYofyzrovxD3MQ&access_token_ttl=0&" - "reuse_cookies=XCookieName%3DXCookieValue%3ASuperCookieName%3DBAZINGA/" - "ws?WOPISrc=http://localhost/owncloud/index.php/apps/richdocuments/wopi/files/" - "165_ocgdpzbkm39u&compat="; + const std::string docUri = "file:///home/ash/prj/lo/online/test/data/hello-world.odt"; LOK_ASSERT_EQUAL(docUri, details.getDocumentURI()); @@ -1010,16 +1002,12 @@ void WhiteBoxTests::testRequestDetails_local() LOK_ASSERT_EQUAL(std::string("lool"), details[0]); LOK_ASSERT(details.equals(0, "lool")); LOK_ASSERT_EQUAL( - std::string("http%3A%2F%2Flocalhost%2Fowncloud%2Findex.php%2Fapps%2Frichdocuments%" - "2Fwopi%2Ffiles%2F165_ocgdpzbkm39u%3Faccess_token%" - "3DODhIXdJdbsVYQoKKCuaYofyzrovxD3MQ%26access_token_ttl%3D0%26reuse_cookies%" - "3DXCookieName%253DXCookieValue%253ASuperCookieName%253DBAZINGA/" - "ws?WOPISrc=http%3A%2F%2Flocalhost%2Fowncloud%2Findex.php%2Fapps%" - "2Frichdocuments%2Fwopi%2Ffiles%2F165_ocgdpzbkm39u&compat="), + std::string( + "file%3A%2F%2F%2Fhome%2Fash%2Fprj%2Flo%2Fonline%2Ftest%2Fdata%2Fhello-world.odt"), details[1]); LOK_ASSERT_EQUAL(std::string("ws"), details[2]); - LOK_ASSERT_EQUAL(std::string("1c99a7bcdbf3209782d7eb38512e6564"), details[3]); - LOK_ASSERT_EQUAL(std::string("write"), details[4]); + LOK_ASSERT_EQUAL(std::string("write"), details[3]); + LOK_ASSERT_EQUAL(std::string("2"), details[4]); } } @@ -1134,7 +1122,7 @@ void WhiteBoxTests::testRequestDetails() LOK_ASSERT_EQUAL(docUri, details.getDocumentURI()); - LOK_ASSERT_EQUAL(static_cast<std::size_t>(5), details.size()); + LOK_ASSERT_EQUAL(static_cast<std::size_t>(6), details.size()); LOK_ASSERT_EQUAL(std::string("lool"), details[0]); LOK_ASSERT(details.equals(0, "lool")); LOK_ASSERT_EQUAL( @@ -1148,6 +1136,7 @@ void WhiteBoxTests::testRequestDetails() LOK_ASSERT_EQUAL(std::string("ws"), details[2]); LOK_ASSERT_EQUAL(std::string("1c99a7bcdbf3209782d7eb38512e6564"), details[3]); LOK_ASSERT_EQUAL(std::string("write"), details[4]); + LOK_ASSERT_EQUAL(std::string("2"), details[5]); } } diff --git a/wsd/RequestDetails.cpp b/wsd/RequestDetails.cpp index cba8f89af..02482a9e1 100644 --- a/wsd/RequestDetails.cpp +++ b/wsd/RequestDetails.cpp @@ -63,12 +63,12 @@ RequestDetails::RequestDetails(Poco::Net::HTTPRequest &request, const std::strin } } - if (i - start > 1) // ignore empty + if (i - start > 0) // ignore empty tokens.emplace_back(start, i - start); start = i + 1; } } - if (i - start > 1) // ignore empty + if (i - start > 0) // ignore empty tokens.emplace_back(start, i - start); _pathSegs = StringVector(_uriString, std::move(tokens)); } commit d1b751a1f6370118c3c829662b37b58a98b10f65 Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> AuthorDate: Tue Jun 2 03:44:08 2020 -0400 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Jun 8 22:19:53 2020 +0200 wsd: performance improvements Change-Id: I137dc67b4231df1cd23a9dce72e6b12dc1bf364e Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95343 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Ashod Nakashian <ashnak...@gmail.com> diff --git a/common/Rectangle.hpp b/common/Rectangle.hpp index 3e73bc251..0869d207f 100644 --- a/common/Rectangle.hpp +++ b/common/Rectangle.hpp @@ -9,6 +9,7 @@ #pragma once +#include <algorithm> // std::min, std::max #include <limits> namespace Util diff --git a/common/Util.hpp b/common/Util.hpp index d41f81319..fed1525e6 100644 --- a/common/Util.hpp +++ b/common/Util.hpp @@ -382,9 +382,8 @@ namespace Util return false; } - /// Tokenize space-delimited values until we hit new-line or the end. - template <typename T> - inline void tokenize(const char* data, const std::size_t size, const T& delimiter, + /// Tokenize delimited values until we hit new-line or the end. + inline void tokenize(const char* data, const std::size_t size, const char delimiter, std::vector<StringToken>& tokens) { if (size == 0 || data == nullptr || *data == '\0') diff --git a/kit/Kit.cpp b/kit/Kit.cpp index 9fbc658b4..743251fef 100644 --- a/kit/Kit.cpp +++ b/kit/Kit.cpp @@ -69,6 +69,7 @@ #if !MOBILEAPP #include <common/SigUtil.hpp> #include <common/Seccomp.hpp> +#include <utility> #endif #ifdef FUZZER @@ -2226,7 +2227,7 @@ public: void setDocument(std::shared_ptr<Document> document) { - _document = document; + _document = std::move(document); } }; diff --git a/wsd/Admin.cpp b/wsd/Admin.cpp index 3f48a0d8a..8d85dfbc7 100644 --- a/wsd/Admin.cpp +++ b/wsd/Admin.cpp @@ -602,7 +602,7 @@ std::string Admin::getChannelLogLevels() return result; } -void Admin::setChannelLogLevel(std::string _channelName, std::string _level) +void Admin::setChannelLogLevel(const std::string& _channelName, std::string _level) { assertCorrectThread(); diff --git a/wsd/Admin.hpp b/wsd/Admin.hpp index ae3967e06..42e7a247a 100644 --- a/wsd/Admin.hpp +++ b/wsd/Admin.hpp @@ -107,7 +107,7 @@ public: std::string getChannelLogLevels(); - void setChannelLogLevel(std::string _channelName, std::string _level); + void setChannelLogLevel(const std::string& _channelName, std::string _level); std::string getLogLines(); diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index d164a0d45..d534a2656 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -1034,7 +1034,7 @@ void ClientSession::writeQueuedMessages() } // NB. also see loleaflet/src/map/Clipboard.js that does this in JS for stubs. -void ClientSession::postProcessCopyPayload(std::shared_ptr<Message> payload) +void ClientSession::postProcessCopyPayload(const std::shared_ptr<Message>& payload) { // Insert our meta origin if we can payload->rewriteDataBody([=](std::vector<char>& data) { @@ -1359,7 +1359,7 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt LOOLWSD::SavedClipboards->insertClipboard( _clipboardKeys, &payload->data()[header], payload->size() - header); - for (auto it : _clipSockets) + for (const auto& it : _clipSockets) { std::ostringstream oss; oss << "HTTP/1.1 200 OK\r\n" diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp index c7d4e66ed..6103d0550 100644 --- a/wsd/ClientSession.hpp +++ b/wsd/ClientSession.hpp @@ -163,7 +163,7 @@ public: std::string getClipboardURI(bool encode = true); /// Adds and/or modified the copied payload before sending on to the client. - void postProcessCopyPayload(std::shared_ptr<Message> payload); + void postProcessCopyPayload(const std::shared_ptr<Message>& payload); /// Returns true if we're expired waiting for a clipboard and should be removed bool staleWaitDisconnect(const std::chrono::steady_clock::time_point &now); diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 2b6bb013e..917ef33fb 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -2567,7 +2567,7 @@ private: Poco::URI requestUri(request.getURI()); Poco::URI::QueryParameters params = requestUri.getQueryParameters(); std::string WOPISrc, serverId, viewId, tag, mime; - for (auto it : params) + for (const auto& it : params) { if (it.first == "WOPISrc") WOPISrc = it.second; @@ -3937,6 +3937,7 @@ std::vector<std::shared_ptr<DocumentBroker>> LOOLWSD::getBrokersTestOnly() std::lock_guard<std::mutex> docBrokersLock(DocBrokersMutex); std::vector<std::shared_ptr<DocumentBroker>> result; + result.reserve(DocBrokers.size()); for (auto& brokerIt : DocBrokers) result.push_back(brokerIt.second); return result; diff --git a/wsd/ProxyProtocol.cpp b/wsd/ProxyProtocol.cpp index 84b56b94b..a2b2c75b9 100644 --- a/wsd/ProxyProtocol.cpp +++ b/wsd/ProxyProtocol.cpp @@ -292,7 +292,7 @@ void ProxyProtocolHandler::dumpState(std::ostream& os) os << '#' << (sock ? sock->getFD() : -2) << ' '; } os << '\n'; - for (auto it : _writeQueue) + for (const auto& it : _writeQueue) Util::dumpHex(os, "\twrite queue entry:", "\t\t", *it); if (_msgHandler) _msgHandler->dumpState(os); @@ -336,7 +336,7 @@ bool ProxyProtocolHandler::flushQueueTo(const std::shared_ptr<StreamSocket> &soc return false; size_t totalSize = 0; - for (auto it : _writeQueue) + for (const auto& it : _writeQueue) totalSize += it->size(); if (!totalSize) @@ -354,7 +354,7 @@ bool ProxyProtocolHandler::flushQueueTo(const std::shared_ptr<StreamSocket> &soc "\r\n"; socket->send(oss.str()); - for (auto it : _writeQueue) + for (const auto& it : _writeQueue) socket->send(it->data(), it->size(), false); _writeQueue.clear(); commit 8e4d859f3eb0d1b6b59afa394ed49dba6ef74599 Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> AuthorDate: Mon Jun 1 23:21:22 2020 -0400 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Jun 8 22:19:53 2020 +0200 wsd: performance-unnecessary-value-param Change-Id: I1eb092c676da8600e0f8ed70cbc7e1f37fdd5a02 Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95338 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Tested-by: Jenkins Reviewed-by: Ashod Nakashian <ashnak...@gmail.com> diff --git a/common/Message.hpp b/common/Message.hpp index 320760c5b..cb27e2c97 100644 --- a/common/Message.hpp +++ b/common/Message.hpp @@ -118,7 +118,7 @@ public: bool isBinary() const { return _type == Type::Binary; } /// Allows some in-line re-writing of the message - void rewriteDataBody(std::function<bool (std::vector<char> &)> func) + void rewriteDataBody(const std::function<bool (std::vector<char> &)>& func) { if (func(_data)) { diff --git a/common/Util.cpp b/common/Util.cpp index b51295ebd..347fc4562 100644 --- a/common/Util.cpp +++ b/common/Util.cpp @@ -841,7 +841,7 @@ namespace Util return std::string::npos; } - std::chrono::system_clock::time_point getFileTimestamp(std::string str_path) + std::chrono::system_clock::time_point getFileTimestamp(const std::string& str_path) { struct stat file; stat(str_path.c_str(), &file); diff --git a/common/Util.hpp b/common/Util.hpp index b4c5c1234..d41f81319 100644 --- a/common/Util.hpp +++ b/common/Util.hpp @@ -1015,7 +1015,7 @@ int main(int argc, char**argv) std::string getHttpTime(std::chrono::system_clock::time_point time); //// Return timestamp of file - std::chrono::system_clock::time_point getFileTimestamp(std::string str_path); + std::chrono::system_clock::time_point getFileTimestamp(const std::string& str_path); //// Return time in ISO8061 fraction format std::string getIso8601FracformatTime(std::chrono::system_clock::time_point time); @@ -1059,7 +1059,7 @@ int main(int argc, char**argv) * Splits string into vector<string>. Does not accept referenced variables for easy * usage like (splitString("test", ..)) or (splitString(getStringOnTheFly(), ..)) */ - inline std::vector<std::string> splitStringToVector(std::string const str, const char delim) + inline std::vector<std::string> splitStringToVector(const std::string& str, const char delim) { size_t start; size_t end = 0; @@ -1069,7 +1069,7 @@ int main(int argc, char**argv) while ((start = str.find_first_not_of(delim, end)) != std::string::npos) { end = str.find(delim, start); - result.push_back(str.substr(start, end - start)); + result.emplace_back(str.substr(start, end - start)); } return result; } diff --git a/test/helpers.hpp b/test/helpers.hpp index e246e5a57..79948f836 100644 --- a/test/helpers.hpp +++ b/test/helpers.hpp @@ -779,7 +779,7 @@ inline void deleteAll(const std::shared_ptr<LOOLWebSocket>& socket, const std::s inline std::string getAllText(const std::shared_ptr<LOOLWebSocket>& socket, const std::string& testname, - const std::string expected = std::string(), + const std::string& expected = std::string(), int retry = COMMAND_RETRY_COUNT) { static const std::string prefix = "textselectioncontent: "; diff --git a/wsd/ServerURL.hpp b/wsd/ServerURL.hpp index 4b42f3917..25d968520 100644 --- a/wsd/ServerURL.hpp +++ b/wsd/ServerURL.hpp @@ -46,7 +46,7 @@ public: _schemeAuthority = serverName; // A well formed ProxyPrefix will override it. - std::string url = proxyPrefix; + const std::string& url = proxyPrefix; if (url.size() <= 0) return; commit a6447467a99bff0b18ff8412691b46806c088a24 Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> AuthorDate: Mon Jun 1 22:46:49 2020 -0400 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Jun 8 22:19:53 2020 +0200 wsd: std::move rather than copy Change-Id: I7c4eea8aa5ab57444aa395f727e4ce6ac713b665 Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95337 Tested-by: Jenkins Reviewed-by: Ashod Nakashian <ashnak...@gmail.com> diff --git a/common/StringVector.hpp b/common/StringVector.hpp index 568ebf989..f995c0b0f 100644 --- a/common/StringVector.hpp +++ b/common/StringVector.hpp @@ -10,6 +10,7 @@ #pragma once #include <string> +#include <utility> #include <vector> /** @@ -42,9 +43,9 @@ class StringVector public: explicit StringVector() = default; - explicit StringVector(const std::string& string, const std::vector<StringToken>& tokens) - : _string(string) - , _tokens(tokens) + explicit StringVector(std::string string, std::vector<StringToken> tokens) + : _string(std::move(string)) + , _tokens(std::move(tokens)) { } diff --git a/wsd/LOOLWSD.hpp b/wsd/LOOLWSD.hpp index 1be418618..8d9653cd5 100644 --- a/wsd/LOOLWSD.hpp +++ b/wsd/LOOLWSD.hpp @@ -15,6 +15,7 @@ #include <map> #include <set> #include <string> +#include <utility> #include <signal.h> @@ -50,7 +51,7 @@ public: _name(name), _pid(pid), - _ws(handler), + _ws(std::move(handler)), _socket(socket) { LOG_INF(_name << " ctor [" << _pid << "]."); diff --git a/wsd/RequestDetails.cpp b/wsd/RequestDetails.cpp index aaaf1936b..cba8f89af 100644 --- a/wsd/RequestDetails.cpp +++ b/wsd/RequestDetails.cpp @@ -70,7 +70,7 @@ RequestDetails::RequestDetails(Poco::Net::HTTPRequest &request, const std::strin } if (i - start > 1) // ignore empty tokens.emplace_back(start, i - start); - _pathSegs = StringVector(_uriString, tokens); + _pathSegs = StringVector(_uriString, std::move(tokens)); } } commit 375e1e27c79e292dc4a3a8bbdf814d55699c8402 Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> AuthorDate: Thu May 21 10:22:49 2020 -0400 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Jun 8 22:19:53 2020 +0200 wsd: move LOOLProtocol::tokenize to Util::tokenize The tokenizer(s) are more generic than the protocol logic, and are used from contexts that don't involve the protocol as such. Change-Id: Ie8c256bf11a91e466bff794021f41603c9596a7f diff --git a/common/Authorization.cpp b/common/Authorization.cpp index cb605fd41..abaddae26 100644 --- a/common/Authorization.cpp +++ b/common/Authorization.cpp @@ -53,7 +53,7 @@ void Authorization::authorizeRequest(Poco::Net::HTTPRequest& request) const // Authorization: Basic .... // X-Something-Custom: Huh // Regular expression evaluates and finds "\n\r" and tokenizes accordingly - StringVector tokens(LOOLProtocol::tokenize(_data, "\n\r")); + StringVector tokens(Util::tokenize(_data, "\n\r")); for (auto it = tokens.begin(); it != tokens.end(); ++it) { std::string token = tokens.getParam(*it); diff --git a/common/Message.hpp b/common/Message.hpp index 8fc894b96..320760c5b 100644 --- a/common/Message.hpp +++ b/common/Message.hpp @@ -31,7 +31,7 @@ public: const enum Dir dir) : _forwardToken(getForwardToken(message.data(), message.size())), _data(skipWhitespace(message.data() + _forwardToken.size()), message.data() + message.size()), - _tokens(LOOLProtocol::tokenize(_data.data(), _data.size())), + _tokens(Util::tokenize(_data.data(), _data.size())), _id(makeId(dir)), _firstLine(LOOLProtocol::getFirstLine(_data.data(), _data.size())), _abbr(_id + ' ' + LOOLProtocol::getAbbreviatedMessage(_data.data(), _data.size())), @@ -48,7 +48,7 @@ public: const size_t reserve) : _forwardToken(getForwardToken(message.data(), message.size())), _data(std::max(reserve, message.size())), - _tokens(LOOLProtocol::tokenize(message.data() + _forwardToken.size(), message.size() - _forwardToken.size())), + _tokens(Util::tokenize(message.data() + _forwardToken.size(), message.size() - _forwardToken.size())), _id(makeId(dir)), _firstLine(LOOLProtocol::getFirstLine(message)), _abbr(_id + ' ' + LOOLProtocol::getAbbreviatedMessage(message)), @@ -67,7 +67,7 @@ public: const enum Dir dir) : _forwardToken(getForwardToken(p, len)), _data(skipWhitespace(p + _forwardToken.size()), p + len), - _tokens(LOOLProtocol::tokenize(_data.data(), _data.size())), + _tokens(Util::tokenize(_data.data(), _data.size())), _id(makeId(dir)), _firstLine(LOOLProtocol::getFirstLine(_data.data(), _data.size())), _abbr(_id + ' ' + LOOLProtocol::getAbbreviatedMessage(_data.data(), _data.size())), diff --git a/common/MessageQueue.cpp b/common/MessageQueue.cpp index b171ce553..a57525984 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); - StringVector tokens(LOOLProtocol::tokenize(seqs, ',')); + StringVector tokens(Util::tokenize(seqs, ',')); getQueue().erase(std::remove_if(getQueue().begin(), getQueue().end(), [&tokens](const Payload& v) { @@ -186,7 +186,7 @@ std::string TileQueue::removeCallbackDuplicate(const std::string& callbackMsg) { assert(LOOLProtocol::matchPrefix("callback", callbackMsg, /*ignoreWhitespace*/ true)); - StringVector tokens = LOOLProtocol::tokenize(callbackMsg); + StringVector tokens = Util::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]; - StringVector queuedTokens = LOOLProtocol::tokenize(it.data(), it.size()); + StringVector queuedTokens = Util::tokenize(it.data(), it.size()); if (queuedTokens.size() < 3) { ++i; @@ -321,7 +321,7 @@ std::string TileQueue::removeCallbackDuplicate(const std::string& callbackMsg) { auto& it = getQueue()[i]; - StringVector queuedTokens = LOOLProtocol::tokenize(it.data(), it.size()); + StringVector queuedTokens = Util::tokenize(it.data(), it.size()); if (queuedTokens.size() < 4) continue; @@ -361,13 +361,13 @@ std::string TileQueue::removeCallbackDuplicate(const std::string& callbackMsg) for (size_t i = 0; i < getQueue().size(); ++i) { - auto& it = getQueue()[i]; + const auto& it = getQueue()[i]; // skip non-callbacks quickly if (!LOOLProtocol::matchPrefix("callback", it)) continue; - StringVector queuedTokens = LOOLProtocol::tokenize(it.data(), it.size()); ... etc. - the rest is truncated _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits