The logging functions already display the thread name on all output
lines. No need to mention it another time in the thread start and
finish logging messages.
A call to Log::error() should be enough to indicate that it is an
error. We don't need to prefix the message with the string "Error: "
in some cases but not others. (If we do want such a prefix for all
errors, surely then we should add it in the actual Log::error()
function.)
Also, change some more Log::error() calls to Log::syserror() where
appropriate.
Much better than assuming that errno would be relevant at all
Log::error() calls (or alternatively, having to remember to append a
false parameter to the Log::error() call, which had not been done a
single time anyway.)
Call log::syserror() right after a system call has returned an
error. Don't call it otherwise.
Loading documents from the local filesystem
opens the door to security issues.
By default filesystem storage is disabled,
even if enabled in the config file. The
only way to enable it is to set the
allowlocalstorage command-line argument.
Change-Id: Ib8f57377260817436d101a16757aab38276cbdcd
Reviewed-on: https://gerrit.libreoffice.org/23881
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Most all configuration values can now be defined
in the configuration XML. The command-line arguments
can be used to override some of these values (for
the convenience of developement and testing) and,
in a few cases, as a security measure to avoid
storing sensitive data in the configuration file.
Change-Id: I040b807b1a59a3537bb94646150d3c7d711f8b62
Reviewed-on: https://gerrit.libreoffice.org/23880
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
A new command-line argument, admincreds, must be provided
to set the Admin Console credentials.
The new command-line argument specifies the username
and password in the following format: username/password
If not provided, Admin Console is disabled for security
reasons. A warning is emitted at startup and an error
on every invocation of Admin Console is logged when
no credentials are defined.
Change-Id: I348623949fd0b292f5066e4955759c708204540f
Reviewed-on: https://gerrit.libreoffice.org/23878
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
loolstat now uses `pgrep loolwsd$` instead
of relying on pid dumping in a temp file.
With the Admin Console this tool (loolstat)
is less useful, so this cleanup is probably
a stepping stone to removing it altogether.
Change-Id: Ib7732a00c3d3ea54dffcb71e9fe1a56c4a88016e
Reviewed-on: https://gerrit.libreoffice.org/23877
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Admin no longer needs a pipe as it's notified
from WSD. It is now a singleton with improved
locking.
The tracking of documents and views still needs
improvement and corrections.
Change-Id: If614331de6dd595c6dd4443f480d4ab588ca4551
Reviewed-on: https://gerrit.libreoffice.org/23860
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
To use such enums would be a mistake. It is quite enough to just use
the message tokens as strings. Duplicating them as enums will just
lead to the enums getting out of synch (as they already were). We
would also need functions to covert between the string and enum
forms. It seems to be hard enough to keep the messages documented in
protocol.txt.
Add a function to determine whether a client message indicates user
interaction. We need that distinction when deciding when to do an
automatic ("idle" or "auto") save of document being edited.
"Interaction" is a loose term, possibly what we actually want is to
see whether the user is actively doing an edit that changes the
contents of meta-data of the document.
In the instdir/program of an --enable-mergelibs build of LibreOffice
there is also a dummy libsofficeapp.so. Obviously we don't want to
even try that. So look for libmergedlo.so first.
I easily got the thing into a state where it kept forking new
processes without limit even if no client was doing anything. The log
output contained lines like:
wsd-09526-04 00:00:48.302029 [client_ws_001b ] MasterToBroker: spawn -1
brk-09528-00 00:00:48.279945 [loolbroker ] Broker command: [spawn -1].
brk-09528-00 00:00:48.279964 [loolbroker ] Spawning -1 childs per request.
brk-09528-00 00:00:48.279974 [loolbroker ] Creating -1 new child.
This fixes that, but there are still several issues remaining.
So just use DEFAULT_CLIENT_PORT_NUMBER and don't confusingly include
ChildProcessSession.hpp in the test program sources just to define a
static (file-local) ClientPortNumber variable that never gets set to
anything except its initialized value. ChildProcessSession is specific
to the internals of loolwsd and loolbroker and should not be used in
the test programs.
Preiniting LibreOfficeKit and forking kit processes (instead of
spawning) has worked fine for a while, and has been the default way
this works.
No 'loolkit' program gets built any more.
Convert-to is broken after re-designing Broker
and simplifying it. Temporarily disabling it
to help `make check` pass until it gets fixed.
Change-Id: Id49f86d8a1a25367233a09b865630ed3a210a4fd
Reviewed-on: https://gerrit.libreoffice.org/23793
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Broker is now just a simple spawn-machine.
It only gets requests from WSD to spawn
new kit instances, which it doesn't even
track.
Once a kit instance is initialized, it
connects to WSD with a control WS.
From there on, it's up to WSD to manage
the kit process.
Also was removed the benchmark logic
since it can no longer function.
Change-Id: I1bf56bc6416c9eadafba637276bbb8b3107e5727
Reviewed-on: https://gerrit.libreoffice.org/23790
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
WSD now communicates on a WebSocket directly
with kit processes. ChildProcess encapsulates
kit processes and the control WS, which itself
is owned by DocumentBroker.
Change-Id: Ica209aaa07974739b8e51a14e11325d084e193f6
Reviewed-on: https://gerrit.libreoffice.org/23789
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
In face of exceptions, the lock was not released
and the condition variable was not signalled,
thereby causing all subsequent views on the
same document to fail loading.
Change-Id: I18d3cefcc74a158facefe1e74a9c802ee048b014
Reviewed-on: https://gerrit.libreoffice.org/23785
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
of the 10k files still linked into the jail; 5700 are from usr/
so bind mount just that directory, also set noatime, ro, and
some other helpful looking options.
Change-Id: I28d2d5cbbdf33fb57ea0f0c0915cb267603ee16d
Reviewed-on: https://gerrit.libreoffice.org/23777
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Create directories top-down and not bottom up for more efficiency too.
Skip the sdk (if we have it) - ~20k files, and misc. other
pieces that we don't need; still more to go there.
Change-Id: Iccd9ebac495fba414d268b339ef82a161d98a9ca
Reviewed-on: https://gerrit.libreoffice.org/23770
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This reverts commit 97c8f35ddf.
Since the Broker design has been extremely simplified,
all communication between Broker <-> Kit are gone.
Only a pipe between WSD and Broker remain.
Temporarily reverting this to apply the Broker redesign,
after which this patch can be reviewed and merged.
This will be easier than trying to merge the redesigned
Broker on top of this.
Change-Id: Ia901fad604008654c01841df62e88918adad45e1
Reviewed-on: https://gerrit.libreoffice.org/23769
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
There is nothing that says a client has even requested any tiles, so
there might be none to persist. Don't let an exception thrown by the
DirectoryIterator propagate upwards and cause potential
issues.
Noticed the issue when testing using the 'connect' test program,
giving it input that did not request any tiles.
Had to add a shared pointer to the BasicTileQueue for the session to
the MasterProcessSession object, and restructure the coe a a bit to
allocate BasicTileQueue objects dynamically. Possibly just passing a
reference to a BasicTileQueue in the stack would have worked, but why
risk it?
The actual logic when to do auto / idle save is not quite right still,
did not change that in this commit.
Time increment is handled on client-side, server only sends the
elapsed time during first page-load.
Change-Id: I73e98fd95ca9f391b625a8dcfc7e3490878c6a40
... which can be taken only one at a time. Others can only view,
not edit. When a session with edit lock exits, the edit lock is
handed over to the next alive session.
Change-Id: I712a4e70369f1d07c1d83af416a0f5c288b05c7d
Don't embed newlines in "lines" written to the log. When logging stuff
read from or written to the fifos, translate newlines to " / " for
clarity.
(If we would want complete, exact verbose logging, we should be really
pedantic and log all non-printable bytes in hex anyway, etc, so
displaying newlines as space-separated slashes should be OK. It isn't
as if there would be totally arbitary data passed through the fifos
anyway.)
We will switch to reading all these values as soon as wip
configuration file object is accessible globally.
Change-Id: I16eff339349683050be1985eefabc34854cccba3
We act as a client when we communicate with owncloud server.
For time being, just accept whatever certificates owncloud server
gives us. We might want to get more strict here in future.
Change-Id: I4813d19412b66ecf57d6cdef9c3ac94fbbaa521f
Spare child processes are now in a separate
container. A FIFO that gives older instances
priority to avoid using instances too young
to have initialized fully.
In addition, spare instances are now
proactively spawned such that there
is at least a minimum number of
spares at any given time.
Change-Id: Ibdb206d88473adb306c274f4af39798c784258a0
Reviewed-on: https://gerrit.libreoffice.org/23647
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
All messages now pass through the queue.
This resolves a race between single-line
messages and multi-line ones.
Previously, single-line messages were
processed on the queue (on a background
thread) while multi-line ones were handled
immediatly. This resulted in order-inversion
due to a race between the queue thread and the
next multi-line message, which caused stability
issues every so often.
Change-Id: Ia220791d1d75c4f3e3e0965dd0c6f81bae63a296
Reviewed-on: https://gerrit.libreoffice.org/23583
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
It is not a good idea to have the same string somewhat arbitrarily
both as a static const members of the LOOLWSD class and then as a
file-local static const in another file. Or defined as a separate
local const static in each compilation unit that includes
Common.hpp. Use constexpr instead, in Common.hpp.
This is C++, not Java. Or is there a school of thought for C++ style
that says one should avoid plain "C-style" file-local static
variables, and instead put everything always in a class, even as
static members? Do we want to follow that?
Dumping of registered tests in preparation
to allow for selective test running.
Change-Id: I83af1c9be211804f757918c326602a0b53815704
Reviewed-on: https://gerrit.libreoffice.org/23531
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
The plan was to keep the document's open and close history right
from the moment the server started up to the end, so we would
only expire() the document. For now, just destroy the document
as soon as user session is closed.
Change-Id: Id8f5b550a2b9bda217f7f8f8f1f82f85b1aa3502
CollaboraCloudSuiteCA_ca-chain.cert.pem is the CA chain that should
be trusted. Move the file to directory of trusted certs, such as
/usr/share/pki/trust.
Configuration XML is added with SSL as sample use-case.
A 'desc' attribute can be used to describe the fields,
and another 'type' to help define the corresponding data
type in the code.
Since Poco allows accessing group nodes (that have the
same name) by index, order can be preserved.
SSL initialization refactored and cert/key file
paths moved to the config file.
Change-Id: I259826a19697bd851587bebcc4f0cd233ab6848b
Reviewed-on: https://gerrit.libreoffice.org/23464
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
When the Document lock cannot be taken
purging doesn't block (which would block
the kit-broker pipe). Instead, purging
is done only when the lock is taken,
otherwise we try again later.
Change-Id: Id201f1c67803d9b1e765e8c55f85206795fe53c0
Reviewed-on: https://gerrit.libreoffice.org/23448
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Tests can modify the test documents they use.
Currently there is data-loss protection that
saves an open doc if connection is lost with
the client. For tests this means modification
are saved when a connection is terminated
ungracefully and this both adds noise
to the git checkout and makes subsequent
tests fail.
This patch makes temp copies of the original
doc before a test is run and deletes them
afterwards.
Change-Id: I1dd6ff2b839701e85c8bd502ba75170c01fa106e
Reviewed-on: https://gerrit.libreoffice.org/23447
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Automake already have rules, make tags and make ctags, for emacs
and vim style tags respectively. We can pass our custom flags to
them using the AM_ macros.
https://www.gnu.org/software/automake/manual/html_node/Tags.html
This commit also adds support to create emacs style tags using
automake's pre-generated tag rules.
Change-Id: I4f6ed997fab6964b3c1f6637e3fd0365f8d4c8b8
Reviewed-on: https://gerrit.libreoffice.org/23442
Reviewed-by: pranavk <pranavk@collabora.com>
Tested-by: pranavk <pranavk@collabora.com>
Poco/SharedPtr.h: In instantiation of ‘static void Poco::ReleasePolicy<C>::release(C*) [with C = Poco::Net::PrivateKeyPassphraseHandler]’:
Poco/SharedPtr.h:130:14: required from ‘Poco::SharedPtr<C, RC, RP>::SharedPtr(C*) [with C = Poco::Net::PrivateKeyPassphraseHandler; RC = Poco::ReferenceCounter; RP = Poco::ReleasePolicy<Poco::Net::PrivateKeyPassphraseHandler>]’
Poco/SharedPtr.h:70:3: warning: possible problem detected in invocation of delete operator: [-Wdelete-incomplete]
delete pObj;
Poco/SharedPtr.h:66:25: warning: ‘pObj’ has incomplete type
static void release(C* pObj)
Poco/Net/PrivateKeyFactory.h:30:7: note: forward declaration of ‘class Poco::Net::PrivateKeyPassphraseHandler’
class PrivateKeyPassphraseHandler;
Just a skeleton, actual saving not yet implemented. Also, not sure
the logic when to trigger save is as intended.
Note that no separate timer classes or objects are used. The existing
watpid/sleep loop that wakes up once every two seconds currently is
used. If that loop is re-factored to be less silly, the auto/idle save code
must be implemented differently.
When no child process has died, I don't see the point in calling
waitpid() eleven times in quick succession (with WNOHANG), doing
nothing else, and then sleeping for a bit. Let's call waitpid() just
once, and sleep only if the return value indicates that no child
process has died.
Use same port (9989) for all client connections. This includes
admin panel, static file serving and normal client websocket
connections.
Change-Id: Idcfd7dd8925523c36e884717c41a3b6a827f6ff3
File server serves the admin html file after successfull
authentication, and sets the cookie in client which would be sent
for all subsequent connections by client to connect to admin websocket.
Change-Id: I0ee3bbfca7eefc428020d29612374410556b1e27
Only purpose, at the moment, is to create
Poco::HTTPRequestHandler which would be passed on the serving
handling the static file requests.
Change-Id: I97c3fc0c73da077d3efee919416098b880c9c2ad
... and use SSL for client connections. Also fix our test suite
to use HTTPS now.
Change-Id: Id396a7c2d1830da8d3b0ce446522403363ac17c1
Reviewed-on: https://gerrit.libreoffice.org/23395
Reviewed-by: Tor Lillqvist <tml@collabora.com>
Tested-by: Tor Lillqvist <tml@collabora.com>
File server serves the admin html file after successfull
authentication, and sets the cookie in client which would be sent
for all subsequent connections by client to connect to admin websocket.
Change-Id: I0ee3bbfca7eefc428020d29612374410556b1e27
Only purpose, at the moment, is to create
Poco::HTTPRequestHandler which would be passed on the serving
handling the static file requests.
Change-Id: I97c3fc0c73da077d3efee919416098b880c9c2ad
... and use SSL for client connections.
Change-Id: Id396a7c2d1830da8d3b0ce446522403363ac17c1
Reviewed-on: https://gerrit.libreoffice.org/23395
Reviewed-by: Tor Lillqvist <tml@collabora.com>
Tested-by: Tor Lillqvist <tml@collabora.com>
To make room for other classes deriving from AuthBase that do not
require any authorization code, such as JWT authentication.
Change-Id: I69a35dd6f775badd7377949df2ca326c910d4021
This is to distinguish the deliberate close of connection, and timeout,
connection drop, or forced close.
When the last session is closed non-deliberately, force a save so that the
edits are not lost.
