Sessions are referrenced in DocumentBroker instances,
which themselves are referrenced in a container.
When exceptions are thrown either while creating a new
session, or during the lifetime of one, these references
must be correctly cleaned up, otherwise we introduce
internal instability in addition to stalling the client.
Change-Id: I3177e45564860897528da6d7fbcbe346d3bd1c75
Reviewed-on: https://gerrit.libreoffice.org/24338
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
The invalidatetiles is normally a notification coming from
LOK and it signifies that the tiles in quesion need
rendering anew. Issuing this internally from the Kit
removes TileCache images unnecessarily.
Furthermore, since this message is always sent in response
useractive message, there is no need in issuing it from
WSD when loleaflet is perfectly capable of issuing it
itself (internally).
Change-Id: Ia97de6d803745dca3f6e73100f2d921dbbdf76f6
Reviewed-on: https://gerrit.libreoffice.org/24316
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Use info() instead. Also, only log the info if the file actually
existed and was removed. We don't want misleading noise logging about
removing files that were not there. Use std::remove() directly to
avoid unnecessary layers that just make it harder to know whether the
removal worked or not.
All changes are supposed to be persistent. This simplifies the tile
caching code quite a lot.
The TileCache object no longer needs to keep any state whether the
document is being edited or whether it has been modified without
saving etc.
Update the modtime.txt file after saving the document. Otherwise the
tile cache would wrongly be considered invalid next time.
As a sanity check, we put a flag file 'unsaved.txt' into the cache
directory whenever we get a callback that indicates the document has
been modified, and remove it when the document is saved. If the flag
file is present when we take an existing tile cache into use, we can't
trust it.
Even after these changes, we still don't use an existing tile cache as
much (or at all?) as we could, though. The INVALIDATE_TILES EMPTY
callback that LO does early on in a conection causes us to remove all
cached tiles...
These notifications are important to be sent once the user
becomes active again to sync their view with the latest.
Change-Id: Id8f9fff83eea888cdcc8d6ed1d4f12111de39a6e
Reviewed-on: https://gerrit.libreoffice.org/24288
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This also avoids the feedback loop that results from the kit
thinking the previously inactive client is now active and
sending commands (.uno:Save).
Change-Id: I47074b35a922da15592d550032d494ba1efab83e
Reviewed-on: https://gerrit.libreoffice.org/24287
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This test requires the renderid parameter to be present in the 'tile:'
response messages, and that is the case only when ENABLE_DEBUG, so we
can run the test only in a debug build.
In a debug build it contains also the parameter renderid. (And in the
future, might be extended in other ways, too.) Construct a new request
message that has exactly and only the parameters we want.
loleaflet can now send userinactive when the user
has switched tabs or the browser window loses
focus. Similarly, it can send useractive when
focus is regained.
Change-Id: Id3186949b10a8263e29ada1a790d3123a79e8f08
Reviewed-on: https://gerrit.libreoffice.org/24272
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Will be used in unit test to verify that several clients of the same
document asking for the same tile simultaneously indeed do cause just
one tile rendering to take place.
It is still possible to access them directly via loleaflet/dist/<something>,
but such use can lead to unexpected behaviour due to various caching in the
browsers etc.
Implement this for tilecombine, and do tile writes in each client's
thread separately. Add env-var. to trigger sleep, and tune it to 1
second; easily long enough to exercise this code-path.
Keep track of tiles being rendered in TileCache, and when asked to
render the same tile as is already being rendered, just "subscribe" to
the existing ongoing rendering. When a tile has been rendered and is
being sent out to clients, check if there are "subscriptions" and send
it to them, too.
One problem is that if the client that caused a tile rendering to be
initiated goes away before the rendering has completed, it will never
complete, and the subscribers are left without the tile.
Change-Id: Icca237876a0f466c29eb5bf60ffd4da3d9d68600
Reviewed-on: https://gerrit.libreoffice.org/24228
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Since auto-discovery is problematic, this patch implements
support for both regex patterned hostnames/IPs to allow,
and those to block/deny.
A hostname/IP must be both allowed, and not denied, to
be accepted.
By setting ranges of allowed hostnames/IPs, and others
to block/deny, an admin can configure Online with
great flexibility.
Defaults updated with same values, but not exhaustive.
Change-Id: Iedfcafe41d07d905b549fb450c3fe625ad44599e
Reviewed-on: https://gerrit.libreoffice.org/24233
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
The closing handshake.
Either peer can send a control frame with data containing
a specified control sequence to begin the closing handshake.
Upon receiving such a frame, the other peer sends a
Close frame in response, if it hasn't already sent one.
When compiled with --enable-debug, when requesting a tile for part=42,
actually use part=0, and sleep five seconds before passing the
rendered tile back up. This makes it easier to debug handling of
simultaneous requests for the same tile from multiple clients.
In loolforkit, whenever we have forked a new loolkit, also check if
any previously forked children have exited. Remove the jails of
those. (The loolkit process itself does not even try to remove all of
its jail, see 3aadd910c6e32c0e557671effa5a4c606cd6e8bf.)
In order to be able to notice exited child processes in loolforkit, we
no longer can set the action for SIGCHLD to SIG_IGN. That means that
exiting loolkit processes will be in the zombie state until loolforkit
picks up their exit status. As loolforkit does this check only in
connection with forking a new child, zombie loolkit processes will
hang around for some time, until the next loolkit process is
forked. Not sure if this is a problem.
countLoolKitProcesses() in httpwstest now needs to skip zombies.
Loolwsd still takes care of removing whatever jails are left when it
finishes.
When inside the chroot, what we would need to do is remove everything
below / . But doing that is a bit too risky, in case some developer
screws up some detail and that code happens to run outside the chroot
after all, and the developer's machine gets trashed. So just remove
paths we can reasonably assume won't exist as global pathnames on a
developer machine: loSubPath and JAILED_DOCUMENT_ROOT.
Currently the actual complete cleanup of loolkit jails happens in
loolwsd when it is exiting. That is a bug and will have to be
fixed. It should be done in loolforkit as soon as possible after the
loolkit process has exited.
At least, that is the value of the num_prespawn_children element in
the loolwsd.xml as shipped. But maybe that is not what is meant with
"default"? It is unclear to me what the "default" attribute means.
When a new view is created on a document that is
in the process of unloading, all sorts of things
can go wrong. This is especially problematic when
the document needs to be saved before unloading,
which takes significantly longer than otherwise.
Change-Id: Ib33a18cafa9d5a3a17f6bd8c6145f9331ae54044
Reviewed-on: https://gerrit.libreoffice.org/24184
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Normally, when each client view closes, the
session count is decremented until the last
view is closed. However this doesn't work
when the kit child process terminates.
Due to a race condition between the last
client disconnecting, and the internal
structure destructing, and the next
client connecting (on the same doc),
the Admin loses track of the doc and pid.
This is an issue of assuming a document
and its pid are unique and will always
remain unchanged.
This patch adds a new API to remove a
doc and all its views unconditionally
to try to avoid the above issues.
Change-Id: I0c181260679875b0464dd9b6548b29b8d6a361f7
Reviewed-on: https://gerrit.libreoffice.org/24183
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Standardized error handling in request-handlers.
There is a new family of internal exeptions designed
to signify the type of error and how to handle it.
All handlers must throw one of those errors
and they will be translated to the correct HTTP
response when caught.
Since some requests send a response as part of their
handling (convert-to, for example) those handlers
must return a flag signlaning whether or not they
sent a response. If not, HTTP OK response is sent
at the end of the handler.
To complicate things, some requests upgrade the
connection to WebSocket. In those cases errors
must be sent via the WebSocket and not as an
HTTP response. The error message sent can (and
in most cases should) be displayed to the end-user.
A new file, UserMessages.hpp, has been added to
hold user-visible messages that can be
reviewed and translated.
Change-Id: Icc725f3313446d4514cf6d092635158ee7171f5d
Reviewed-on: https://gerrit.libreoffice.org/24133
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This ensures that bundled fonts in instdir/share end up resolved to
the same path that they were in when the forkit font config was setup.
It may also help locate other pre-inited resources.
Also copy in ~/.fonts in debug mode - can't hurt.
SocketProcessor doesn't need to take response
instance, since by the time it is called we
are already upgraded to WebSocket and it's
too late to set a request-level status.
Change-Id: Id95087e60354a50148c88427130613356679cf82
Reviewed-on: https://gerrit.libreoffice.org/24110
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Connecting to a Kit process is managed by document broker, that it does several
jobs to establish the bridge connection between the Client and Kit process,
The result, it is mostly time outs to get messages in the unit test and it could fail.
connectLOKit ensures the websocket is connected to a kit process.
Some messages are not forwarded to the client session, this is caused
by the time the client session is assigned, the prison session,
it is already forwarding to not assigned peer session.
Sleep in both places before counting them, and catch Poco exceptions
while counting (can happen for instance when a /proc entry goes away
while we are looking for its 'comm' file).
Do not distinguish between normal shutdown or abnormal shutdown.
Also remove 'disconnect' frame to indicate normal shutdown.
Change-Id: I98fd9f5a219feb1097c57302dba14e08ad9bf143
The number of loolkit processes after running the cppunit tests should
be the same as before running them. (At least, that is the intention
at the moment, I think. If/when this changes, the test will have to be
adapted, of course.)
That global flag is checked all over the place, so setting it will
actually make the threads eventually finish. (All polling is done with
timeout, I think, and then checking TerminationFlag whenever the poll
times out.)
Sure, it would be much better to use an eventfd and poll that, too,
instead of timing out from the polls all the time to check a plain old
boolean flag.
If you can't come up with a meaningful message to use in a
CPPUNIT_ASSERT_MESSAGE(), just use CPPUNIT_ASSERT() instead. These
messages aren't intended for end-users but for developers, so it is
pointless to make them high-level and dumbed-down.
In case of abnormal termination of session from client-side,
we might still have data being processed in the kit process, for
example, during auto-save. Trying to send such data over an
expired socket towards the client would throw exceptions which
need to be handled, otherwise the auto-save process would not be
successful. Further, the unhandled exception can bring the document
broker in an unstable state with dockey still present in the
object.
Also do not send 'editlock: 0' to a websocket session which is
going to be removed because the session might have been
forcefully terminated from the client-side, in which case sending
data would go nowhere.
Change-Id: I10eb9c818bc81d4db26d5a19dc8bd44f6fbdf32c
Enforce user being 'lool' for setcap binaries loolmount and loolforkit.
Add warnings if configured without --enable-debug.
Developers should pass --enable-debug to configure.
The loolwsd process created it and opened it for reading, but nothing
opened it for writing.
There is still documentation for it in README, that needs to be either
rewritten to match reality or removed.
Comes in handly in some testing situations where you don't want to
send a signal to get loolwsd to finish. Option is present only in an
--enable-debug build.
If the first session used to save does not hold the edit-lock,
all the .uno:Save commands will get converted to dummy messages
and never reach the kit process.
Change-Id: I36cbee778f8c2c5866dcf276cf156fdf9ed8388e
It is a status code, a 2-byte unsigned integer in network byte order,
potentially followed by a textual reason. But we never include any
specific status codes in the CLOSE frames anyway. (With a Poco-based
WebSocket peer it is always WS_NORMAL_CLOSE, 1000, 0x03 0xE8.)
Since WSD now takes into account the fact that
ForKit always spawns one child, the unit-test
shouldn't expect +1 preforked children.
Change-Id: I5cbe9d817a0d2ffdf9fb0953ef85450f7b8b224f
Reviewed-on: https://gerrit.libreoffice.org/23980
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Last client disconnection now correctly issues a save
and waits for the confirmation before tearing down
the sockets, queues and threads.
Change-Id: I28c28d79a17d359e9aa1fe67b983ca9fb592b847
Reviewed-on: https://gerrit.libreoffice.org/23978
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Connection thread should not attempt to disconnect the session,
which in turn will try to unload the document, which will
wait on the connection to destroy. The latter will never
happen since the connection destructor must, correctly,
wait for its thread to finish, which is waiting on itself now.
Since the session disconnect is already called from the session
destructor, there is no need to explicitly invoke it here.
Change-Id: Iaf9e8a10d4caa9001208084e909a14b4d4c5105e
Reviewed-on: https://gerrit.libreoffice.org/23966
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
The sessions container already has the number of sessions.
No need for separate counters to track them.
Change-Id: I838865e2b8a843e87e81a6cc1226bcacd774b032
Reviewed-on: https://gerrit.libreoffice.org/23964
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Autosaving is done by DocumentBroker, which
tracks the last save time.
There are two triggers: idle and auto save.
The first triggers when sufficient time passes
after the last interaction the user had with
the UI (currently 30 seconds).
The second triggers when it's been more than
5 minutes since the last save.
Both triggers are conditional on the user
being active after the last save.
The new code auto-saves doesn't issue
a save command per session, but only
one per doc.
Change-Id: Iada15c16002e70710d2c13a3dcfdab036d8935c6
Reviewed-on: https://gerrit.libreoffice.org/23951
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This seems to get rid of the "terminate called after throwing an
instance of 'Poco::SystemException'" problem for me at least.
Sigh. Why can't the compiler warn about such things? I build with
clang++ -Wall -Wextra. The Connection class is fully defined inside
the LOOLKit.cpp so it should be able to, right?
Presumably it is only developers that are interested in signals, and
terms like SEGV or ABRT are more precise than their textual
descriptions like "Segmentation violation" or "Aborted".
Sleep a second before exiting in case we get a fatal signal just when
about to finish, which sadly seems to happen often. (In fact, if
handleFatalSignal() is running at the same time, it will kill the
process so we never get to the _Exit() call.)
Also, display more information about the exception.
Actually I think I should factor out the code to display as much
information as possible from an exception. Currently the amount of
information displayed varies from case to case in the code-base.
Poco::Channel is reference counted, but the initial refcount is 1, so we
need to release channel in order to have it deleted when Poco::Logger
releases it.
Calls to Poco::Logger::shutdown() are still missing though (from
forkit/kit/wsd).
Change-Id: I12ab32047d32e55902c60639d71eb6ef30ffa3bd
Put this inside #if, add the 'override' back for current Poco. Avoids
"warning: 'expectContinue' overrides a member function but is not
marked 'override'" when compiling against current Poco.
Not sure what good it would do to define it at all if compiling
against a bleeding-edge Poco as it won't call it anyway?
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.
