[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-1-9' - 84 commits - loleaflet/dist loleaflet/po loleaflet/src loolwsd/ChildSession.cpp loolwsd/ChildSession.hpp loolwsd/ClientSession.cpp loolwsd/ClientSession.hpp loolwsd/Common.hpp loolwsd/Connect.cpp loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/Exceptions.hpp loolwsd/IoUtil.cpp loolwsd/IoUtil.hpp loolwsd/Log.cpp loolwsd/Log.hpp loolwsd/LOOLForKit.cpp loolwsd/LOOLKit.cpp loolwsd/loolmap.c loolwsd/LOOLProtocol.hpp loolwsd/LOOLSession.cpp loolwsd/LOOLSession.hpp loolwsd/LOOLWSD.cpp loolwsd/PrisonerSession.cpp loolwsd/PrisonerSession.hpp loolwsd/protocol.txt loolwsd/Storage.cpp loolwsd/Storage.hpp loolwsd/test loolwsd/Unit.hpp loolwsd/UserMessages.hpp loolwsd/Util.cpp loolwsd/Util.hpp
Henry Castro
hcastro at collabora.com
Tue Oct 25 12:59:59 UTC 2016
loleaflet/dist/errormessages.js | 6
loleaflet/dist/spreadsheet.css | 2
loleaflet/dist/toolbar/toolbar.js | 28 -
loleaflet/po/templates/loleaflet-help.pot | 2
loleaflet/po/templates/loleaflet-ui.pot | 97 ++--
loleaflet/src/control/Control.ColumnHeader.js | 4
loleaflet/src/control/Control.Header.js | 17
loleaflet/src/control/Control.Menubar.js | 1
loleaflet/src/control/Control.RowHeader.js | 6
loleaflet/src/control/Parts.js | 3
loleaflet/src/core/Socket.js | 20
loleaflet/src/layer/marker/Cursor.js | 10
loleaflet/src/layer/tile/TileLayer.js | 90 +++-
loleaflet/src/map/handler/Map.SlideShow.js | 2
loolwsd/ChildSession.cpp | 6
loolwsd/ChildSession.hpp | 1
loolwsd/ClientSession.cpp | 122 +++--
loolwsd/ClientSession.hpp | 54 +-
loolwsd/Common.hpp | 11
loolwsd/Connect.cpp | 6
loolwsd/DocumentBroker.cpp | 212 ++++++---
loolwsd/DocumentBroker.hpp | 69 ++-
loolwsd/Exceptions.hpp | 6
loolwsd/IoUtil.cpp | 10
loolwsd/IoUtil.hpp | 8
loolwsd/LOOLForKit.cpp | 106 +++-
loolwsd/LOOLKit.cpp | 60 --
loolwsd/LOOLProtocol.hpp | 39 +
loolwsd/LOOLSession.cpp | 5
loolwsd/LOOLSession.hpp | 5
loolwsd/LOOLWSD.cpp | 580 ++++++++++----------------
loolwsd/Log.cpp | 12
loolwsd/Log.hpp | 8
loolwsd/PrisonerSession.cpp | 10
loolwsd/PrisonerSession.hpp | 3
loolwsd/Storage.cpp | 108 ++--
loolwsd/Storage.hpp | 92 ++--
loolwsd/Unit.hpp | 4
loolwsd/UserMessages.hpp | 4
loolwsd/Util.cpp | 5
loolwsd/Util.hpp | 16
loolwsd/loolmap.c | 253 +++++------
loolwsd/protocol.txt | 10
loolwsd/test/UnitAdmin.cpp | 6
loolwsd/test/UnitStorage.cpp | 4
loolwsd/test/countloolkits.hpp | 14
loolwsd/test/helpers.hpp | 36 +
loolwsd/test/httpcrashtest.cpp | 23 -
loolwsd/test/httpwserror.cpp | 71 ++-
loolwsd/test/httpwstest.cpp | 15
loolwsd/test/run_unit.sh.in | 4
loolwsd/test/test.cpp | 73 +--
52 files changed, 1344 insertions(+), 1015 deletions(-)
New commits:
commit fc46e846d4d369ec1230c16249188ff7a9816b38
Author: Henry Castro <hcastro at collabora.com>
Date: Thu Oct 20 09:31:27 2016 -0400
loleaflet: refocus map due drop down insert table
(cherry picked from commit e189f60ad4e2f2fe00fb45268b481f9e97121b43)
diff --git a/loleaflet/dist/toolbar/toolbar.js b/loleaflet/dist/toolbar/toolbar.js
index 33a172f..5102811 100644
--- a/loleaflet/dist/toolbar/toolbar.js
+++ b/loleaflet/dist/toolbar/toolbar.js
@@ -265,6 +265,8 @@ function insertTable() {
' }, "Rows": { "type": "long","value": '
+ row + ' }}';
map._socket.sendMessage(msg);
+ // refocus map due popup
+ map.focus();
}
}, '.col');
commit e5b3525b2f26b7b4b8e06fddc12eaa45b835d2af
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Thu Oct 20 17:21:50 2016 +0530
loleaflet: Click user item to go to its cursor
Change-Id: Iac87da20cfe422000eb9a32ccad50e8483637616
(cherry picked from commit 49f4e696e517b5f278aab465eebccaae9d8cc675)
diff --git a/loleaflet/dist/toolbar/toolbar.js b/loleaflet/dist/toolbar/toolbar.js
index c3379d0..33a172f 100644
--- a/loleaflet/dist/toolbar/toolbar.js
+++ b/loleaflet/dist/toolbar/toolbar.js
@@ -1292,11 +1292,24 @@ map.on('statusindicator', function (e) {
}
});
+function onUseritemClicked(e) {
+ var viewId = parseInt(e.currentTarget.id.replace('user-', ''));
+
+ if (map._docLayer) {
+ if (map.getDocType() === 'spreadsheet') {
+ map._docLayer.goToCellViewCursor(viewId);
+ } else if (map.getDocType() === 'text') {
+ map._docLayer.goToViewCursor(viewId);
+ }
+ }
+}
+
function getUserItem(viewId, userName, color) {
- var html = '<tr class="useritem" id="user-' + viewId + '">' +
+ var html = '<tr class="useritem" id="user-' + viewId + '" onclick="onUseritemClicked(event)">' +
'<td class=usercolor style="background-color: ' + color +';"></td>' +
'<td class="username loleaflet-font">' + userName + '</td>' +
- '</tr>';
+ '</tr>';
+
return html;
}
var nUsers = _('%n users');
diff --git a/loleaflet/src/layer/marker/Cursor.js b/loleaflet/src/layer/marker/Cursor.js
index 1335e8e..0a5e622 100644
--- a/loleaflet/src/layer/marker/Cursor.js
+++ b/loleaflet/src/layer/marker/Cursor.js
@@ -64,6 +64,16 @@ L.Cursor = L.Layer.extend({
}
},
+ showCursorHeader: function() {
+ if (this._cursorHeader) {
+ L.DomUtil.setStyle(this._cursorHeader, 'visibility', 'visible');
+
+ setTimeout(L.bind(function() {
+ L.DomUtil.setStyle(this._cursorHeader, 'visibility', 'hidden');
+ }, this), this.options.headerTimeout);
+ }
+ },
+
_initLayout: function () {
this._container = L.DomUtil.create('div', 'leaflet-cursor-container');
if (this.options.header) {
diff --git a/loleaflet/src/layer/tile/TileLayer.js b/loleaflet/src/layer/tile/TileLayer.js
index aa3b330..c26dd47 100644
--- a/loleaflet/src/layer/tile/TileLayer.js
+++ b/loleaflet/src/layer/tile/TileLayer.js
@@ -704,6 +704,43 @@ L.TileLayer = L.GridLayer.extend({
}
},
+ goToCellViewCursor: function(viewId) {
+ if (this._cellViewCursors[viewId] && !this._isEmptyRectangle(this._cellViewCursors[viewId].bounds)) {
+ if (!this._map.getBounds().contains(this._cellViewCursors[viewId].bounds)) {
+ var mapBounds = this._map.getBounds();
+ var scrollX = 0;
+ var scrollY = 0;
+ var spacingX = Math.abs(this._cellViewCursors[viewId].bounds.getEast() - this._cellViewCursors[viewId].bounds.getWest()) / 4.0;
+ var spacingY = Math.abs(this._cellViewCursors[viewId].bounds.getSouth() - this._cellViewCursors[viewId].bounds.getNorth()) / 4.0;
+ if (this._cellViewCursors[viewId].bounds.getWest() < mapBounds.getWest()) {
+ scrollX = this._cellViewCursors[viewId].bounds.getWest() - mapBounds.getWest() - spacingX;
+ } else if (this._cellViewCursors[viewId].bounds.getEast() > mapBounds.getEast()) {
+ scrollX = this._cellViewCursors[viewId].bounds.getEast() - mapBounds.getEast() + spacingX;
+ }
+
+ if (this._cellViewCursors[viewId].bounds.getNorth() > mapBounds.getNorth()) {
+ scrollY = this._cellViewCursors[viewId].bounds.getNorth() - mapBounds.getNorth() + spacingY;
+ } else if (this._cellViewCursors[viewId].bounds.getSouth() < mapBounds.getSouth()) {
+ scrollY = this._cellViewCursors[viewId].bounds.getSouth() - mapBounds.getSouth() - spacingY;
+ }
+
+ if (scrollX !== 0 || scrollY !== 0) {
+ var newCenter = mapBounds.getCenter();
+ newCenter.lat += scrollX;
+ newCenter.lat += scrollY;
+ var center = this._map.project(newCenter);
+ center = center.subtract(this._map.getSize().divideBy(2));
+ center.x = Math.round(center.x < 0 ? 0 : center.x);
+ center.y = Math.round(center.y < 0 ? 0 : center.y);
+ this._map.fire('scrollto', {x: center.x, y: center.y});
+ }
+ }
+
+ var borderColor = L.LOUtil.rgbToHex(this._map.getViewColor(viewId));
+ this._cellViewCursors[viewId].marker.bindPopup(this._map.getViewName(viewId), {autoClose: false, autoPan: false, borderColor: borderColor});
+ }
+ },
+
_onViewCursorVisibleMsg: function(textMsg) {
textMsg = textMsg.substring('viewcursorvisible:'.length + 1);
var obj = JSON.parse(textMsg);
@@ -1267,6 +1304,27 @@ L.TileLayer = L.GridLayer.extend({
}
},
+ goToViewCursor: function(viewId) {
+ if (viewId === this._viewId) {
+ this._onUpdateCursor();
+ return;
+ }
+
+ if (this._viewCursors[viewId] && this._viewCursors[viewId].visible && !this._isEmptyRectangle(this._viewCursors[viewId].bounds)) {
+ if (!this._map.getBounds().contains(this._viewCursors[viewId].bounds)) {
+ var viewCursorPos = this._viewCursors[viewId].bounds.getNorthWest();
+ var center = this._map.project(viewCursorPos);
+ center = center.subtract(this._map.getSize().divideBy(2));
+ center.x = Math.round(center.x < 0 ? 0 : center.x);
+ center.y = Math.round(center.y < 0 ? 0 : center.y);
+
+ this._map.fire('scrollto', {x: center.x, y: center.y});
+ }
+
+ this._viewCursors[viewId].marker.showCursorHeader();
+ }
+ },
+
_onUpdateTextViewSelection: function (viewId) {
viewId = parseInt(viewId);
var viewPolygons = this._viewSelections[viewId].polygons;
@@ -1446,7 +1504,7 @@ L.TileLayer = L.GridLayer.extend({
}
}
else {
- var spacingX = Math.abs((this._cellCursor.getEast() - this._cellCursor.getWest())) / 4.0;
+ var spacingX = Math.abs(this._cellCursor.getEast() - this._cellCursor.getWest()) / 4.0;
var spacingY = Math.abs((this._cellCursor.getSouth() - this._cellCursor.getNorth())) / 4.0;
if (horizontalDirection === -1 && this._cellCursor.getWest() < mapBounds.getWest()) {
scrollX = this._cellCursor.getWest() - mapBounds.getWest() - spacingX;
commit 3a4e60a08caa155878ec84390a43f2622d76caae
Author: Andras Timar <andras.timar at collabora.com>
Date: Thu Oct 20 11:03:24 2016 +0200
loolwsd: fix error: declaration of ‘isLoaded’ shadows a member of 'this' [-Werror=shadow]
(cherry picked from commit 0cdc9d70b501749de5d836b7043f5617583d105f)
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 3960ef2..9ae52af 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -439,8 +439,8 @@ size_t DocumentBroker::addSession(std::shared_ptr<ClientSession>& session)
_lastEditableSession = false;
_markToDestroy = false;
- bool isLoaded = load(id, std::to_string(_childProcess->getPid()));
- if (!isLoaded)
+ bool loaded = load(id, std::to_string(_childProcess->getPid()));
+ if (!loaded)
{
Log::error("Error loading document with URI [" + session->getPublicUri().toString() + "].");
throw;
commit 7bba14bf50da07d804412d7b4372e2753d57aaf3
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Wed Oct 19 21:08:33 2016 +0530
loleaflet: Disable these toolbar buttons too, in readonly mode
Change-Id: I8bc661dd3cfd689c7c7ded367deacdba853153e4
(cherry picked from commit 9f69359b33c40a2c196f43bee4c91cfc0d5b0a31)
diff --git a/loleaflet/dist/toolbar/toolbar.js b/loleaflet/dist/toolbar/toolbar.js
index 2489878..c3379d0 100644
--- a/loleaflet/dist/toolbar/toolbar.js
+++ b/loleaflet/dist/toolbar/toolbar.js
@@ -472,7 +472,8 @@ var formatButtons = {
'annotation': true, 'inserttable': true,
'fontcolor': true, 'backcolor': true, 'bullet': true, 'numbering': true,
'alignleft': true, 'alignhorizontal': true, 'alignright': true, 'alignblock': true,
- 'incrementindent': true, 'decrementindent': true, 'insertgraphic': true
+ 'incrementindent': true, 'decrementindent': true, 'insertgraphic': true,
+ 'insertfootnote': true, 'repair': true
};
var userJoinedPopupMessage = '<div>' + _('%user has joined') + '</div>';
commit 97586fe808de7d4580fd1375cdb00156703ec255
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Wed Oct 19 20:22:53 2016 +0530
Handle WOPI's UserCanWrite to determine readonliness
permission= parameter in URL is still supported, but overridden
by UserCanWrite parameter.
Also, introduce a new protocol message, perm: which dictates
loleaflet about the permission rather than the other way around
(only in case of WOPI)
It is to be noted that by default loolwsd assumes very
restrictive permissions, so not providing UserCanWrite in WOPI
implementation by a WOPI host would lead to opening of only
readonly session.
Change-Id: I2013c1661fd491c79bb367a41e1a7036fa03f984
(cherry picked from commit 1be2a78564afd84172aba44edc4bab42cd30c34d)
diff --git a/loleaflet/src/control/Parts.js b/loleaflet/src/control/Parts.js
index cdde4d1..1139594 100644
--- a/loleaflet/src/control/Parts.js
+++ b/loleaflet/src/control/Parts.js
@@ -295,6 +295,9 @@ L.Map.include({
},
getDocType: function () {
+ if (!this._docLayer)
+ return null;
+
return this._docLayer._docType;
}
});
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index 07126bb..488aeda 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -154,6 +154,19 @@ L.Socket = L.Class.extend({
lokitVersionObj.ProductVersion + lokitVersionObj.ProductExtension.replace('.10.','-') +
' (git hash: ' + lokitVersionObj.BuildId.substring(0, 7) + ')');
}
+ else if (textMsg.startsWith('perm:')) {
+ var perm = textMsg.substring('perm:'.length);
+
+ // This message is often received very early before doclayer is initialized
+ // Change options.permission so that when docLayer is initialized, it
+ // picks up the new value of permission rather than something else
+ this._map.options.permission = 'readonly';
+ // Lets also try to set the permission ourself since this can well be received
+ // after doclayer is initialized. There's no harm to call this in any case.
+ this._map.setPermission(perm);
+
+ return;
+ }
else if (textMsg.startsWith('error:') && command.errorCmd === 'internal') {
this._map._fatal = true;
if (command.errorKind === 'diskfull') {
diff --git a/loolwsd/ClientSession.cpp b/loolwsd/ClientSession.cpp
index 2aad600..ee5510b 100644
--- a/loolwsd/ClientSession.cpp
+++ b/loolwsd/ClientSession.cpp
@@ -368,4 +368,11 @@ bool ClientSession::forwardToChild(const std::string& message,
return docBroker->forwardToChild(getId(), message);
}
+void ClientSession::setReadOnly()
+{
+ _isReadOnly = true;
+ // Also inform the client
+ sendTextFrame("perm: readonly");
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/ClientSession.hpp b/loolwsd/ClientSession.hpp
index 201e7c1..6ba901b 100644
--- a/loolwsd/ClientSession.hpp
+++ b/loolwsd/ClientSession.hpp
@@ -30,6 +30,7 @@ public:
virtual ~ClientSession();
+ void setReadOnly();
bool isReadOnly() const { return _isReadOnly; }
void setPeer(const std::shared_ptr<PrisonerSession>& peer) { _peer = peer; }
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 9ce4488..3960ef2 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -215,6 +215,11 @@ bool DocumentBroker::load(const std::string& sessionId, const std::string& jailI
}
Log::debug("Setting username of the session to: " + fileInfo._userName);
it->second->setUserName(fileInfo._userName);
+ if (!fileInfo._canWrite)
+ {
+ Log::debug("Setting the session as readonly");
+ it->second->setReadOnly();
+ }
// Lets load the document now
if (_storage->isLoaded())
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 312d7b6..df9cf2e 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -796,6 +796,9 @@ private:
isReadOnly = param.second == "readonly";
}
+ // In case of WOPI and if this session is not set as readonly, it might be set so
+ // later after making a call to WOPI host which tells us the permission on files
+ // (UserCanWrite param)
auto session = std::make_shared<ClientSession>(id, ws, docBroker, uriPublic, isReadOnly);
// Above this point exceptions are safe and will auto-cleanup.
diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp
index d500aae..f12c056 100644
--- a/loolwsd/Storage.cpp
+++ b/loolwsd/Storage.cpp
@@ -185,7 +185,7 @@ StorageBase::FileInfo LocalStorage::getFileInfo(const Poco::URI& uriPublic)
const auto file = Poco::File(path);
const auto lastModified = file.getLastModified();
const auto size = file.getSize();
- return FileInfo({filename, lastModified, size, "localhost", std::string("Local Host #") + std::to_string(_localStorageId)});
+ return FileInfo({filename, lastModified, size, "localhost", std::string("Local Host #") + std::to_string(_localStorageId), true});
}
std::string LocalStorage::loadStorageFileToLocal()
@@ -293,6 +293,7 @@ StorageBase::FileInfo WopiStorage::getFileInfo(const Poco::URI& uriPublic)
size_t size = 0;
std::string userId;
std::string userName;
+ bool canWrite = false;
std::string resMsg;
Poco::StreamCopier::copyToString(rs, resMsg);
@@ -313,10 +314,12 @@ StorageBase::FileInfo WopiStorage::getFileInfo(const Poco::URI& uriPublic)
userId = (userIdVar.isString() ? userIdVar.toString() : "");
const auto userNameVar = object->get("UserFriendlyName");
userName = (userNameVar.isString() ? userNameVar.toString() : "anonymous");
+ const auto canWriteVar = object->get("UserCanWrite");
+ canWrite = canWriteVar.isString() ? (canWriteVar.toString() == "true") : false;
}
// WOPI doesn't support file last modified time.
- _fileInfo = FileInfo({filename, Poco::Timestamp(), size, userId, userName});
+ _fileInfo = FileInfo({filename, Poco::Timestamp(), size, userId, userName, canWrite});
return _fileInfo;
}
@@ -371,6 +374,7 @@ std::string WopiStorage::loadStorageFileToLocal()
bool WopiStorage::saveLocalFileToStorage(const Poco::URI& uriPublic)
{
Log::info("Uploading URI [" + uriPublic.toString() + "] from [" + _jailedFilePath + "].");
+ // TODO: Check if this URI has write permission (canWrite = true)
const auto size = getFileSize(_jailedFilePath);
Poco::URI uriObject(uriPublic);
@@ -406,7 +410,7 @@ StorageBase::FileInfo WebDAVStorage::getFileInfo(const Poco::URI& uriPublic)
{
Log::debug("Getting info for webdav uri [" + uriPublic.toString() + "].");
assert(false && "Not Implemented!");
- return FileInfo({"bazinga", Poco::Timestamp(), 0, "admin", "admin"});
+ return FileInfo({"bazinga", Poco::Timestamp(), 0, "admin", "admin", false});
}
std::string WebDAVStorage::loadStorageFileToLocal()
diff --git a/loolwsd/Storage.hpp b/loolwsd/Storage.hpp
index 2e53b6a..163443c 100644
--- a/loolwsd/Storage.hpp
+++ b/loolwsd/Storage.hpp
@@ -35,12 +35,14 @@ public:
const Poco::Timestamp& modifiedTime,
size_t size,
const std::string& userId,
- const std::string& userName)
+ const std::string& userName,
+ const bool canWrite)
: _filename(filename),
_modifiedTime(modifiedTime),
_size(size),
_userId(userId),
- _userName(userName)
+ _userName(userName),
+ _canWrite(canWrite)
{
}
@@ -54,6 +56,7 @@ public:
size_t _size;
std::string _userId;
std::string _userName;
+ bool _canWrite;
};
/// localStorePath the absolute root path of the chroot.
@@ -64,7 +67,7 @@ public:
_uri(uri),
_localStorePath(localStorePath),
_jailPath(jailPath),
- _fileInfo("", Poco::Timestamp(), 0, "", ""),
+ _fileInfo("", Poco::Timestamp(), 0, "", "", false),
_isLoaded(false)
{
Log::debug("Storage ctor: " + uri.toString());
diff --git a/loolwsd/protocol.txt b/loolwsd/protocol.txt
index 51d608a..eb40f7a 100644
--- a/loolwsd/protocol.txt
+++ b/loolwsd/protocol.txt
@@ -337,6 +337,11 @@ stats: <key> <value>
Contains statistical data. Eg: 'stats: wopiduration 5' means that
wopi calls made in loading of document took 5 seconds.
+perm: <permission>
+
+ <permission> can be one of 'edit', 'view', 'readonly'. Client must
+ change the UI accordingly (disabling buttons etc.)
+
child -> parent
===============
commit 5bc050a5d540688ec141aab24df885411bb01af5
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Wed Oct 19 15:51:29 2016 +0530
loolwsd: Fix a crash due to invalid fileinfo
Throw if document load fails. Ignoring such a scenario would lead
to crash later in the document load process.
Change-Id: Ie80fc4f26e08e4920d4c04726f947edd2837a0cf
(cherry picked from commit ef32f300171d7e9824d86cf32f0e0954448b0014)
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 3adc1ad..9ce4488 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -434,7 +434,12 @@ size_t DocumentBroker::addSession(std::shared_ptr<ClientSession>& session)
_lastEditableSession = false;
_markToDestroy = false;
- load(id, std::to_string(_childProcess->getPid()));
+ bool isLoaded = load(id, std::to_string(_childProcess->getPid()));
+ if (!isLoaded)
+ {
+ Log::error("Error loading document with URI [" + session->getPublicUri().toString() + "].");
+ throw;
+ }
}
catch (const StorageSpaceLowException&)
{
commit fb1e2fef1158e38c45f2011f920dc188fab5953c
Author: Andras Timar <andras.timar at collabora.com>
Date: Wed Oct 19 11:53:18 2016 +0200
loleaflet: updated port files
(cherry picked from commit 1f2be910098b2920bba6915d6b8ba4a640fa4d03)
diff --git a/loleaflet/po/templates/loleaflet-help.pot b/loleaflet/po/templates/loleaflet-help.pot
index b05eab5..4a13fda 100644
--- a/loleaflet/po/templates/loleaflet-help.pot
+++ b/loleaflet/po/templates/loleaflet-help.pot
@@ -3,7 +3,7 @@ msgid ""
msgstr ""
"Project-Id-Version: PACKAGE VERSION\n"
"Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2016-10-11 12:11+0200\n"
+"POT-Creation-Date: 2016-10-19 11:51+0200\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL at ADDRESS>\n"
"Language-Team: LANGUAGE <LL at li.org>\n"
diff --git a/loleaflet/po/templates/loleaflet-ui.pot b/loleaflet/po/templates/loleaflet-ui.pot
index 307e66f..e12b9c9 100644
--- a/loleaflet/po/templates/loleaflet-ui.pot
+++ b/loleaflet/po/templates/loleaflet-ui.pot
@@ -8,7 +8,7 @@ msgid ""
msgstr ""
"Project-Id-Version: PACKAGE VERSION\n"
"Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2016-10-11 12:11+0200\n"
+"POT-Creation-Date: 2016-10-19 11:51+0200\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL at ADDRESS>\n"
"Language-Team: LANGUAGE <LL at li.org>\n"
@@ -197,8 +197,8 @@ msgstr ""
#: dist/errormessages.js:1
msgid ""
-"Wrong WOPISrc, usage: WOPISrc=valid encoded URI, or file_path, usage: "
-"file_path=/path/to/doc/"
+"No disk space left on server, please contact the server administrator to "
+"continue."
msgstr ""
#: dist/errormessages.js:2
@@ -209,18 +209,30 @@ msgstr ""
#: dist/errormessages.js:3
msgid ""
-"No disk space left on server, please contact the server administrator to "
-"continue."
-msgstr ""
-
-#: dist/errormessages.js:4
-msgid ""
"This development build is limited to %0 documents, and %1 connections - to "
"avoid the impression that it is suitable for deployment in large "
"enterprises. To find out more about deploying and scaling %2 checkout: <br/"
"><a href=\"%3\">%3</a>."
msgstr ""
+#: dist/errormessages.js:4
+msgid ""
+"Service is unavailable. Please try again later and report to your "
+"administrator if the issue persists."
+msgstr ""
+
+#: dist/errormessages.js:5
+msgid ""
+"Unauthorized WOPI host. Please try again later and report to your "
+"administrator if the issue persists."
+msgstr ""
+
+#: dist/errormessages.js:6
+msgid ""
+"Wrong WOPISrc, usage: WOPISrc=valid encoded URI, or file_path, usage: "
+"file_path=/path/to/doc/"
+msgstr ""
+
#: dist/toolbar/toolbar.js:150
msgid "Are you sure you want to delete this page?"
msgstr ""
@@ -415,7 +427,7 @@ msgstr ""
msgid "Cancel the search"
msgstr ""
-#: dist/toolbar/toolbar.js:462 dist/toolbar/toolbar.js:1328
+#: dist/toolbar/toolbar.js:462 dist/toolbar/toolbar.js:1326
msgid "No users"
msgstr ""
@@ -428,17 +440,17 @@ msgid "Next page"
msgstr ""
#: dist/toolbar/toolbar.js:467 src/control/Control.Menubar.js:41
-#: src/control/Control.Menubar.js:86 src/control/Control.Menubar.js:135
+#: src/control/Control.Menubar.js:86 src/control/Control.Menubar.js:136
msgid "Reset zoom"
msgstr ""
#: dist/toolbar/toolbar.js:468 src/control/Control.Menubar.js:40
-#: src/control/Control.Menubar.js:85 src/control/Control.Menubar.js:134
+#: src/control/Control.Menubar.js:85 src/control/Control.Menubar.js:135
msgid "Zoom out"
msgstr ""
#: dist/toolbar/toolbar.js:470 src/control/Control.Menubar.js:39
-#: src/control/Control.Menubar.js:84 src/control/Control.Menubar.js:133
+#: src/control/Control.Menubar.js:84 src/control/Control.Menubar.js:134
msgid "Zoom in"
msgstr ""
@@ -450,7 +462,7 @@ msgstr ""
msgid "%user has left"
msgstr ""
-#: dist/toolbar/toolbar.js:625 dist/toolbar/toolbar.js:1117
+#: dist/toolbar/toolbar.js:625 dist/toolbar/toolbar.js:1115
msgid "Size"
msgstr ""
@@ -554,39 +566,39 @@ msgstr ""
msgid "Number of Slides"
msgstr ""
-#: dist/toolbar/toolbar.js:941
+#: dist/toolbar/toolbar.js:939
msgid "Document saved"
msgstr ""
-#: dist/toolbar/toolbar.js:1100
+#: dist/toolbar/toolbar.js:1098
msgid "Style"
msgstr ""
-#: dist/toolbar/toolbar.js:1112
+#: dist/toolbar/toolbar.js:1110
msgid "Font"
msgstr ""
-#: dist/toolbar/toolbar.js:1136
+#: dist/toolbar/toolbar.js:1134
msgid "Previous slide"
msgstr ""
-#: dist/toolbar/toolbar.js:1137
+#: dist/toolbar/toolbar.js:1135
msgid "Next slide"
msgstr ""
-#: dist/toolbar/toolbar.js:1305
+#: dist/toolbar/toolbar.js:1303
msgid "Layout"
msgstr ""
-#: dist/toolbar/toolbar.js:1319
+#: dist/toolbar/toolbar.js:1317
msgid "%n users"
msgstr ""
-#: dist/toolbar/toolbar.js:1326
+#: dist/toolbar/toolbar.js:1324
msgid "1 user"
msgstr ""
-#: dist/toolbar/toolbar.js:1351 src/control/Control.DocumentRepair.js:87
+#: dist/toolbar/toolbar.js:1349 src/control/Control.DocumentRepair.js:87
msgid "You"
msgstr ""
@@ -630,7 +642,7 @@ msgstr ""
msgid "Insert column before"
msgstr ""
-#: src/control/Control.ColumnHeader.js:45 src/control/Control.Menubar.js:141
+#: src/control/Control.ColumnHeader.js:45 src/control/Control.Menubar.js:142
msgid "Delete column"
msgstr ""
@@ -655,6 +667,7 @@ msgid "Index"
msgstr ""
#: src/control/Control.DocumentRepair.js:44 src/control/Control.Menubar.js:29
+#: src/control/Control.Menubar.js:127
msgid "Comment"
msgstr ""
@@ -764,12 +777,12 @@ msgid "Column break"
msgstr ""
#: src/control/Control.Menubar.js:37 src/control/Control.Menubar.js:82
-#: src/control/Control.Menubar.js:131
+#: src/control/Control.Menubar.js:132
msgid "View"
msgstr ""
#: src/control/Control.Menubar.js:37 src/control/Control.Menubar.js:82
-#: src/control/Control.Menubar.js:131
+#: src/control/Control.Menubar.js:132
msgid "Full screen"
msgstr ""
@@ -813,11 +826,11 @@ msgstr ""
msgid "Select"
msgstr ""
-#: src/control/Control.Menubar.js:52 src/control/Control.Menubar.js:128
+#: src/control/Control.Menubar.js:52 src/control/Control.Menubar.js:129
msgid "Row"
msgstr ""
-#: src/control/Control.Menubar.js:53 src/control/Control.Menubar.js:129
+#: src/control/Control.Menubar.js:53 src/control/Control.Menubar.js:130
msgid "Column"
msgstr ""
@@ -830,17 +843,17 @@ msgid "Merge cells"
msgstr ""
#: src/control/Control.Menubar.js:57 src/control/Control.Menubar.js:103
-#: src/control/Control.Menubar.js:143
+#: src/control/Control.Menubar.js:144
msgid "Help"
msgstr ""
#: src/control/Control.Menubar.js:57 src/control/Control.Menubar.js:103
-#: src/control/Control.Menubar.js:143
+#: src/control/Control.Menubar.js:144
msgid "Keyboard shortcuts"
msgstr ""
#: src/control/Control.Menubar.js:58 src/control/Control.Menubar.js:104
-#: src/control/Control.Menubar.js:144
+#: src/control/Control.Menubar.js:145
msgid "About"
msgstr ""
@@ -876,23 +889,23 @@ msgstr ""
msgid "Microsoft Excel (.xlsx)"
msgstr ""
-#: src/control/Control.Menubar.js:137
+#: src/control/Control.Menubar.js:138
msgid "Cells"
msgstr ""
-#: src/control/Control.Menubar.js:137
+#: src/control/Control.Menubar.js:138
msgid "Insert row"
msgstr ""
-#: src/control/Control.Menubar.js:138
+#: src/control/Control.Menubar.js:139
msgid "Insert column"
msgstr ""
-#: src/control/Control.Menubar.js:140 src/control/Control.RowHeader.js:43
+#: src/control/Control.Menubar.js:141 src/control/Control.RowHeader.js:43
msgid "Delete row"
msgstr ""
-#: src/control/Control.Menubar.js:313
+#: src/control/Control.Menubar.js:314
msgid "Are you sure you want to delete this slide?"
msgstr ""
@@ -952,27 +965,27 @@ msgstr ""
msgid "Unsupported server version."
msgstr ""
-#: src/core/Socket.js:170
+#: src/core/Socket.js:173
msgid "Document requires password to view."
msgstr ""
-#: src/core/Socket.js:173
+#: src/core/Socket.js:176
msgid "Document requires password to modify."
msgstr ""
-#: src/core/Socket.js:175
+#: src/core/Socket.js:178
msgid "Hit Cancel to open in view-only mode."
msgstr ""
-#: src/core/Socket.js:179
+#: src/core/Socket.js:182
msgid "Wrong password provided. Please try again."
msgstr ""
-#: src/core/Socket.js:221
+#: src/core/Socket.js:231
msgid "Connecting..."
msgstr ""
-#: src/core/Socket.js:339
+#: src/core/Socket.js:349
msgid ""
"Well, this is embarrassing, we cannot connect to your document. Please try "
"again."
commit 5df5ec9c6bef1aec9d4eb468a8117fe38d8079a0
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Tue Oct 18 21:51:11 2016 +0100
Avoid deadlock when notifying users of document load failure.
(cherry picked from commit 8d4c5d4e68eaf196a9e6c7478fbee15285beac04)
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 624b2be..3adc1ad 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -410,30 +410,30 @@ size_t DocumentBroker::addSession(std::shared_ptr<ClientSession>& session)
const auto id = session->getId();
const std::string aMessage = "session " + id + " " + _docKey + "\n";
- std::lock_guard<std::mutex> lock(_mutex);
+ try
+ {
+ std::lock_guard<std::mutex> lock(_mutex);
- // Request a new session from the child kit.
- Log::debug("DocBroker to Child: " + aMessage.substr(0, aMessage.length() - 1));
- _childProcess->sendTextFrame(aMessage);
+ // Request a new session from the child kit.
+ Log::debug("DocBroker to Child: " + aMessage.substr(0, aMessage.length() - 1));
+ _childProcess->sendTextFrame(aMessage);
- auto ret = _sessions.emplace(id, session);
- if (!ret.second)
- {
- Log::warn("DocumentBroker: Trying to add already existing session.");
- }
+ auto ret = _sessions.emplace(id, session);
+ if (!ret.second)
+ {
+ Log::warn("DocumentBroker: Trying to add already existing session.");
+ }
- if (session->isReadOnly())
- {
- Log::debug("Adding a readonly session [" + id + "]");
- }
+ if (session->isReadOnly())
+ {
+ Log::debug("Adding a readonly session [" + id + "]");
+ }
- // Below values are recalculated when startDestroy() is called (before destroying the
- // document). It is safe to reset their values to their defaults whenever a new session is added.
- _lastEditableSession = false;
- _markToDestroy = false;
+ // Below values are recalculated when startDestroy() is called (before destroying the
+ // document). It is safe to reset their values to their defaults whenever a new session is added.
+ _lastEditableSession = false;
+ _markToDestroy = false;
- try
- {
load(id, std::to_string(_childProcess->getPid()));
}
catch (const StorageSpaceLowException&)
@@ -491,6 +491,7 @@ size_t DocumentBroker::removeSession(const std::string& id)
void DocumentBroker::alertAllUsersOfDocument(const std::string& cmd, const std::string& kind)
{
+ Util::assertIsNotLocked(_mutex);
std::lock_guard<std::mutex> lock(_mutex);
std::stringstream ss;
diff --git a/loolwsd/Util.hpp b/loolwsd/Util.hpp
index a55c70e..d99fd36 100644
--- a/loolwsd/Util.hpp
+++ b/loolwsd/Util.hpp
@@ -106,6 +106,13 @@ namespace Util
assert(!mtx.try_lock());
}
+ inline
+ void assertIsNotLocked(std::mutex& mtx)
+ {
+ assert(mtx.try_lock());
+ mtx.unlock();
+ }
+
/// Safely remove a file or directory.
/// Supresses exception when the file is already removed.
/// This can happen when there is a race (unavoidable) or when
commit b8b113e1e1f56fdce6c6c09484db6cab5ab9dddd
Author: Tor Lillqvist <tml at collabora.com>
Date: Tue Oct 18 16:54:31 2016 +0300
Use LOOLProtocol::getAbbreviatedFrameDump()
(cherry picked from commit ece974092616445983da34283228c80a81642a19)
diff --git a/loolwsd/Connect.cpp b/loolwsd/Connect.cpp
index 237e7c2..3c14b22 100644
--- a/loolwsd/Connect.cpp
+++ b/loolwsd/Connect.cpp
@@ -95,7 +95,7 @@ public:
{
{
std::unique_lock<std::mutex> lock(coutMutex);
- std::cout << "Got " << n << " bytes: " << getAbbreviatedMessage(buffer, n) << std::endl;
+ std::cout << "Got " << getAbbreviatedFrameDump(buffer, n, flags) << std::endl;
}
std::string firstLine = getFirstLine(buffer, n);
diff --git a/loolwsd/test/helpers.hpp b/loolwsd/test/helpers.hpp
index ab2b1ba..cdc7515 100644
--- a/loolwsd/test/helpers.hpp
+++ b/loolwsd/test/helpers.hpp
@@ -185,6 +185,7 @@ std::vector<char> getResponseMessage(Poco::Net::WebSocket& ws, const std::string
response.resize(READ_BUFFER_SIZE);
int bytes = ws.receiveFrame(response.data(), response.size(), flags);
response.resize(bytes >= 0 ? bytes : 0);
+ std::cerr << name << "Got " << LOOLProtocol::getAbbreviatedFrameDump(response.data(), bytes, flags) << std::endl;
auto message = LOOLProtocol::getAbbreviatedMessage(response);
if (bytes > 0 && (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE)
{
@@ -206,12 +207,10 @@ std::vector<char> getResponseMessage(Poco::Net::WebSocket& ws, const std::string
if (message.find(prefix) == 0)
{
- std::cerr << name << "Got " << bytes << " bytes: " << message << std::endl;
return response;
}
else if (message.find("nextmessage") == 0)
{
- std::cerr << name << "Got " << bytes << " bytes: " << message << std::endl;
Poco::StringTokenizer tokens(message, " ", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
int size = 0;
if (tokens.count() == 2 &&
@@ -220,10 +219,10 @@ std::vector<char> getResponseMessage(Poco::Net::WebSocket& ws, const std::string
response.resize(size);
bytes = ws.receiveFrame(response.data(), response.size(), flags);
response.resize(bytes >= 0 ? bytes : 0);
+ std::cerr << name << "Got " << LOOLProtocol::getAbbreviatedFrameDump(response.data(), bytes, flags) << std::endl;
message = LOOLProtocol::getAbbreviatedMessage(response);
if (bytes > 0 && message.find(prefix) == 0)
{
- std::cerr << name << "Got " << bytes << " bytes: " << message << std::endl;
return response;
}
}
@@ -232,7 +231,6 @@ std::vector<char> getResponseMessage(Poco::Net::WebSocket& ws, const std::string
else
{
response.resize(0);
- std::cerr << name << "Got " << bytes << " bytes, flags: " << std::hex << flags << std::dec << std::endl;
}
if (bytes <= 0)
@@ -405,18 +403,14 @@ void SocketProcessor(const std::string& name,
}
n = socket->receiveFrame(buffer, sizeof(buffer), flags);
+ std::cerr << name << "Got " << LOOLProtocol::getAbbreviatedFrameDump(buffer, n, flags) << std::endl;
if (n > 0 && (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE)
{
- std::cerr << name << "Got " << n << " bytes: " << LOOLProtocol::getAbbreviatedMessage(buffer, n) << std::endl;
if (!handler(std::string(buffer, n)))
{
break;
}
}
- else
- {
- std::cerr << name << "Got " << n << " bytes, flags: " << std::hex << flags << std::dec << std::endl;
- }
}
while (n > 0 && (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE);
}
diff --git a/loolwsd/test/httpcrashtest.cpp b/loolwsd/test/httpcrashtest.cpp
index 53e6f0c..ab8f018 100644
--- a/loolwsd/test/httpcrashtest.cpp
+++ b/loolwsd/test/httpcrashtest.cpp
@@ -188,7 +188,7 @@ void HTTPCrashTest::testCrashKit()
do
{
bytes = socket.receiveFrame(buffer, sizeof(buffer), flags);
- std::cerr << testname << "Got " << bytes << " bytes: " << LOOLProtocol::getAbbreviatedMessage(buffer, bytes) << std::endl;
+ std::cerr << testname << "Got " << LOOLProtocol::getAbbreviatedFrameDump(buffer, bytes, flags) << std::endl;
}
while ((flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE);
commit 51cc161b52f6a298eacbdbec65c5ced41190612e
Author: Tor Lillqvist <tml at collabora.com>
Date: Tue Oct 18 16:24:41 2016 +0300
Introduce LOOLProtocol::getAbbreviatedFrameDump()
Produces a human-readable logging string describing a LOOL protocol
frame. To be used for re-factoring snippets of similar code that
sometimes print also the flags, sometimes pointlessly print the
(binary) contents of a CLOSE frame (as if it was text), etc.
(cherry picked from commit 24442a080cd3e27db513a766d913c3a63f682613)
diff --git a/loolwsd/LOOLProtocol.hpp b/loolwsd/LOOLProtocol.hpp
index 3857a4e..41c5b3c 100644
--- a/loolwsd/LOOLProtocol.hpp
+++ b/loolwsd/LOOLProtocol.hpp
@@ -12,8 +12,11 @@
#include <cstring>
#include <map>
+#include <sstream>
#include <string>
+#include <Poco/Format.h>
+#include <Poco/Net/WebSocket.h>
#include <Poco/StringTokenizer.h>
#define LOK_USE_UNSTABLE_API
@@ -168,6 +171,36 @@ namespace LOOLProtocol
{
return getAbbreviatedMessage(message.data(), message.size());
}
+
+ // Return a string dump of a WebSocket frame: Its opcode, length, first line (if present),
+ // flags. For human-readable logging purposes. Format not guaranteed to be stable. Not to be
+ // inspected programmatically.
+ inline
+ std::string getAbbreviatedFrameDump(const char *message, const int length, const int flags)
+ {
+ std::ostringstream result;
+ switch (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK)
+ {
+#define CASE(x) case Poco::Net::WebSocket::FRAME_OP_##x: result << #x; break
+ CASE(CONT);
+ CASE(TEXT);
+ CASE(BINARY);
+ CASE(CLOSE);
+ CASE(PING);
+ CASE(PONG);
+#undef CASE
+ default:
+ result << Poco::format("%#x", flags);
+ break;
+ }
+ result << " frame: " << length << " bytes";
+
+ if (length > 0 &&
+ ((flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) == Poco::Net::WebSocket::FRAME_OP_TEXT ||
+ (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) == Poco::Net::WebSocket::FRAME_OP_BINARY))
+ result << ": '" << getAbbreviatedMessage(message, length) << "'";
+ return result.str();
+ }
};
#endif
commit 2a1870d220b0838d96cc88209a58b0b9a79016c7
Author: Tor Lillqvist <tml at collabora.com>
Date: Tue Oct 18 16:23:33 2016 +0300
Add a FIXME for some sillyness
One thing works mainly by accident, another is inefficient.
(cherry picked from commit 53b718844fd2c0b6366055cf6fc4d018cf6c339b)
diff --git a/loolwsd/test/helpers.hpp b/loolwsd/test/helpers.hpp
index 9e3756e..ab2b1ba 100644
--- a/loolwsd/test/helpers.hpp
+++ b/loolwsd/test/helpers.hpp
@@ -188,6 +188,22 @@ std::vector<char> getResponseMessage(Poco::Net::WebSocket& ws, const std::string
auto message = LOOLProtocol::getAbbreviatedMessage(response);
if (bytes > 0 && (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE)
{
+ // FIXME: This is wrong in two ways:
+
+ // 1) The message variable is the return value from getAbbreviatedMessage(),
+ // That is a string that is intended for human-readable logging only. It is not
+ // intended to be used for actually decoding the protocol. Sure, at the moment
+ // it happens to work, but is still wrong.
+
+ // 2) Using std::string::find() like this is silly. If message does not start
+ // with prefix, the find() function will search through the whole buffer. That
+ // is a waste of cycles. Use the LOOLProtocol functions to manipulate and
+ // inspect LOOL protocol frames, that is what they are there
+ // for. getFirstToken() should be quite efficient, and it doesn't incur the
+ // (potential) overhead of setting up a StringTokenizer. Or, if this is actually
+ // used to look for not just a first token, then introduce a startsWith()
+ // function.
+
if (message.find(prefix) == 0)
{
std::cerr << name << "Got " << bytes << " bytes: " << message << std::endl;
commit 2c9064ee6e099bbed0eb02eb7f3337dc566aaf08
Author: Tor Lillqvist <tml at collabora.com>
Date: Tue Oct 18 15:51:29 2016 +0300
Expand documentation for getAbbreviatedMessage()
It has always been the intent that getAbbreviatedMessage() is for
producing human-readbale nice output for logging purposes only. But
this has not been mentioned, so (naturally) the function has been
misused. Sigh.
(cherry picked from commit b0c870cbeb77b69c5bf1046646b5253b411a546a)
diff --git a/loolwsd/LOOLProtocol.hpp b/loolwsd/LOOLProtocol.hpp
index 6120e0c..3857a4e 100644
--- a/loolwsd/LOOLProtocol.hpp
+++ b/loolwsd/LOOLProtocol.hpp
@@ -133,7 +133,11 @@ namespace LOOLProtocol
return getFirstLine(message.data(), message.size());
}
- /// Returns an abbereviation of the message (the first line, indicating truncation).
+ /// Returns an abbereviation of the message (the first line, indicating truncation). We assume
+ /// that it adhers to the LOOL protocol, i.e. that there is always a first (or only) line that
+ /// is in printable UTF-8. I.e. no encoding of binary bytes is done. The format of the result is
+ /// not guaranteed to be stable. It is to be used for logging purposes only, not for decoding
+ /// protocol frames.
inline
std::string getAbbreviatedMessage(const char *message, const int length)
{
commit c855ba5e283bb4c9d15862e516383ead0d8cd646
Author: Tor Lillqvist <tml at collabora.com>
Date: Tue Oct 18 12:29:05 2016 +0300
Adapt to new URI format in the GET request
(cherry picked from commit e87cf1e5d817835e885af5cd0b4818227a51a1e0)
diff --git a/loolwsd/Connect.cpp b/loolwsd/Connect.cpp
index 211e56e..237e7c2 100644
--- a/loolwsd/Connect.cpp
+++ b/loolwsd/Connect.cpp
@@ -175,7 +175,9 @@ protected:
#else
HTTPClientSession cs(_uri.getHost(), _uri.getPort());
#endif
- HTTPRequest request(HTTPRequest::HTTP_GET, std::string("lool/ws/") + args[0]);
+ std::string encodedUri;
+ URI::encode(args[0], ":/?", encodedUri);
+ HTTPRequest request(HTTPRequest::HTTP_GET, "/lool/" + encodedUri + "/ws");
HTTPResponse response;
WebSocket ws(cs, request, response);
commit 5f4f58d311150354e3e80d08b338a88f9c45cc74
Author: Tor Lillqvist <tml at collabora.com>
Date: Tue Oct 18 09:45:02 2016 +0300
We can use LoolException::toString() in one more place
(cherry picked from commit 38ccbd019617d3d4fba4dad4ac31f964f0294adf)
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 90dba8e..312d7b6 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -906,7 +906,7 @@ private:
}
catch (const UnauthorizedRequestException& exc)
{
- Log::error("Error in client request handler: " + std::string(exc.what()));
+ Log::error("Error in client request handler: " + exc.toString());
status = "error: cmd=internal kind=unauthorized";
Log::trace("Sending to Client [" + status + "].");
ws->sendFrame(status.data(), (int) status.size());
commit 6fbe4c158ad0c43ff37f12407eb39029e50b8638
Author: Tor Lillqvist <tml at collabora.com>
Date: Tue Oct 18 09:02:03 2016 +0300
Bin pointless copy-pasted (?) code snippet
We did not use the 'request' variable for anyhthing.
(cherry picked from commit 620c08a32d68fa179f0b7e93f4808a2c301240da)
diff --git a/loolwsd/test/httpwserror.cpp b/loolwsd/test/httpwserror.cpp
index 75ccc67..1e35593 100644
--- a/loolwsd/test/httpwserror.cpp
+++ b/loolwsd/test/httpwserror.cpp
@@ -101,18 +101,16 @@ void HTTPWSError::testMaxDocuments()
try
{
// Load a document.
- std::string docPath;
- std::string docURL;
std::vector<std::shared_ptr<Poco::Net::WebSocket>> docs;
for (int it = 1; it <= MAX_DOCUMENTS; ++it)
{
- getDocumentPathAndURL("empty.odt", docPath, docURL);
- Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, docURL);
docs.emplace_back(loadDocAndGetSocket("empty.odt", _uri, testname));
}
// try to open MAX_DOCUMENTS + 1
+ std::string docPath;
+ std::string docURL;
getDocumentPathAndURL("empty.odt", docPath, docURL);
Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, docURL);
std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(_uri));
commit 875af1ca213271ed3a73a9756d7b68b1d59c0bf0
Author: Tor Lillqvist <tml at collabora.com>
Date: Tue Oct 18 08:32:18 2016 +0300
Introduce LoolException::toString() to avoid a few std::string casts
Also consistently cast the call of std::exception::what() and not a
string literal being conatenated with that.
(cherry picked from commit b02a917f0a6b7e6f12ea312a2624c8556b268a05)
diff --git a/loolwsd/Exceptions.hpp b/loolwsd/Exceptions.hpp
index b9839c8..de5fcaa 100644
--- a/loolwsd/Exceptions.hpp
+++ b/loolwsd/Exceptions.hpp
@@ -17,6 +17,12 @@
// Generic LOOL errors and base for others.
class LoolException : public std::runtime_error
{
+public:
+ std::string toString() const
+ {
+ return what();
+ }
+
protected:
using std::runtime_error::runtime_error;
};
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 507f6fe..90dba8e 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -507,7 +507,7 @@ private:
}
catch (const std::exception& ex)
{
- Log::error(std::string("Failed to get save-as url: ") + ex.what());
+ Log::error("Failed to get save-as url: " + std::string(ex.what()));
}
lock.lock();
@@ -1080,7 +1080,7 @@ public:
catch (const WebSocketErrorMessageException& exc)
{
// Internal error that should be passed on to the client.
- Log::error(std::string("ClientRequestHandler::handleClientRequest: WebSocketErrorMessageException: ") + exc.what());
+ Log::error("ClientRequestHandler::handleClientRequest: WebSocketErrorMessageException: " + exc.toString());
try
{
ws->sendFrame(exc.what(), std::strlen(exc.what()));
@@ -1089,7 +1089,7 @@ public:
}
catch (const std::exception& exc2)
{
- Log::error(std::string("ClientRequestHandler::handleClientRequest: exception while sending WS error message: ") + exc2.what());
+ Log::error("ClientRequestHandler::handleClientRequest: exception while sending WS error message: " + std::string(exc2.what()));
}
}
}
@@ -1108,17 +1108,17 @@ public:
}
catch (const UnauthorizedRequestException& exc)
{
- Log::error(std::string("ClientRequestHandler::handleClientRequest: UnauthorizedException: ") + exc.what());
+ Log::error("ClientRequestHandler::handleClientRequest: UnauthorizedException: " + exc.toString());
response.setStatusAndReason(HTTPResponse::HTTP_UNAUTHORIZED);
}
catch (const BadRequestException& exc)
{
- Log::error(std::string("ClientRequestHandler::handleClientRequest: BadRequestException: ") + exc.what());
+ Log::error("ClientRequestHandler::handleClientRequest: BadRequestException: " + exc.toString());
response.setStatusAndReason(HTTPResponse::HTTP_BAD_REQUEST);
}
catch (const std::exception& exc)
{
- Log::error(std::string("ClientRequestHandler::handleClientRequest: Exception: ") + exc.what());
+ Log::error("ClientRequestHandler::handleClientRequest: Exception: " + std::string(exc.what()));
response.setStatusAndReason(HTTPResponse::HTTP_SERVICE_UNAVAILABLE);
}
commit d3abab7fff22dc4b9ff9d60d8dd5e34598cab211
Author: Tor Lillqvist <tml at collabora.com>
Date: Tue Oct 18 08:22:46 2016 +0300
Use correct function name in some log messages
These Log::foo() calls are in ClientRequestHandler::handleClientRequest(),
not in ClientRequestHandler::handleRequest().
Actually I wonder why we show the name of the function in this handful of
places. We don't do it in general. Consistency, bah.
(cherry picked from commit 72133868b58006db1fdd4fc09ee3e850494f83af)
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 42c605f..507f6fe 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -1080,7 +1080,7 @@ public:
catch (const WebSocketErrorMessageException& exc)
{
// Internal error that should be passed on to the client.
- Log::error(std::string("ClientRequestHandler::handleRequest: WebSocketErrorMessageException: ") + exc.what());
+ Log::error(std::string("ClientRequestHandler::handleClientRequest: WebSocketErrorMessageException: ") + exc.what());
try
{
ws->sendFrame(exc.what(), std::strlen(exc.what()));
@@ -1089,7 +1089,7 @@ public:
}
catch (const std::exception& exc2)
{
- Log::error(std::string("ClientRequestHandler::handleRequest: exception while sending WS error message: ") + exc2.what());
+ Log::error(std::string("ClientRequestHandler::handleClientRequest: exception while sending WS error message: ") + exc2.what());
}
}
}
@@ -1101,24 +1101,24 @@ public:
}
catch (const Exception& exc)
{
- Log::error() << "ClientRequestHandler::handleRequest: " << exc.displayText()
+ Log::error() << "ClientRequestHandler::handleClientRequest: " << exc.displayText()
<< (exc.nested() ? " (" + exc.nested()->displayText() + ")" : "")
<< Log::end;
response.setStatusAndReason(HTTPResponse::HTTP_SERVICE_UNAVAILABLE);
}
catch (const UnauthorizedRequestException& exc)
{
- Log::error(std::string("ClientRequestHandler::handleRequest: UnauthorizedException: ") + exc.what());
+ Log::error(std::string("ClientRequestHandler::handleClientRequest: UnauthorizedException: ") + exc.what());
response.setStatusAndReason(HTTPResponse::HTTP_UNAUTHORIZED);
}
catch (const BadRequestException& exc)
{
- Log::error(std::string("ClientRequestHandler::handleRequest: BadRequestException: ") + exc.what());
+ Log::error(std::string("ClientRequestHandler::handleClientRequest: BadRequestException: ") + exc.what());
response.setStatusAndReason(HTTPResponse::HTTP_BAD_REQUEST);
}
catch (const std::exception& exc)
{
- Log::error(std::string("ClientRequestHandler::handleRequest: Exception: ") + exc.what());
+ Log::error(std::string("ClientRequestHandler::handleClientRequest: Exception: ") + exc.what());
response.setStatusAndReason(HTTPResponse::HTTP_SERVICE_UNAVAILABLE);
}
commit f84c9b6d9206dd6782496d8db304040bfb60200a
Author: Tor Lillqvist <tml at collabora.com>
Date: Tue Oct 18 08:06:16 2016 +0300
Add logging of NumDocBrokers and the size of the DocBrokers array
I fear they get can out of sync when HTTPCrashTest::killLoKitProcesses()
kills a loolkit process, and this then causes HTTPWSError::testMaxDocuments()
to fail. This is to help debugging that.
Haven't fully understood what is going on yet. But one thing is sure:
It is a bad idea to duplicate the same state information in two
places, we shouldn't really use that separate NumDocBrokers
variable.
Probably also NumConnections tracks state that can easily be
calculated from the data structures, but maybe NumConnections does not
that easily get out of sync.
(cherry picked from commit 32b33ee68503b1ff649cced8ffa1ef25fe988e08)
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 468135c..42c605f 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -176,6 +176,19 @@ static int careerSpanSeconds = 0;
namespace {
+static void logNumDocBrokers(int lineNo)
+{
+ int size = 0;
+ int nonEmpty = 0;
+ for (auto& i : DocBrokers)
+ {
+ size++;
+ if (i.second->getPublicUri().toString() != "")
+ nonEmpty++;
+ }
+ Log::debug() << "line " << lineNo << ": NumDocBrokers=" << LOOLWSD::NumDocBrokers << " size: " << size << " of which non-empty: " << nonEmpty << Log::end;
+}
+
static inline
void shutdownLimitReached(WebSocket& ws)
{
@@ -731,13 +744,17 @@ private:
}
#if MAX_DOCUMENTS > 0
+ std::unique_lock<std::mutex> DocBrokersLock(DocBrokersMutex);
+ logNumDocBrokers(__LINE__);
if (++LOOLWSD::NumDocBrokers > MAX_DOCUMENTS)
{
--LOOLWSD::NumDocBrokers;
Log::error("Maximum number of open documents reached.");
+ logNumDocBrokers(__LINE__);
shutdownLimitReached(*ws);
return;
}
+ DocBrokersLock.unlock();
#endif
// Set one we just created.
@@ -757,6 +774,7 @@ private:
DocBrokers.erase(docKey);
#if MAX_DOCUMENTS > 0
--LOOLWSD::NumDocBrokers;
+ logNumDocBrokers(__LINE__);
#endif
}
@@ -873,6 +891,7 @@ private:
DocBrokers.erase(docKey);
#if MAX_DOCUMENTS > 0
--LOOLWSD::NumDocBrokers;
+ logNumDocBrokers(__LINE__);
#endif
Log::info("Removing complete doc [" + docKey + "] from Admin.");
Admin::instance().rmDoc(docKey);
commit ed0491aa9ea92ebdbea072d5682aec97e8db0291
Author: Tor Lillqvist <tml at collabora.com>
Date: Mon Oct 17 21:56:14 2016 +0300
killLoKitProcesses() already calls countLoolKitProcesses(0)
No need to immetiately call it again in testBarren().
(cherry picked from commit 970259d170ddcb938bccdecad7f32792639ba62a)
diff --git a/loolwsd/test/httpcrashtest.cpp b/loolwsd/test/httpcrashtest.cpp
index fad34a2..53e6f0c 100644
--- a/loolwsd/test/httpcrashtest.cpp
+++ b/loolwsd/test/httpcrashtest.cpp
@@ -122,7 +122,6 @@ void HTTPCrashTest::testBarren()
try
{
killLoKitProcesses();
- countLoolKitProcesses(0);
std::cerr << "Loading after kill." << std::endl;
commit 5fe6d7b7ee4a84c9f84a718ffa24fdd4488dd7d5
Author: Tor Lillqvist <tml at collabora.com>
Date: Mon Oct 17 21:44:28 2016 +0300
Fix HTTPWSTest::testConnectNoLoad
The waiting for the expected number of loolkit processes after that
test never succeeded. That apparently was caused by the fact that the
loolwsd process is waiting for up to COMMAND_TIMEOUT_MS (five seconds
currently) for an auto-save of the document to succeed, and that never
happens in this case.
countLoolKitProcesses() on the other hand was waiting only a bit over
three seconds (calculated in a complicated way from POLL_TIMEOUT_MS
and a few magic numbers) for the number of loolkit processes to reach
the expected number. Or something like that.
COMMAND_TIMEOUT_MS=5000 and POLL_TIMEOUT_MS=COMMAND_TIMEOUT_MS/10 =>
500. We used to have in countLoolKitProcesses():
sleepMs=POLL_TIMEOUT_MS/3 => 166 and repeat=(3000/sleepMs)+1 =>
19. 19*166 => 3154.
Fix by calculating the max time to wait for the expected number of
loolkit processes in countLoolKitProcesses() in a different way, so
that it is always longer than COMMAND_TIMEOUT_MS.
(cherry picked from commit a3236497a85c8fcdc48c55a2c7830cefdb05e619)
diff --git a/loolwsd/test/countloolkits.hpp b/loolwsd/test/countloolkits.hpp
index b96e173..275efea 100644
--- a/loolwsd/test/countloolkits.hpp
+++ b/loolwsd/test/countloolkits.hpp
@@ -69,9 +69,17 @@ int countLoolKitProcesses(const int expected)
{
std::cerr << "Waiting to have " << expected << " loolkit processes. Loolkits: ";
- // Retry for about 3 seconds.
- const auto sleepMs = static_cast<int>(POLL_TIMEOUT_MS / 3);
- const size_t repeat = (3000 / sleepMs) + 1;
+ // We have to wait at least for the time the call docBroker->autoSave(forceSave,
+ // COMMAND_TIMEOUT_MS)) in ClientRequestHandler:::handleGetRequest() can take to wait for
+ // information about a successful auto-save. In the HTTPWSTest::testConnectNoLoad() there is
+ // nothing to auto-save, so it waits in vain.
+
+ // This does not need to depend on any constant from Common.hpp. The shorter the better (the
+ // quicker the test runs).
+ const auto sleepMs = 200;
+
+ // This has to cause waiting for at least COMMAND_TIMEOUT_MS. Add one second for safety.
+ const size_t repeat = ((COMMAND_TIMEOUT_MS + 1000) / sleepMs);
auto count = getLoolKitProcessCount();
for (size_t i = 0; i < repeat; ++i)
{
commit 9a862918cc5288ffedaff851c7e0605c389f94eb
Author: Tor Lillqvist <tml at collabora.com>
Date: Mon Oct 17 16:55:20 2016 +0300
Attempt to handle unauthorized WOPI usage better
Use the previously unused UnauthorizedRequestException for this, and
throw a such in StorageBase::create() when the WOPI host doesn't match
any of those configured.
In a developer debug build, without access to any real WOPI
functionality, you can test by setting the FAKE_UNAUTHORIZED
environment variable and attempting to edit a plain local file:
URI. That will cause such an exception to be thrown in that function.
Catch that UnauthorizedRequestException in
ClientRequestHandler::handleGetRequest(), and send an 'error:
cmd=internal kind=unauthorized' message to the client. Handle that in
loleaflet in the same place where the 'error: cmd=internal
kild=diskfull' message is handled, and in the same fashion, giving up
on the document.
Actually, using exceptions for relatively non-exceptional situations
like this is lame and makes understanding the code harder, but that is
just my personal preference...
FIXME: By the time StorageBase::create() gets called we have already
sent three 'statusindicator:' messages ('find', 'connect', and
'ready') to the client. We should ideally do the checks we do in
StorageBase::create() much earlier.
Also consider that ClientRequestHandler::handleClientRequest() has
code that catches UnauthorizedRequestException and
BadRequestException, and tries to set the HTTP response in those
cases. I am not sure if that functionality has ever been exercised,
though. Currently, we upgrade the HTTP connection to WebSocket early,
and only after that we check whether the WOPI host is authorized
etc. By that time it is too late to return an HTTP response to the
user. If that even is what we ideally should do? If not, then we
probably should drop the code that constructs HTTP responses and
attempts to send them.
Also, if I, as a test, force an HTTPResponse::HTTP_BAD_REQUEST to be
sent before the HTTP connection is upgraded to WebSocket, loleaflet
throws up the generic "Well, this is embarrassing" dialog anyway. At
least in Firefox on Linux. (Instead of the browser showing some own
dialog, which I was half-expecting to happen.)
(cherry picked from commit bb36ca79d4318d7916d74c587673f477659c4d0c)
diff --git a/loleaflet/dist/errormessages.js b/loleaflet/dist/errormessages.js
index 8c97e00..9ccfaf8 100644
--- a/loleaflet/dist/errormessages.js
+++ b/loleaflet/dist/errormessages.js
@@ -2,4 +2,5 @@ exports.diskfull = _('No disk space left on server, please contact the server ad
exports.emptyhosturl = _('The host URL is empty. The loolwsd server is probably misconfigured, please contact the administrator.');
exports.limitreached = _('This development build is limited to %0 documents, and %1 connections - to avoid the impression that it is suitable for deployment in large enterprises. To find out more about deploying and scaling %2 checkout: <br/><a href=\"%3\">%3</a>.');
exports.serviceunavailable = _('Service is unavailable. Please try again later and report to your administrator if the issue persists.');
+exports.unauthorized = _('Unauthorized WOPI host. Please try again later and report to your administrator if the issue persists.');
exports.wrongwopisrc = _('Wrong WOPISrc, usage: WOPISrc=valid encoded URI, or file_path, usage: file_path=/path/to/doc/');
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index a86cd5f..07126bb 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -159,6 +159,9 @@ L.Socket = L.Class.extend({
if (command.errorKind === 'diskfull') {
this._map.fire('error', {msg: errorMessages.diskfull});
}
+ else if (command.errorKind === 'unauthorized') {
+ this._map.fire('error', {msg: errorMessages.unauthorized});
+ }
if (this._map._docLayer) {
this._map._docLayer.removeAllViews();
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 0631c6c..468135c 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -885,6 +885,13 @@ private:
{
throw;
}
+ catch (const UnauthorizedRequestException& exc)
+ {
+ Log::error("Error in client request handler: " + std::string(exc.what()));
+ status = "error: cmd=internal kind=unauthorized";
+ Log::trace("Sending to Client [" + status + "].");
+ ws->sendFrame(status.data(), (int) status.size());
+ }
catch (const std::exception& exc)
{
Log::error("Error in client request handler: " + std::string(exc.what()));
@@ -1096,10 +1103,18 @@ public:
response.setStatusAndReason(HTTPResponse::HTTP_SERVICE_UNAVAILABLE);
}
+ if (responded)
+ Log::debug("Already sent response!?");
if (!responded)
{
+ // I wonder if this code path has ever been exercised
+ Log::debug("Attempting to send response");
response.setContentLength(0);
- response.send();
+ std::ostream& os = response.send();
+ if (!os.good())
+ Log::debug("Response stream is not good after send");
+ else
+ Log::debug("Response stream *is* good after send");
}
Log::debug("Thread finished.");
diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp
index ca4ca25..d500aae 100644
--- a/loolwsd/Storage.cpp
+++ b/loolwsd/Storage.cpp
@@ -130,6 +130,11 @@ bool isLocalhost(const std::string& targetHost)
std::unique_ptr<StorageBase> StorageBase::create(const Poco::URI& uri, const std::string& jailRoot, const std::string& jailPath)
{
+ // FIXME: By the time this gets called we have already sent to the client three
+ // 'statusindicator:' messages: 'find', 'connect' and 'ready'. We should ideally do the checks
+ // here much earlier. Also, using exceptions is lame and makes understanding the code harder,
+ // but that is just my personal preference.
+
std::unique_ptr<StorageBase> storage;
if (UnitWSD::get().createStorage(uri, jailRoot, jailPath, storage))
@@ -141,6 +146,13 @@ std::unique_ptr<StorageBase> StorageBase::create(const Poco::URI& uri, const std
else if (uri.isRelative() || uri.getScheme() == "file")
{
Log::info("Public URI [" + uri.toString() + "] is a file.");
+#if ENABLE_DEBUG
+ if (std::getenv("FAKE_UNAUTHORIZED"))
+ {
+ Log::fatal("Faking an UnauthorizedRequestException");
+ throw UnauthorizedRequestException("No acceptable WOPI hosts found matching the target host in config.");
+ }
+#endif
if (FilesystemEnabled)
{
return std::unique_ptr<StorageBase>(new LocalStorage(uri, jailRoot, jailPath));
@@ -157,7 +169,7 @@ std::unique_ptr<StorageBase> StorageBase::create(const Poco::URI& uri, const std
return std::unique_ptr<StorageBase>(new WopiStorage(uri, jailRoot, jailPath));
}
- Log::error("No acceptable WOPI hosts found matching the target host [" + targetHost + "] in config.");
+ throw UnauthorizedRequestException("No acceptable WOPI hosts found matching the target host [" + targetHost + "] in config.");
}
throw BadRequestException("No Storage configured or invalid URI.");
commit cc5c292b4a822b3801be81a92715c56f1b222e16
Author: Tor Lillqvist <tml at collabora.com>
Date: Mon Oct 17 16:23:22 2016 +0300
Sort lines for clarity
(cherry picked from commit b29ae3c0327c0617afe4b59328b1b3f31855087c)
diff --git a/loleaflet/dist/errormessages.js b/loleaflet/dist/errormessages.js
index 47a81fa..8c97e00 100644
--- a/loleaflet/dist/errormessages.js
+++ b/loleaflet/dist/errormessages.js
@@ -1,5 +1,5 @@
-exports.wrongwopisrc = _('Wrong WOPISrc, usage: WOPISrc=valid encoded URI, or file_path, usage: file_path=/path/to/doc/');
-exports.emptyhosturl = _('The host URL is empty. The loolwsd server is probably misconfigured, please contact the administrator.');
exports.diskfull = _('No disk space left on server, please contact the server administrator to continue.');
+exports.emptyhosturl = _('The host URL is empty. The loolwsd server is probably misconfigured, please contact the administrator.');
exports.limitreached = _('This development build is limited to %0 documents, and %1 connections - to avoid the impression that it is suitable for deployment in large enterprises. To find out more about deploying and scaling %2 checkout: <br/><a href=\"%3\">%3</a>.');
exports.serviceunavailable = _('Service is unavailable. Please try again later and report to your administrator if the issue persists.');
+exports.wrongwopisrc = _('Wrong WOPISrc, usage: WOPISrc=valid encoded URI, or file_path, usage: file_path=/path/to/doc/');
commit 177abe949800b7563938bd3d956bd0af1f3b63c0
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Mon Oct 17 21:21:54 2016 +0530
loolwsd: Fix handling of CPPUNIT_TEST_NAME in external test-suite
Doing a plain CPPUNIT_TEST_NAME='somettest' make check will
result in executing all the tests in external test suite. This is
a problem when one wants to execute only internal tests (unit-*
ones) as test harness first executes those followed by all of the
tests in external test-suites.
Lets execute all the tests only when no CPPUNIT_TEST_NAME is
provided, and ignore when it is provided but no match is found.
Change-Id: I7e40b6f3124e6965a86cfb6395d246df3b5c17ba
(cherry picked from commit 8ab4682a710dbea6286ccd09c2239413038c80fd)
diff --git a/loolwsd/test/test.cpp b/loolwsd/test/test.cpp
index 7a081a9..ba80912 100644
--- a/loolwsd/test/test.cpp
+++ b/loolwsd/test/test.cpp
@@ -24,51 +24,34 @@
class HTTPGetTest;
-bool filterTests(CPPUNIT_NS::TestRunner& runner, CPPUNIT_NS::Test* testRegistry)
+bool filterTests(CPPUNIT_NS::TestRunner& runner, CPPUNIT_NS::Test* testRegistry, const std::string testName)
{
- const char* envar = std::getenv("CPPUNIT_TEST_NAME");
- if (envar)
- {
- std::string testName(envar);
- if (testName.empty())
- {
- return false;
- }
-
- Poco::RegularExpression re(testName, Poco::RegularExpression::RE_CASELESS);
- Poco::RegularExpression::Match reMatch;
+ Poco::RegularExpression re(testName, Poco::RegularExpression::RE_CASELESS);
+ Poco::RegularExpression::Match reMatch;
- bool haveTests = false;
- for (int i = 0; i < testRegistry->getChildTestCount(); ++i)
+ bool haveTests = false;
+ for (int i = 0; i < testRegistry->getChildTestCount(); ++i)
+ {
+ CPPUNIT_NS::Test* testSuite = testRegistry->getChildTestAt(i);
+ for (int j = 0; j < testSuite->getChildTestCount(); ++j)
{
- CPPUNIT_NS::Test* testSuite = testRegistry->getChildTestAt(i);
- for (int j = 0; j < testSuite->getChildTestCount(); ++j)
+ CPPUNIT_NS::Test* testCase = testSuite->getChildTestAt(j);
+ try
{
- CPPUNIT_NS::Test* testCase = testSuite->getChildTestAt(j);
- try
- {
- if (re.match(testCase->getName(), reMatch))
- {
- runner.addTest(testCase);
- haveTests = true;
- }
- }
- catch (const std::exception& exc)
+ if (re.match(testCase->getName(), reMatch))
{
- // Nothing to do; skip.
+ runner.addTest(testCase);
+ haveTests = true;
}
}
+ catch (const std::exception& exc)
+ {
+ // Nothing to do; skip.
+ }
}
-
- if (!haveTests)
- {
- std::cerr << "Failed to match [" << testName << "] to any names in the test-suite. Running all tests." << std::endl;
- }
-
- return haveTests;
}
- return false;
+ return haveTests;
}
int main(int /*argc*/, char** /*argv*/)
@@ -86,11 +69,27 @@ int main(int /*argc*/, char** /*argv*/)
CPPUNIT_NS::Test* testRegistry = CPPUNIT_NS::TestFactoryRegistry::getRegistry().makeTest();
CPPUNIT_NS::TestRunner runner;
- if (!filterTests(runner, testRegistry))
+ const char* envar = std::getenv("CPPUNIT_TEST_NAME");
+ std::string testName;
+ if (envar)
+ {
+ testName = std::string(envar);
+ }
+
+ if (testName.empty())
{
- // All tests.
+ // Add all tests.
runner.addTest(testRegistry);
}
+ else
+ {
+ const bool testsAdded = filterTests(runner, testRegistry, testName);
+ if (!testsAdded)
+ {
+ std::cerr << "Failed to match [" << testName << "] to any names in the external test-suite. "
+ << "No external tests will be executed" << std::endl;
+ }
+ }
runner.run(controller);
commit a468d03b357ac103ddd5295624a565c673a88160
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Mon Oct 17 18:30:27 2016 +0530
loolwsd: Bring this test back to life
There doesn't seem to be any failure on this test anymore.
Consecutive 20 runs of this test gives no failure, so lets enable
it again.
Change-Id: I77ddd1c36d18162bdc75fd24d51c1a2df22f749d
(cherry picked from commit be1e49715c471598eaee53e036b8d6b92948a0d8)
diff --git a/loolwsd/test/UnitAdmin.cpp b/loolwsd/test/UnitAdmin.cpp
index 6014f76..540318f 100644
--- a/loolwsd/test/UnitAdmin.cpp
+++ b/loolwsd/test/UnitAdmin.cpp
@@ -422,7 +422,7 @@ public:
_tests.push_back(&UnitAdmin::testAddDocNotify);
_tests.push_back(&UnitAdmin::testUsersCount);
_tests.push_back(&UnitAdmin::testDocCount);
- // FIXME make this one reliable, and enable again _tests.push_back(&UnitAdmin::testRmDocNotify);
+ _tests.push_back(&UnitAdmin::testRmDocNotify);
_tests.push_back(&UnitAdmin::testUsersCount);
_tests.push_back(&UnitAdmin::testDocCount);
#endif
commit 3cbd452680969e16617a0ed8d559eca21b205828
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Mon Oct 17 17:40:35 2016 +0530
loolwsd: Fix admin remove doc after unloading the session
Change-Id: Ia512d5b4c5f0e230542caed6cebc242e3a345430
(cherry picked from commit 38e8a38034325cea62d721481704035c746d8691)
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index e591244..0631c6c 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -861,6 +861,9 @@ private:
sessionsCount = docBroker->removeSession(id);
Log::trace(docKey + ", ws_sessions--: " + std::to_string(sessionsCount));
}
+
+ // Lets remove this session from the admin console too
+ Admin::instance().rmDoc(docKey, id);
}
if (sessionsCount == 0)
commit b98dc34c40a65ef12d4b81d4d84d91fadf089014
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Mon Oct 17 17:15:58 2016 +0530
loolwsd: Fix admin console document add
Change-Id: I987f26b4aae2c4ea8ef65919f570576ef8c5d2a7
(cherry picked from commit d4fbc92023b1a81ad269ab7d87f0fcfba26294f9)
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index f9ef407..9472fb9 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -268,6 +268,9 @@ public:
void childSocketTerminated();
+ /// Get the PID of the associated child process
+ Poco::Process::PID getPid() const { return _childProcess->getPid(); }
+
private:
/// Sends the .uno:Save command to LoKit.
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 9538842..e591244 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -809,6 +809,9 @@ private:
ws->sendFrame(status.data(), (int) status.size());
}
+ // Tell the admin console about this new doc
+ Admin::instance().addDoc(docKey, docBroker->getPid(), docBroker->getFilename(), id);
+
LOOLWSD::dumpEventTrace(docBroker->getJailId(), id, "NewSession: " + uri);
// Let messages flow.
commit e617d9ad30d763dd3989c1112da248e38c537526
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Mon Oct 17 16:43:47 2016 +0530
loolwsd: Query docbroker for load duration after loading the doc
addSession() now also loads the document, so querying docbroker
for load duration before it doesn't make any sense.
Change-Id: I3c60bef5e2054878ba695b8f76b6800cdedffe8d
(cherry picked from commit 1c2c03fcb64042263601365073642e8a726f8045)
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 95c83b5..9538842 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -796,6 +796,10 @@ private:
Util::checkDiskSpaceOnRegisteredFileSystems();
+ // Request the child to connect to us and add this session.
+ auto sessionsCount = docBroker->addSession(session);
+ Log::trace(docKey + ", ws_sessions++: " + std::to_string(sessionsCount));
+
// If its a WOPI host, return time taken to make calls to it
const auto storageCallDuration = docBroker->getStorageLoadDuration();
if (storageCallDuration != std::chrono::duration<double>::zero())
@@ -805,10 +809,6 @@ private:
ws->sendFrame(status.data(), (int) status.size());
}
- // Request the child to connect to us and add this session.
- auto sessionsCount = docBroker->addSession(session);
- Log::trace(docKey + ", ws_sessions++: " + std::to_string(sessionsCount));
-
LOOLWSD::dumpEventTrace(docBroker->getJailId(), id, "NewSession: " + uri);
// Let messages flow.
commit c9c75320b61a51fff49c9115db5a9c394ae78a2d
Author: Tor Lillqvist <tml at collabora.com>
Date: Mon Oct 17 13:40:44 2016 +0300
Log how we are actually exiting the test when calling exitTest()
unit-admin fails every time for me at the moment. Make it at least a
bit easier to figure out what is going on.
(cherry picked from commit 9ced5ffdd58017eee0c72211a748c1820a2b551e)
diff --git a/loolwsd/test/UnitAdmin.cpp b/loolwsd/test/UnitAdmin.cpp
index 24fe35f..6014f76 100644
--- a/loolwsd/test/UnitAdmin.cpp
+++ b/loolwsd/test/UnitAdmin.cpp
@@ -440,13 +440,17 @@ public:
Log::info("UnitAdmin:: Finished test #" + std::to_string(_testCounter));
if (res != TestResult::TEST_OK)
{
+ Log::info("Exiting with " + (res == TestResult::TEST_FAILED ? "FAIL" : (res == TestResult::TEST_TIMED_OUT) ? "TIMEOUT" : "??? (" + std::to_string(res) + ")"));
exitTest(res);
return;
}
// End this when all tests are finished
if (_tests.size() == _testCounter)
+ {
+ Log::info("Exiting with OK");
exitTest(TestResult::TEST_OK);
+ }
_isTestRunning = false;
}
commit 70b32120afef3800c89463e6b33fce93664e27fd
Author: Marco Cecchetti <marco.cecchetti at collabora.com>
Date: Mon Oct 17 12:36:50 2016 +0200
loleaflet: handle EMPTY invalid tiles msg with part in the payload
Change-Id: I73e363f51101c8e4e258131ea1692a7709d6a544
Reviewed-on: https://gerrit.libreoffice.org/29964
Reviewed-by: Marco Cecchetti <mrcekets at gmail.com>
Tested-by: Marco Cecchetti <mrcekets at gmail.com>
(cherry picked from commit e61d8aaa5f3baa27651338b1074ccda3ecbaa8fb)
diff --git a/loleaflet/src/layer/tile/TileLayer.js b/loleaflet/src/layer/tile/TileLayer.js
index 8f96d3d..aa3b330 100644
--- a/loleaflet/src/layer/tile/TileLayer.js
+++ b/loleaflet/src/layer/tile/TileLayer.js
@@ -306,21 +306,24 @@ L.TileLayer = L.GridLayer.extend({
else if (textMsg.startsWith('invalidatecursor:')) {
this._onInvalidateCursorMsg(textMsg);
}
- else if (textMsg.startsWith('invalidatetiles:') && !textMsg.startsWith('EMPTY')) {
- this._onInvalidateTilesMsg(textMsg);
- }
- else if (textMsg.startsWith('invalidatetiles:') && textMsg.startsWith('EMPTY')) {
- var msg = 'invalidatetiles: ';
- if (this._docType === 'text') {
- msg += 'part=0 ';
- } else {
- var partNumber = parseInt(textMsg.substring(6));
- msg += 'part=' + (isNaN(partNumber) ? this._selectedPart : partNumber) + ' ';
+ else if (textMsg.startsWith('invalidatetiles:')) {
+ var payload = textMsg.substring('invalidatetiles:'.length + 1);
+ if (!payload.startsWith('EMPTY')) {
+ this._onInvalidateTilesMsg(textMsg);
+ }
+ else {
+ var msg = 'invalidatetiles: ';
+ if (this._docType === 'text') {
+ msg += 'part=0 ';
+ } else {
+ var partNumber = parseInt(payload.substring('EMPTY'.length + 1));
+ msg += 'part=' + (isNaN(partNumber) ? this._selectedPart : partNumber) + ' ';
+ }
+ msg += 'x=0 y=0 ';
+ msg += 'width=' + this._docWidthTwips + ' ';
+ msg += 'height=' + this._docHeightTwips;
+ this._onInvalidateTilesMsg(msg);
}
- msg += 'x=0 y=0 ';
- msg += 'width=' + this._docWidthTwips + ' ';
- msg += 'height=' + this._docHeightTwips;
- this._onInvalidateTilesMsg(msg);
}
else if (textMsg.startsWith('mousepointer:')) {
this._onMousePointerMsg(textMsg);
commit 591955175d0b98072414ca958c51b36148e946b9
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Mon Oct 17 15:24:33 2016 +0530
loolwsd: Remove this leftover duplicate comment
Change-Id: I243e2fd745b4b0c70cec79deefffcb5b8f7d2548
(cherry picked from commit cd92dc277b33963012f1dcbe9e065d11a60f2c92)
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 99e400e..95c83b5 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -782,10 +782,6 @@ private:
// Above this point exceptions are safe and will auto-cleanup.
// Below this, we need to cleanup internal references.
-
-
- // Above this point exceptions are safe and will auto-cleanup.
- // Below this, we need to cleanup internal references.
try
{
// indicator to a client that is waiting to connect to lokit process
commit 044cdaa4f495b11d2fc767100c3157721ecd77eb
Author: Tor Lillqvist <tml at collabora.com>
Date: Mon Oct 17 12:44:58 2016 +0300
Add comment to hopefully avoid others being confused like I was
(cherry picked from commit fdb16a5d522e68af825fa6a4b2c7baa462623b12)
diff --git a/loolwsd/test/run_unit.sh.in b/loolwsd/test/run_unit.sh.in
index 764c258..e00aad0 100755
--- a/loolwsd/test/run_unit.sh.in
+++ b/loolwsd/test/run_unit.sh.in
@@ -18,6 +18,10 @@ valgrind_cmd="valgrind --tool=memcheck --trace-children=no -v --read-var-info=ye
export LOOL_TEST_CLIENT_PORT=9984
export LOOL_TEST_MASTER_PORT=9985
+# Note that these options are used by commands in the Makefile that
+# Automake generates. Don't be mislead by 'git grep' not showing any
+# use of --test-name for instance.
+
tst=
while test $# -gt 0; do
case $1 in
commit 962a142b7acc4c48f3f0455b802e8ffd141d5843
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun Oct 16 17:56:25 2016 -0400
loolwsd: static variables must start with uppercase per the style guidelines
Change-Id: I1e8105983f98cc0cd15448e6d9cb1e6fca36ca9d
Reviewed-on: https://gerrit.libreoffice.org/29955
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit cb09f50d8c6767249a6058b7ca2bbea90b7c1c9f)
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index e46f839..99e400e 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -163,12 +163,12 @@ int MasterPortNumber = DEFAULT_MASTER_PORT_NUMBER;
/// New LOK child processes ready to host documents.
//TODO: Move to a more sensible namespace.
static bool DisplayVersion = false;
-static std::vector<std::shared_ptr<ChildProcess>> newChildren;
-static std::mutex newChildrenMutex;
-static std::condition_variable newChildrenCV;
-static std::chrono::steady_clock::time_point lastForkRequestTime = std::chrono::steady_clock::now();
-static std::map<std::string, std::shared_ptr<DocumentBroker>> docBrokers;
-static std::mutex docBrokersMutex;
+static std::vector<std::shared_ptr<ChildProcess>> NewChildren;
+static std::mutex NewChildrenMutex;
+static std::condition_variable NewChildrenCV;
+static std::chrono::steady_clock::time_point LastForkRequestTime = std::chrono::steady_clock::now();
+static std::map<std::string, std::shared_ptr<DocumentBroker>> DocBrokers;
+static std::mutex DocBrokersMutex;
#if ENABLE_DEBUG
static int careerSpanSeconds = 0;
@@ -217,7 +217,7 @@ void shutdownLimitReached(WebSocket& ws)
static void forkChildren(const int number)
{
- Util::assertIsLocked(newChildrenMutex);
+ Util::assertIsLocked(NewChildrenMutex);
if (number > 0)
{
@@ -225,14 +225,14 @@ static void forkChildren(const int number)
const std::string aMessage = "spawn " + std::to_string(number) + "\n";
Log::debug("MasterToForKit: " + aMessage.substr(0, aMessage.length() - 1));
IoUtil::writeToPipe(LOOLWSD::ForKitWritePipe, aMessage);
- lastForkRequestTime = std::chrono::steady_clock::now();
+ LastForkRequestTime = std::chrono::steady_clock::now();
}
}
/// Called on startup only.
static void preForkChildren()
{
- std::unique_lock<std::mutex> lock(newChildrenMutex);
+ std::unique_lock<std::mutex> lock(NewChildrenMutex);
int numPreSpawn = LOOLWSD::NumPreSpawnedChildren;
UnitWSD::get().preSpawnCount(numPreSpawn);
@@ -243,7 +243,7 @@ static void preForkChildren()
static void prespawnChildren()
{
- std::unique_lock<std::mutex> lock(newChildrenMutex, std::defer_lock);
+ std::unique_lock<std::mutex> lock(NewChildrenMutex, std::defer_lock);
if (!lock.try_lock())
{
// We are forking already? Try later.
@@ -252,13 +252,13 @@ static void prespawnChildren()
// Do the cleanup first.
bool rebalance = false;
- for (int i = newChildren.size() - 1; i >= 0; --i)
+ for (int i = NewChildren.size() - 1; i >= 0; --i)
{
- if (!newChildren[i]->isAlive())
+ if (!NewChildren[i]->isAlive())
{
- Log::warn() << "Removing unused dead child [" << newChildren[i]->getPid()
+ Log::warn() << "Removing unused dead child [" << NewChildren[i]->getPid()
<< "]." << Log::end;
- newChildren.erase(newChildren.begin() + i);
+ NewChildren.erase(NewChildren.begin() + i);
// Rebalance after cleanup.
rebalance = true;
@@ -266,13 +266,13 @@ static void prespawnChildren()
}
int balance = LOOLWSD::NumPreSpawnedChildren;
- balance -= newChildren.size();
+ balance -= NewChildren.size();
if (balance <= 0)
{
return;
}
- const auto duration = (std::chrono::steady_clock::now() - lastForkRequestTime);
+ const auto duration = (std::chrono::steady_clock::now() - LastForkRequestTime);
if (!rebalance &&
std::chrono::duration_cast<std::chrono::milliseconds>(duration).count() <= CHILD_TIMEOUT_MS)
{
@@ -285,26 +285,26 @@ static void prespawnChildren()
static size_t addNewChild(const std::shared_ptr<ChildProcess>& child)
{
- std::unique_lock<std::mutex> lock(newChildrenMutex);
- newChildren.emplace_back(child);
- const auto count = newChildren.size();
+ std::unique_lock<std::mutex> lock(NewChildrenMutex);
+ NewChildren.emplace_back(child);
+ const auto count = NewChildren.size();
Log::info() << "Have " << count << " "
<< (count == 1 ? "child" : "children")
<< "." << Log::end;
- newChildrenCV.notify_one();
+ NewChildrenCV.notify_one();
return count;
}
static std::shared_ptr<ChildProcess> getNewChild()
{
- std::unique_lock<std::mutex> lock(newChildrenMutex);
+ std::unique_lock<std::mutex> lock(NewChildrenMutex);
namespace chrono = std::chrono;
const auto startTime = chrono::steady_clock::now();
do
{
- const int available = newChildren.size();
+ const int available = NewChildren.size();
int balance = LOOLWSD::NumPreSpawnedChildren;
if (available == 0)
{
@@ -320,10 +320,10 @@ static std::shared_ptr<ChildProcess> getNewChild()
forkChildren(balance);
const auto timeout = chrono::milliseconds(CHILD_TIMEOUT_MS);
- if (newChildrenCV.wait_for(lock, timeout, [](){ return !newChildren.empty(); }))
+ if (NewChildrenCV.wait_for(lock, timeout, [](){ return !NewChildren.empty(); }))
{
- auto child = newChildren.back();
- newChildren.pop_back();
+ auto child = NewChildren.back();
+ NewChildren.pop_back();
// Validate before returning.
if (child && child->isAlive())
@@ -445,11 +445,11 @@ private:
// This lock could become a bottleneck.
// In that case, we can use a pool and index by publicPath.
- std::unique_lock<std::mutex> lock(docBrokersMutex);
+ std::unique_lock<std::mutex> lock(DocBrokersMutex);
//FIXME: What if the same document is already open? Need a fake dockey here?
Log::debug("New DocumentBroker for docKey [" + docKey + "].");
- docBrokers.emplace(docKey, docBroker);
+ DocBrokers.emplace(docKey, docBroker);
// Load the document.
std::shared_ptr<WebSocket> ws;
@@ -506,7 +506,7 @@ private:
Log::debug("Closing child for docKey [" + docKey + "].");
child->close(true);
Log::debug("Removing DocumentBroker for docKey [" + docKey + "].");
- docBrokers.erase(docKey);
+ DocBrokers.erase(docKey);
}
else
{
@@ -547,18 +547,18 @@ private:
const std::string formName(form.get("name"));
// Validate the docKey
- std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
+ std::unique_lock<std::mutex> DocBrokersLock(DocBrokersMutex);
std::string decodedUri;
URI::decode(tokens[2], decodedUri);
const auto docKey = DocumentBroker::getDocKey(DocumentBroker::sanitizeURI(decodedUri));
- auto docBrokerIt = docBrokers.find(docKey);
+ auto docBrokerIt = DocBrokers.find(docKey);
// Maybe just free the client from sending childid in form ?
- if (docBrokerIt == docBrokers.end() || docBrokerIt->second->getJailId() != formChildid)
+ if (docBrokerIt == DocBrokers.end() || docBrokerIt->second->getJailId() != formChildid)
{
throw BadRequestException("DocKey [" + docKey + "] or childid [" + formChildid + "] is invalid.");
}
- docBrokersLock.unlock();
+ DocBrokersLock.unlock();
// protect against attempts to inject something funny here
if (formChildid.find('/') == std::string::npos && formName.find('/') == std::string::npos)
@@ -582,9 +582,9 @@ private:
std::string decodedUri;
URI::decode(tokens[2], decodedUri);
const auto docKey = DocumentBroker::getDocKey(DocumentBroker::sanitizeURI(decodedUri));
- std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
- auto docBrokerIt = docBrokers.find(docKey);
- if (docBrokerIt == docBrokers.end())
+ std::unique_lock<std::mutex> DocBrokersLock(DocBrokersMutex);
+ auto docBrokerIt = DocBrokers.find(docKey);
+ if (docBrokerIt == DocBrokers.end())
{
throw BadRequestException("DocKey [" + docKey + "] is invalid.");
}
@@ -602,7 +602,7 @@ private:
{
throw BadRequestException("RandomDir cannot be equal to ChildId");
}
- docBrokersLock.unlock();
+ DocBrokersLock.unlock();
std::string fileName;
bool responded = false;
@@ -652,13 +652,13 @@ private:
const auto docKey = DocumentBroker::getDocKey(uriPublic);
std::shared_ptr<DocumentBroker> docBroker;
- // scope the docBrokersLock
+ // scope the DocBrokersLock
{
- std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
+ std::unique_lock<std::mutex> DocBrokersLock(DocBrokersMutex);
// Lookup this document.
- auto it = docBrokers.find(docKey);
- if (it != docBrokers.end())
+ auto it = DocBrokers.find(docKey);
+ if (it != DocBrokers.end())
{
// Get the DocumentBroker from the Cache.
Log::debug("Found DocumentBroker for docKey [" + docKey + "].");
@@ -672,7 +672,7 @@ private:
Log::debug("Inserting a dummy DocumentBroker for docKey [" + docKey + "] temporarily.");
std::shared_ptr<DocumentBroker> tempBroker = std::make_shared<DocumentBroker>();
- docBrokers.emplace(docKey, tempBroker);
+ DocBrokers.emplace(docKey, tempBroker);
}
}
@@ -687,16 +687,16 @@ private:
{
std::this_thread::sleep_for(std::chrono::milliseconds(timeout));
- std::unique_lock<std::mutex> lock(docBrokersMutex);
- auto it = docBrokers.find(docKey);
- if (it == docBrokers.end())
+ std::unique_lock<std::mutex> lock(DocBrokersMutex);
+ auto it = DocBrokers.find(docKey);
+ if (it == DocBrokers.end())
{
// went away successfully
docBroker.reset();
Log::debug("Inserting a dummy DocumentBroker for docKey [" + docKey + "] temporarily after the other instance is gone.");
std::shared_ptr<DocumentBroker> tempBroker = std::make_shared<DocumentBroker>();
- docBrokers.emplace(docKey, tempBroker);
+ DocBrokers.emplace(docKey, tempBroker);
timedOut = false;
break;
@@ -753,8 +753,8 @@ private:
if (!newDoc)
{
// Remove.
- std::unique_lock<std::mutex> lock(docBrokersMutex);
- docBrokers.erase(docKey);
+ std::unique_lock<std::mutex> lock(DocBrokersMutex);
+ DocBrokers.erase(docKey);
#if MAX_DOCUMENTS > 0
--LOOLWSD::NumDocBrokers;
#endif
@@ -765,8 +765,8 @@ private:
if (newDoc)
{
- std::unique_lock<std::mutex> lock(docBrokersMutex);
- docBrokers[docKey] = docBroker;
+ std::unique_lock<std::mutex> lock(DocBrokersMutex);
+ DocBrokers[docKey] = docBroker;
}
// Check if readonly session is required
@@ -827,7 +827,7 @@ private:
// Connection terminated. Destroy session.
Log::debug("Client session [" + id + "] terminated. Cleaning up.");
{
- std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
+ std::unique_lock<std::mutex> DocBrokersLock(DocBrokersMutex);
// We cannot destroy it, before save, if this is the last session.
// Otherwise, we may end up removing the one and only session.
@@ -866,9 +866,9 @@ private:
if (sessionsCount == 0)
{
- std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
+ std::unique_lock<std::mutex> DocBrokersLock(DocBrokersMutex);
Log::debug("Removing DocumentBroker for docKey [" + docKey + "].");
- docBrokers.erase(docKey);
+ DocBrokers.erase(docKey);
#if MAX_DOCUMENTS > 0
--LOOLWSD::NumDocBrokers;
#endif
@@ -1695,7 +1695,7 @@ Process::PID LOOLWSD::createForKit()
Log::info("Launching forkit process: " + forKitPath + " " +
Poco::cat(std::string(" "), args.begin(), args.end()));
- lastForkRequestTime = std::chrono::steady_clock::now();
+ LastForkRequestTime = std::chrono::steady_clock::now();
Pipe inPipe;
ProcessHandle child = Process::launch(forKitPath, args, &inPipe, nullptr, nullptr);
@@ -1884,8 +1884,8 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
{
try
{
- std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
- for (auto& brokerIt : docBrokers)
+ std::unique_lock<std::mutex> DocBrokersLock(DocBrokersMutex);
+ for (auto& brokerIt : DocBrokers)
{
brokerIt.second->autoSave(false, 0);
}
@@ -1923,7 +1923,7 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
// Terminate child processes
Log::info("Requesting child process " + std::to_string(forKitPid) + " to terminate");
Util::requestTermination(forKitPid);
- for (auto& child : newChildren)
+ for (auto& child : NewChildren)
{
child->close(true);
}
@@ -1978,9 +1978,9 @@ namespace Util
void alertAllUsers(const std::string& cmd, const std::string& kind)
{
- std::lock_guard<std::mutex> docBrokersLock(docBrokersMutex);
+ std::lock_guard<std::mutex> DocBrokersLock(DocBrokersMutex);
- for (auto& brokerIt : docBrokers)
+ for (auto& brokerIt : DocBrokers)
{
brokerIt.second->alertAllUsersOfDocument(cmd, kind);
}
commit 15c7899221ed1e016e3fe53be6e1136228df4b32
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun Oct 16 11:06:59 2016 -0400
loolwsd: limit test documents and connections to the config
...if configured with limits.
Change-Id: Ic148f725c58485ea88f62ddf7b4ac47b3b43ff04
Reviewed-on: https://gerrit.libreoffice.org/29951
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 72fb908086ab6d4aa3e3fab038e82a6c198d8922)
diff --git a/loolwsd/test/httpwstest.cpp b/loolwsd/test/httpwstest.cpp
index 2218a9f..bcd8169 100644
--- a/loolwsd/test/httpwstest.cpp
+++ b/loolwsd/test/httpwstest.cpp
@@ -1895,7 +1895,12 @@ void HTTPWSTest::testEachView(const std::string& doc, const std::string& type, c
CPPUNIT_ASSERT_MESSAGE(Poco::format(error, itView, protocol), !response.empty());
// Connect and load 0..N Views, where N=10
- for (itView = 0; itView < 10; ++itView)
+#if MAX_DOCUMENTS > 0
+ const auto limit = std::min(10, MAX_DOCUMENTS - 1); // +1 connection above
+#else
+ constexpr auto limit = 10;
+#endif
+ for (itView = 0; itView < limit; ++itView)
{
views.emplace_back(loadDocAndGetSocket(_uri, documentURL, Poco::format(view, itView)));
}
commit 41433c2e72acd89180dd9a3f4ac8ca301db990cc
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun Oct 16 13:43:44 2016 -0400
loolwsd: argument cleanup
Pass std::string instead of char* and length where
a string is always constructed anyway. Also cleaner
and safer code.
Change-Id: I1c9341e2c81bbdb7adeb29d3fba59849b2617e95
Reviewed-on: https://gerrit.libreoffice.org/29954
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit d266d124dfb5383c113996d799d3efe8274e0a99)
diff --git a/loolwsd/ClientSession.cpp b/loolwsd/ClientSession.cpp
index b46fe73..2aad600 100644
--- a/loolwsd/ClientSession.cpp
+++ b/loolwsd/ClientSession.cpp
@@ -172,7 +172,8 @@ bool ClientSession::_handleInput(const char *buffer, int length)
}
else if (tokens[0] == "status")
{
- return getStatus(buffer, length, docBroker);
+ assert(firstLine.size() == static_cast<size_t>(length));
+ return forwardToChild(firstLine, docBroker);
}
else if (tokens[0] == "tile")
{
@@ -188,12 +189,12 @@ bool ClientSession::_handleInput(const char *buffer, int length)
if ( (isReadOnly()) && tokens[0] != "downloadas" &&
tokens[0] != "userinactive" && tokens[0] != "useractive")
{
- std::string dummyFrame = "dummymsg";
- return forwardToChild(dummyFrame.c_str(), dummyFrame.size(), docBroker);
+ const std::string dummyFrame = "dummymsg";
+ return forwardToChild(dummyFrame, docBroker);
}
else if (tokens[0] != "requestloksession")
{
- return forwardToChild(buffer, length, docBroker);
+ return forwardToChild(std::string(buffer, length), docBroker);
}
else
{
@@ -243,7 +244,7 @@ bool ClientSession::loadDocument(const char* /*buffer*/, int /*length*/, StringT
oss << " options=" << _docOptions;
const auto loadRequest = oss.str();
- return forwardToChild(loadRequest.c_str(), loadRequest.size(), docBroker);
+ return forwardToChild(loadRequest, docBroker);
}
catch (const Poco::SyntaxException&)
{
@@ -253,11 +254,6 @@ bool ClientSession::loadDocument(const char* /*buffer*/, int /*length*/, StringT
return false;
}
-bool ClientSession::getStatus(const char *buffer, int length, const std::shared_ptr<DocumentBroker>& docBroker)
-{
- return forwardToChild(buffer, length, docBroker);
-}
-
bool ClientSession::getCommandValues(const char *buffer, int length, StringTokenizer& tokens,
const std::shared_ptr<DocumentBroker>& docBroker)
{
@@ -273,7 +269,7 @@ bool ClientSession::getCommandValues(const char *buffer, int length, StringToken
return sendTextFrame(cmdValues);
}
- return forwardToChild(buffer, length, docBroker);
+ return forwardToChild(std::string(buffer, length), docBroker);
}
bool ClientSession::getPartPageRectangles(const char *buffer, int length,
@@ -285,7 +281,7 @@ bool ClientSession::getPartPageRectangles(const char *buffer, int length,
... etc. - the rest is truncated
More information about the Libreoffice-commits
mailing list