[Libreoffice-commits] online.git: Branch 'distro/collabora/co-4-2' - 52 commits - android/app android/lib common/Authorization.cpp common/Crypto.cpp common/FileUtil.cpp common/JsonUtil.hpp common/Log.cpp common/LOOLWebSocket.hpp common/Message.hpp common/MessageQueue.cpp common/Protocol.cpp common/Protocol.hpp common/Rectangle.hpp common/Seccomp.cpp common/Session.cpp common/SigUtil.cpp common/StringVector.cpp common/StringVector.hpp common/Util.cpp common/Util.hpp configure.ac cypress_test/integration_tests debian/changelog debian/control debian/rules EULA fuzzer/ClientSession.cpp .gitreview ios/Mobile kit/ChildSession.cpp kit/Delta.hpp kit/ForKit.cpp kit/Kit.cpp kit/KitHelper.hpp loleaflet/admin loleaflet/css loleaflet/html loleaflet/js loleaflet/Makefile.am loleaflet/po loleaflet/src loleaflet/welcome loolkitconfig.xcu loolwsd.spec.in Makefile.am net/clientnb.cpp net/DelaySocket.cpp net/FakeSocket.cpp net/ServerSocket.hpp net/Socket.cpp net/Socket.hpp net/Ssl.cpp net/WebSocketHandler.hpp test/ countloolkits.hpp test/DeltaTests.cpp test/fakesockettest.cpp test/helpers.hpp test/lokassert.hpp test/Makefile.am test/TileCacheTests.cpp test/UnitAdmin.cpp test/UnitBadDocLoad.cpp test/UnitClient.cpp test/UnitClose.cpp test/UnitConvert.cpp test/UnitCopyPaste.cpp test/UnitCursor.cpp test/UnitHTTP.cpp test/UnitInsertDelete.cpp test/UnitLoad.cpp test/UnitPasswordProtected.cpp test/UnitRenderingOptions.cpp test/UnitSession.cpp test/UnitTyping.cpp test/UnitUNOCommand.cpp test/UnitWOPITemplate.cpp test/UnitWOPIWatermark.cpp test/WhiteBoxTests.cpp test/WopiTestServer.hpp tools/Config.cpp tools/Connect.cpp tools/KitClient.cpp tools/map.cpp tools/Replay.hpp tools/Tool.cpp tools/WebSocketDump.cpp wsd/Admin.cpp wsd/Admin.hpp wsd/AdminModel.cpp wsd/AdminModel.hpp wsd/Auth.cpp wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/FileServer.cpp wsd/LOOLWSD.cpp wsd/LOOLWSD.hpp wsd/protocol.txt wsd/ProxyProtocol.cpp wsd/ProxyProtocol.hpp wsd/QueueHandler.hpp wsd/RequestDetails.c pp wsd/RequestDetails.hpp wsd/SenderQueue.hpp wsd/ServerURL.hpp wsd/Storage.cpp wsd/TileCache.cpp wsd/TileDesc.hpp wsd/TraceFile.hpp
Andras Timar (via logerrit)
logerrit at kemper.freedesktop.org
Mon Jun 8 20:45:12 UTC 2020
Rebased ref, commits from common ancestor:
commit 73e40db867e78301cb1882c5fd469a735dcf6675
Author: Andras Timar <andras.timar at collabora.com>
AuthorDate: Mon Jun 8 22:42:38 2020 +0200
Commit: Andras Timar <andras.timar at collabora.com>
CommitDate: Mon Jun 8 22:42:38 2020 +0200
Bump package version to 4.2.4-3
Change-Id: I7c49f4d89522a01fa737978e1831c5900406f772
diff --git a/debian/changelog b/debian/changelog
index 241e84064..1e0a86899 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+loolwsd (4.2.4-3) unstable; urgency=medium
+
+ * https://cgit.freedesktop.org/libreoffice/online/log/?h=cp-4.2.4-3
+
+ -- Andras Timar <andras.timar at collabora.com> Mon, 08 Jun 2020 22:00:00 +0200
+
loolwsd (4.2.4-2) unstable; urgency=medium
* https://cgit.freedesktop.org/libreoffice/online/log/?h=cp-4.2.4-2
diff --git a/loolwsd.spec.in b/loolwsd.spec.in
index 2da566289..8cc3a2ce3 100644
--- a/loolwsd.spec.in
+++ b/loolwsd.spec.in
@@ -12,7 +12,7 @@ Name: loolwsd%{name_suffix}
Name: loolwsd
%endif
Version: @PACKAGE_VERSION@
-Release: 2%{?dist}
+Release: 3%{?dist}
Vendor: %{vendor}
Summary: LibreOffice Online WebSocket Daemon
License: EULA
commit d5f77d46554908d44e3328ca49272e5664e3dc55
Author: Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Mon Jun 8 15:46:29 2020 +0100
Commit: Andras Timar <andras.timar at collabora.com>
CommitDate: Mon Jun 8 22:22:16 2020 +0200
Proxy: handle server startup / un-responsive DocumentBroker creation.
If our 'open' request fails - then we could get this loop:
loadDocument (proxy.php?req=/loleaflet/d2d049224/src/map/Map.js:318)
_activate (proxy.php?req=/loleaflet/d2d049224/src/map/Map.js:1233)
_onSocketClose (proxy.php?req=/loleaflet/d2d049224/src/core/Socket.js:1045)
_signalErrorClose (proxy.php?req=/loleaflet/0acb00fc2/loleaflet.html?file_path=file:///tmp/copy.odt:576)
(anonymous) (proxy.php?req=/loleaflet/0acb00fc2/loleaflet.html?file_path=file:///tmp/copy.odt:688)
load (async)
getSessionId (proxy.php?req=/loleaflet/0acb00fc2/loleaflet.html?file_path=file:///tmp/copy.odt:682)
global.ProxySocket (proxy.php?req=/loleaflet/0acb00fc2/loleaflet.html?file_path=file:///tmp/copy.odt:753)
global.createWebSocket (proxy.php?req=/loleaflet/0acb00fc2/loleaflet.html?file_path=file:///tmp/copy.odt:798)
connect (proxy.php?req=/loleaflet/d2d049224/src/core/Socket.js:52)
loadDocument (proxy.php?req=/loleaflet/d2d049224/src/map/Map.js:318)
Which would hammer the server with large numbers of requests triggering
DoS protection in some cases, so:
1. only allow one 'open' in-flight at a time
2. global time-accounting to not allow >1 new ProxySocket every 250ms
3. handle error returns from 'open' correctly.
Change-Id: I1692acd72a445ebc70a83c66a2e802a532c66e21
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95837
Tested-by: Jenkins
Tested-by: Michael Meeks <michael.meeks at collabora.com>
Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
diff --git a/loleaflet/js/global.js b/loleaflet/js/global.js
index 9036da5e5..0cafa649c 100644
--- a/loleaflet/js/global.js
+++ b/loleaflet/js/global.js
@@ -221,6 +221,7 @@
this.id = window.proxySocketCounter++;
this.sendCounter = 0;
this.msgInflight = 0;
+ this.openInflight = 0;
this.inSerial = 0;
this.outSerial = 0;
this.minPollMs = 25; // Anything less than ~25 ms can overwhelm the HTTP server.
@@ -354,8 +355,13 @@
this.onerror();
this.onclose();
clearInterval(this.waitInterval);
+ clearTimeout(this.delaySession);
this.waitInterval = undefined;
this.sessionId = 'open';
+ this.inSerial = 0;
+ this.outSerial = 0;
+ this.msgInflight = 0;
+ this.openInflight = 0;
}
this.readyState = 3; // CLOSED
};
@@ -371,9 +377,6 @@
console.debug('Session closed, opening a new one.');
that.getSessionId();
}
- else
- console.debug('New session not completed.');
-
return;
}
@@ -448,14 +451,41 @@
that.msgInflight++;
};
this.getSessionId = function() {
+ if (this.openInflight > 0)
+ {
+ console.debug('Waiting for session open');
+ return;
+ }
+
+ if (this.delaySession)
+ return;
+
+ // avoid attempting to re-connect too quickly
+ if (global.lastCreatedProxySocket)
+ {
+ var msSince = performance.now() - global.lastCreatedProxySocket;
+ if (msSince < 250) {
+ var delay = 250 - msSince;
+ console.debug('Wait to re-try session creation for ' + delay + 'ms');
+ this.curPollMs = delay; // ms
+ this.delaySession = setTimeout(function() {
+ that.delaySession = undefined;
+ that.getSessionId();
+ }, delay);
+ return;
+ }
+ }
+ global.lastCreatedProxySocket = performance.now();
+
var req = new XMLHttpRequest();
req.open('POST', that.getEndPoint('open'));
req.responseType = 'text';
req.addEventListener('load', function() {
console.debug('got session: ' + this.responseText);
- if (this.responseText.indexOf('\n') >= 0)
+ if (this.status !== 200 || !this.responseText ||
+ this.responseText.indexOf('\n') >= 0) // multi-line error
{
- console.debug('Error: failed to fetch session id!');
+ console.debug('Error: failed to fetch session id! error: ' + this.status);
that._signalErrorClose();
}
else
@@ -465,7 +495,12 @@
that.onopen();
}
});
+ req.addEventListener('loadend', function() {
+ console.debug('Open completed state: ' + that.readyState);
+ that.openInflight--;
+ });
req.send('');
+ this.openInflight++;
};
this.send = function(msg) {
var hadData = this.sendQueue.length > 0;
@@ -503,6 +538,7 @@
this.readyState = 3;
this.onclose();
clearInterval(this.waitInterval);
+ clearTimeout(this.delaySession);
this.waitInterval = undefined;
if (oldState === 1) // was open
this.sendCloseMsg(this.unloading);
commit a4bcbd74ab90eb3b1e6da421dad9215393c36483
Author: Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Mon Jun 8 16:27:50 2020 +0100
Commit: Andras Timar <andras.timar at collabora.com>
CommitDate: Mon Jun 8 22:22:09 2020 +0200
Proxy: remove double images/ in path.
Change-Id: I0db7953501acf397e361a0483a999735bfce7557
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95842
Tested-by: Michael Meeks <michael.meeks at collabora.com>
Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
diff --git a/loleaflet/src/control/Control.JSDialogBuilder.js b/loleaflet/src/control/Control.JSDialogBuilder.js
index debd6739a..8c8426c9e 100644
--- a/loleaflet/src/control/Control.JSDialogBuilder.js
+++ b/loleaflet/src/control/Control.JSDialogBuilder.js
@@ -567,7 +567,7 @@ L.Control.JSDialogBuilder = L.Control.extend({
if (commandName && commandName.length && L.LOUtil.existsIconForCommand(commandName, builder.map.getDocType())) {
var iconName = builder._generateMenuIconName(commandName);
var iconSpan = L.DomUtil.create('span', 'menu-entry-icon ' + iconName, sectionTitle);
- var iconURL = L.LOUtil.getImageURL('images/lc_' + iconName + '.svg');
+ var iconURL = L.LOUtil.getImageURL('lc_' + iconName + '.svg');
icon = L.DomUtil.create('img', '', iconSpan);
icon.src = iconURL;
icon.alt = '';
commit 77e075b1dfd2f5fecb55ad9e272e2d74bff74ee4
Author: Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Mon Jun 8 14:48:57 2020 +0100
Commit: Andras Timar <andras.timar at collabora.com>
CommitDate: Mon Jun 8 22:22:04 2020 +0200
Proxy: only send close if we were connected.
Change-Id: I4b80adb1d4f44efc02b784cad10f27e458921449
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95836
Tested-by: Jenkins
Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
diff --git a/loleaflet/js/global.js b/loleaflet/js/global.js
index 1c951b089..9036da5e5 100644
--- a/loleaflet/js/global.js
+++ b/loleaflet/js/global.js
@@ -498,12 +498,14 @@
navigator.sendBeacon(url, '');
};
this.close = function() {
+ var oldState = this.readyState;
console.debug('proxy: close socket');
this.readyState = 3;
this.onclose();
clearInterval(this.waitInterval);
this.waitInterval = undefined;
- this.sendCloseMsg(this.unloading);
+ if (oldState === 1) // was open
+ this.sendCloseMsg(this.unloading);
this.sessionId = 'open';
};
this.setUnloading = function() {
commit 9f12876417c04442f309452afea789ea0d242a3a
Author: Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Mon Jun 8 14:43:04 2020 +0100
Commit: Andras Timar <andras.timar at collabora.com>
CommitDate: Mon Jun 8 22:21:58 2020 +0200
Proxy: shutdown socket with error if we can't find documentbroker.
Also called if/as/when the document is unloading as you connect.
Change-Id: I494dc207219298e07fba664cd2cbdd5d5b8ac889
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95809
Tested-by: Jenkins
Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 39365cbe7..12898f506 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -3016,6 +3016,21 @@ private:
});
});
}
+ else
+ {
+ auto streamSocket = std::static_pointer_cast<StreamSocket>(disposition.getSocket());
+ LOG_ERR("Failed to find document");
+ // badness occurred:
+ std::ostringstream oss;
+ oss << "HTTP/1.1 400\r\n"
+ << "Date: " << Util::getHttpTimeNow() << "\r\n"
+ << "User-Agent: LOOLWSD WOPI Agent\r\n"
+ << "Content-Length: \r\n"
+ << "\r\n";
+ // FIXME: send docunloading & re-try on client ?
+ streamSocket->send(oss.str());
+ streamSocket->shutdown();
+ }
}
void handleClientWsUpgrade(const Poco::Net::HTTPRequest& request,
commit cc428e99a7aca82d67f97c5efd36ab4d631e9730
Author: Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Wed Jun 3 17:14:03 2020 +0100
Commit: Andras Timar <andras.timar at 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 <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Michael Meeks <michael.meeks at 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.masei at 1and1.ro>
AuthorDate: Fri May 29 14:32:04 2020 +0300
Commit: Andras Timar <andras.timar at 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 <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Michael Meeks <michael.meeks at 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.vajngerl at collabora.co.uk>
AuthorDate: Thu Jun 4 12:13:17 2020 +0200
Commit: Andras Timar <andras.timar at 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 <lpranam at collabora.com>
AuthorDate: Wed Jun 3 16:45:24 2020 +0530
Commit: Andras Timar <andras.timar at 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 <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Andras Timar <andras.timar at 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.meeks at collabora.com>
AuthorDate: Wed Jun 3 17:13:40 2020 +0100
Commit: Andras Timar <andras.timar at 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 <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Michael Meeks <michael.meeks at 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.meeks at collabora.com>
AuthorDate: Wed Jun 3 16:06:10 2020 +0100
Commit: Andras Timar <andras.timar at 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 <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
diff --git a/net/Socket.cpp b/net/Socket.cpp
index e388d412a..ee76a5a1a 100644
--- a/net/Socket.cpp
+++ b/net/Socket.cpp
@@ -1022,14 +1022,19 @@ namespace HttpHelper
}
}
- void sendFile(const std::shared_ptr<StreamSocket>& socket,
- const std::string& path,
- const std::string& mediaType,
- Poco::Net::HTTPResponse& response,
- const bool noCache,
- const bool deflate,
- const bool headerOnly)
+ void sendFileAndShutdown(const std::shared_ptr<StreamSocket>& socket,
+ const std::string& path,
+ const std::string& mediaType,
+ Poco::Net::HTTPResponse *optResponse,
+ const bool noCache,
+ const bool deflate,
+ const bool headerOnly)
{
+ Poco::Net::HTTPResponse *response = optResponse;
+ Poco::Net::HTTPResponse localResponse;
+ if (!response)
+ response = &localResponse;
+
struct stat st;
if (stat(path.c_str(), &st) != 0)
{
@@ -1040,16 +1045,16 @@ namespace HttpHelper
if (!noCache)
{
// 60 * 60 * 24 * 128 (days) = 11059200
- response.set("Cache-Control", "max-age=11059200");
- response.set("ETag", "\"" LOOLWSD_VERSION_HASH "\"");
+ response->set("Cache-Control", "max-age=11059200");
+ response->set("ETag", "\"" LOOLWSD_VERSION_HASH "\"");
}
else
{
- response.set("Cache-Control", "no-cache");
+ response->set("Cache-Control", "no-cache");
}
- response.setContentType(mediaType);
- response.add("X-Content-Type-Options", "nosniff");
+ response->setContentType(mediaType);
+ response->add("X-Content-Type-Options", "nosniff");
int bufferSize = std::min(st.st_size, (off_t)Socket::MaximumSendBufferSize);
if (st.st_size >= socket->getSendBufferSize())
@@ -1063,24 +1068,25 @@ namespace HttpHelper
// IE/Edge before enabling the deflate again
if (!deflate || true)
{
- response.setContentLength(st.st_size);
+ response->setContentLength(st.st_size);
LOG_TRC('#' << socket->getFD() << ": Sending " <<
(headerOnly ? "header for " : "") << " file [" << path << "].");
- socket->send(response);
+ socket->send(*response);
if (!headerOnly)
sendUncompressedFileContent(socket, path, bufferSize);
}
else
{
- response.set("Content-Encoding", "deflate");
+ response->set("Content-Encoding", "deflate");
LOG_TRC('#' << socket->getFD() << ": Sending " <<
(headerOnly ? "header for " : "") << " file [" << path << "].");
- socket->send(response);
+ socket->send(*response);
if (!headerOnly)
sendDeflatedFileContent(socket, path, st.st_size);
}
+ socket->shutdown();
}
}
diff --git a/net/Socket.hpp b/net/Socket.hpp
index 7e011bb9d..55cfe05bb 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -1278,10 +1278,10 @@ enum class WSOpCode : unsigned char {
namespace HttpHelper
{
- /// Sends file as HTTP response.
- void sendFile(const std::shared_ptr<StreamSocket>& socket, const std::string& path, const std::string& mediaType,
- Poco::Net::HTTPResponse& response, bool noCache = false, bool deflate = false,
- const bool headerOnly = false);
+ /// Sends file as HTTP response and shutdown the socket.
+ void sendFileAndShutdown(const std::shared_ptr<StreamSocket>& socket, const std::string& path, const std::string& mediaType,
+ Poco::Net::HTTPResponse *optResponse = nullptr, bool noCache = false, bool deflate = false,
+ const bool headerOnly = false);
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/test/UnitWOPITemplate.cpp b/test/UnitWOPITemplate.cpp
index 2ebb1a28c..83e1bb64f 100644
--- a/test/UnitWOPITemplate.cpp
+++ b/test/UnitWOPITemplate.cpp
@@ -94,9 +94,7 @@ public:
{
LOG_INF("Fake wopi host request, handling template GetFile: " << uriReq.getPath());
- Poco::Net::HTTPResponse response;
- HttpHelper::sendFile(socket, TDOC "/test.ott", "", response);
- socket->shutdown();
+ HttpHelper::sendFileAndShutdown(socket, TDOC "/test.ott", "");
return true;
}
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index d534a2656..904867b18 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -1271,7 +1271,7 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt
if (!fileName.empty())
response.set("Content-Disposition", "attachment; filename=\"" + fileName + '"');
- HttpHelper::sendFile(_saveAsSocket, encodedFilePath, mimeType, response);
+ HttpHelper::sendFileAndShutdown(_saveAsSocket, encodedFilePath, mimeType, &response);
}
// Conversion is done, cleanup this fake session.
diff --git a/wsd/FileServer.cpp b/wsd/FileServer.cpp
index 1c47b0f8d..a298adb6c 100644
--- a/wsd/FileServer.cpp
+++ b/wsd/FileServer.cpp
@@ -442,7 +442,7 @@ void FileServerRequestHandler::handleRequest(const HTTPRequest& request,
// Useful to not serve from memory sometimes especially during loleaflet development
// Avoids having to restart loolwsd everytime you make a change in loleaflet
const std::string filePath = Poco::Path(LOOLWSD::FileServerRoot, relPath).absolute().toString();
- HttpHelper::sendFile(socket, filePath, mimeType, response, noCache);
+ HttpHelper::sendFileAndShutdown(socket, filePath, mimeType, &response, noCache);
return;
}
#endif
@@ -470,6 +470,7 @@ void FileServerRequestHandler::handleRequest(const HTTPRequest& request,
(!gzip ? "un":"") << "compressed : file [" << relPath << "]: " << header);
socket->send(header);
socket->send(*content);
+ // shutdown by caller
}
}
catch (const Poco::Net::NotAuthenticatedException& exc)
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index db7bfbaef..c672932f0 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2485,13 +2485,9 @@ private:
std::string mimeType = "image/vnd.microsoft.icon";
std::string faviconPath = Path(Application::instance().commandPath()).parent().toString() + "favicon.ico";
if (!File(faviconPath).exists())
- {
faviconPath = LOOLWSD::FileServerRoot + "/favicon.ico";
- }
- Poco::Net::HTTPResponse response;
- HttpHelper::sendFile(socket, faviconPath, mimeType, response);
- socket->shutdown();
+ HttpHelper::sendFileAndShutdown(socket, faviconPath, mimeType);
}
void handleWopiDiscoveryRequest(const RequestDetails &requestDetails,
@@ -2900,7 +2896,7 @@ private:
try
{
- HttpHelper::sendFile(socket, filePath.toString(), contentType, response);
+ HttpHelper::sendFileAndShutdown(socket, filePath.toString(), contentType, &response);
}
catch (const Exception& exc)
{
commit 89a8713b694a1b8b886ec374e67804d8752889da
Author: Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Wed Jun 3 16:06:52 2020 +0200
Commit: Andras Timar <andras.timar at 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 <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Miklos Vajna <vmiklos at 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.nakashian at collabora.co.uk>
AuthorDate: Mon Jun 1 08:18:13 2020 -0400
Commit: Andras Timar <andras.timar at 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 <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Ashod Nakashian <ashnakash at 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.nakashian at collabora.co.uk>
AuthorDate: Mon May 25 10:52:37 2020 -0400
Commit: Andras Timar <andras.timar at 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 <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Ashod Nakashian <ashnakash at 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.nakashian at collabora.co.uk>
AuthorDate: Mon May 25 07:51:04 2020 -0400
Commit: Andras Timar <andras.timar at 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 <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Ashod Nakashian <ashnakash at 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.nakashian at collabora.co.uk>
AuthorDate: Tue Jun 2 03:44:08 2020 -0400
Commit: Andras Timar <andras.timar at 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 <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Ashod Nakashian <ashnakash at 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.nakashian at collabora.co.uk>
AuthorDate: Mon Jun 1 23:21:22 2020 -0400
Commit: Andras Timar <andras.timar at 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 <jenkinscollaboraoffice at gmail.com>
Tested-by: Jenkins
Reviewed-by: Ashod Nakashian <ashnakash at 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)
{
... etc. - the rest is truncated
More information about the Libreoffice-commits
mailing list