When WOPI's CheckFileInfo or GetFile responds with status code other
than HTTP 200, show a message to the user indicating some problem in the
storage.
Currently, we open an empty document if storage doesn't return a
document which surely is not correct.
Mention the storage server address when asking user to contact the
server administrator to be more friendly.
Change-Id: I15f0489f36db8689b43d42f6b691fdd21815e4fa
Turns out this introduces two calls to the CheckFileInfo which is not really
what we should be doing; instead, let's do a kind of cannonicalization in the
WOPI host directly.
This reverts commit ec2fd0844f.
Change-Id: I311bf8a45b706ed9a4d8cd00db0a990ac6d461b4
The docKey creation moved to Storage where we first
invoke WOPI (if/when it's a WOPI-hosted doc and WOPI enabled)
and see if the user has access to the document at all.
If they do, we expect the server to give us a
unique ID to use for identifying the host regardless
of hostname aliases.
If a unique ID is not returned (i.e. empty or missing)
we use the hostname and port in its place as fallback.
This will break hostname aliases, but it will still work.
Change-Id: I407b0087395f9df6ad9cc6e037570487999be4a4
Reviewed-on: https://gerrit.libreoffice.org/37697
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Document broker needs to know when the save request is sent and when the
save finished. It uses these parameters to avoid shutting down document,
in the document broker main polling loop, if save is already going on.
But direct .uno:Save commands issued from
loleaflet precludes document broker to keep track of it - in this case a
.uno:Save command issued from loleaflet followed by closing the
session will prevent saving the document to storage, if document is huge
enough and LO core takes a bit of time to save it. A save wrapper
command, 'save', ensures that document broker is aware of all such save
requests (_saveRequestTime member variable) and doesn't close the
document until we completely save it (to storage and other cleanups).
Change-Id: I5ec73d45adff23b2e7543e93dfd0624a5e5af46d
...but in combination with the appropriate session's access_token to always
authenticate against the same instance of the WOPI host.
Change-Id: Ic94dfa8fcb226a2d134272b22edc1f8f76c24e34
... to avoid different WOPI hosts using same loolwsd instance sharing a
file because the path of the file (file id etc.) is same in both of
WOPI hosts.
Though, having hosts + port in doc key engenders the possibility of
opening the same file as two if the WOPI host is accessed using
different aliases but that is much less of a concern than security bug
mentioned in previous paragraph.
Change-Id: Iaa2f880edce952925a85e56ed8eba99514e19751
Reviewed-on: https://gerrit.libreoffice.org/37400
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Jan Holesovsky <kendy@collabora.com>
When a document is idle (no activity from
any views) for this timeout duration, the
document is saved and unloaded to minimize
resource consumption.
Change-Id: If6f09136ae40c7e84180fc8c8adbf6db8396d292
Reviewed-on: https://gerrit.libreoffice.org/37374
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Jail paths are now generate from a PRNG
instead of using the PID of the kit process.
The PRN is converted to base-64 and used
as the directory name where a given
kit is jailed.
Change-Id: I8e4bc35d9ccdfdae0e542ab707c417cd29ad52f3
Reviewed-on: https://gerrit.libreoffice.org/37372
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
We should only remove sessions that had already been
disconnected, which caused the saving.
Change-Id: I22fc54356af50df7a7bd2107fa17f6951d40812f
Reviewed-on: https://gerrit.libreoffice.org/36762
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Not to confuse with the DocumentBroker isLoaded,
this is view-specific. Except for the first view,
which is identical to the document being loaded,
subsequent view loadings are independent from,
though follow, document loading.
Change-Id: I0f21016fa84bf507ca4bc0e7a6c7d0d76b033c60
Reviewed-on: https://gerrit.libreoffice.org/36761
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
When a session is disconnecting and we use it to save
(because the other sessions are not viable for saving),
then we need to correctly detect if by the time
saving is done there are no other sessions left.
Otherwise, we end up thinking there are other sessions
when the others had been disconnected during saving.
Change-Id: I55687376f5237a495ae163b53f51ee1d2414d770
Reviewed-on: https://gerrit.libreoffice.org/36711
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
If a session is not loaded, it might never
do so. We should skip them when deciding
whether to save using a disconnecting
session or rely on another.
This is to avoid failing to save when
the remaining sessions never really load
the document at all, and are therefore useless.
Change-Id: I4e7127e12960a31e9f66e835bb63fb2347390711
Reviewed-on: https://gerrit.libreoffice.org/36710
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
The server correctly saves all documents
and waits to upload them before exiting.
Change-Id: I04dc9ce588bc0fa39a9deb298d0a5efa61a03f1a
Reviewed-on: https://gerrit.libreoffice.org/36654
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
When calling the cleanup from DocumentBroker::destroyIfLastEditor(), it might
be (theoretically) too early; so let's do it safer - as the last thing when
shutting down the DocumentBroker::pollThread().
Change-Id: Idc5e34b36d524c8f87a3a0fc99c87244e9784abf
There is a race between creating and adding
a DocBroker into the DocBrokers container
and cleanupDocBrokers is invoked (on timer)
before it had a chance to start its poll
thread. This is exceedingly rare, but it
has happened.
We check that_stop==false flag when deciding
isAlive such that cleanDocBrokers will
not remove it before its thread had
a chance to run (which would happen
after adding it and creating the
ClientSession).
Also, no point in checking isAlive from
the polling thread itself (of course it is
alive).
Change-Id: If54fe2b5fce0697ee0e2f38f1662c71105e29347
Reviewed-on: https://gerrit.libreoffice.org/36500
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
These conditions must be checked together. Otherwise we might
set _stop prematurely.
Change-Id: I3de0d2b3833959593315669ad245f94c1243f7f7
Reviewed-on: https://gerrit.libreoffice.org/36242
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Plenty of time to do that next time around the cleanup.
We should still, really be doing the majority of the timeout work
inside the DocumentBroker poll itself.
When shutting down accept_poll from
main, we can't remove sockets or cleanup.
That work needs to be done fro within accept_poll's
thread. This is different from when DocBroker's
poll needs to cleanup its own sockets before
it exists.
So we split the stop and removeSockets so they
can each be called in the proper way.
For accept_poll and others that joinThread
we queue a callback to cleanup before stopping.
Change-Id: If780d6a97ac0fc6da6897f895d5b4dda443f9e73
Reviewed-on: https://gerrit.libreoffice.org/36186
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
By stopping the poll we fail to notify the clients
of the shutdown. Let the DocBroker poll thread
take care of the poll stopping when it's ready.
Change-Id: I2cb4c76da2722ce41a60fc1983b10dc8b18b4cab
Reviewed-on: https://gerrit.libreoffice.org/36184
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
And assume correct thread if poll thread is
not running (i.e. no race).
Change-Id: I17958e682aba434ebb47fe0de199b9f530b54dee
Reviewed-on: https://gerrit.libreoffice.org/36183
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This simplifies things, and keeps process management in one thread.
Also - wakeup the DocumentBroker when we want to stop it.
Change-Id: I597ba4b34719fc072a4b4ad3697442b5eebe5784
Reviewed-on: https://gerrit.libreoffice.org/36182
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
assert()'s are no-op in the release builds, but we still want to see threading
problems in the log at least.
Change-Id: Idb02bb018e8f2d628a57ab570249613ad00bcff2
This was a workaround to Poco's limitation
of requiring socket receiveFrame be given
preallocated buffer, which couldn't be
exceeded by a larger payload. This meant
the receiver had to know the maximum
payload in advance.
Since only the Kit uses Poco sockets,
and the Kit never receives large payloads,
this preamble is now obsolete.
100% (94/94) of old-style tests PASS.
Change-Id: I76776f89497409e5755e335a3e25553e91cf0876
Reviewed-on: https://gerrit.libreoffice.org/36037
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This matches the document between WSD and kit,
making logs much easier to read.
Change-Id: If55a9eb84b4a22d2dc4dd53f5f6ab322ebc3646e
Reviewed-on: https://gerrit.libreoffice.org/36028
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
==20033== Invalid read of size 4
==20033== at 0x466504: ChildProcess::close(bool) (DocumentBroker.hpp:111)
==20033== by 0x44EA28: DocumentBroker::terminateChild(std::string const&, bool) (DocumentBroker.cpp:1313)
==20033== by 0x45F70E: DocumentBroker::pollThread() (DocumentBroker.cpp:264)
==20033== by 0x504B2F: SocketPoll::pollingThreadEntry() (Socket.hpp:486)
==20033== by 0x7310E6F: execute_native_thread_routine (thread.cc:84)
==20033== by 0x7AF60A3: start_thread (pthread_create.c:309)
==20033== by 0x7DF002C: clone (clone.S:111)
==20033== Address 0x0 is not stack'd, malloc'd or (recently) free'd
Remove locks and replace with isCorrectThread
assertions instead.
Crash recovery still needs some work, but
otherwise tests are clean (91/94 pass).
Change-Id: I9ac3e21854447d19a8e6106487dfd8be00fcf5ef
Since this doesn't necessary mean the document
was loaded completely (as the similarly named
flag in DocumentBroker signifies) rather that
the session was added to DocumentBroker.
Change-Id: Ibfc702bbd111ade2715dcb28ac3aa4e9e8e025dd
It is accessed from the PrisonerPoll
when cleaning up.
Change-Id: Ieb57cdd63cc08632bcdaa4fc5ccd4a1a53c06fe7
Reviewed-on: https://gerrit.libreoffice.org/35788
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Now the poll thread can exit immediately when loading
fails, since we don't think there is an active
saving on going.
Change-Id: I602b2b506f92309d34ec697553bac05523d70e20
Reviewed-on: https://gerrit.libreoffice.org/35714
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Sessions are now added to the DocBroker
_sessions map and loaded from the poll
thread first before processing their
messages.
Since messages are not read from the
sockets outside of the poll thread,
there is no reason to queue them
at all. The only exception is when
messages are passed directly
to ClientSession during convert-to
requests. That will be handled
separately (for now convert-to test
fails).
Change-Id: I798166b9e45b5707a33d31137e01a32ce63630b1
Reviewed-on: https://gerrit.libreoffice.org/35705
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Since these numbers are against the thread start time,
they aren't useful for loading subsequent views.
Change-Id: Ib5cf580282841e5b2dbb71c7db4a1868f7eb29e1
Reviewed-on: https://gerrit.libreoffice.org/35704
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Detection of modified documents used the
directory path rather than the document
path, which obviously wasn't correct.
Change-Id: I4a054a9ce2b64d70cd7a0a1c488dcc38ef46a581
Reviewed-on: https://gerrit.libreoffice.org/35526
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
The rendering options must be last in the load
command. This is because it could contain
characters that interfere with the parsing.
Change-Id: I14930d580d59014532d07410251acbd1316b4f61
Normally the last session either stopped and terminated
the DocBroker upon saving the doc, or immediately upon
removal when nothing to save.
However, saving could fail, or the session could be
disconnected by the client, or saving could timeout, etc.
In all those failure scenarios the DocBroker should
not linger as a zombie (alive but without sessions).
Here we detect that we are left with no sessions
and terminate correctly.
Change-Id: I31862234e321f63e686f54fa69daacc1fa06ae75
Termination should normally be initiated by the
DocumentBroker in question, so sending of termination
message on the sockets come from the correct thread.
When termination happens from elsewhere
(f.e. cleanupDocBrokers) we cannot send socket
messages, and have to resort to rude termination.
Change-Id: I94acb7b314f5dbdc45c57049fc1ac8527ba72fb9
While message emplacement happens in the DocumentBroker poll, we
can be sure that the next iteration of the poll will call
hasQueuedWrites before polling.
This reverts commit 388d7b1dbf.
It is vital to have clean control of thread start. By starting
a thread during init. of a member (or base-clase) we loose lots of
control, some examples:
Any members initialized after a member that auto-starts a
thread, is effectively un-defined, and cannot be accessed
reliably.
This is particularly problematic for sub-classes.
I've seen threads started before the base-class has
finished constructing in the original creating thread -
such that the vtable was not yet updated to the sub-class
causing the transient parent vtable used during construction
to be used in-error.
Now that DocumentBroker has SocketPoll thread,
it's isAlive() must be defined by the lifetime of
both the SocketPoll thread and the ChildProcess,
which it previously did.
Change-Id: I093f8774cf4374d01729a383f6c535de4143fec6
Reviewed-on: https://gerrit.libreoffice.org/35122
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Since all TerminatingPoll instances need to fire
a thread, no reason to do it manually and risk
races. Now TerminatingPoll does it in the ctor.
Change-Id: I59850ad48b3789f3a23d01abb05a7f28e5717031
Reviewed-on: https://gerrit.libreoffice.org/35114
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
DocumentBrokerPoll is always owned by a
single DocumentBroker instance, so we
can hold a reference to it. This eliminates
the need to hold a shared_ptr to the owner
which, in turn, eliminates the need for
a create wrapper.
Change-Id: I954c9dddcc3b2cfdd5dfcc8248ab3d47a897f684
Reviewed-on: https://gerrit.libreoffice.org/35113
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
When the last client session is disconnected
docBroker must first issue a save and wait
until the kit processes the save and sends
back notfication. Since said notification
goes to the ChildSession (which is the last)
and said ChildSession is the one that signals
to docBroker to persist the doc to the Storage,
we need to keep all components alive and kicking
during this final saving.
As such, when the last session is to be removed
from docBroker, we instead issue an autosave and
continue everything as normal. When the save
notification even arrives and ChildSession signals
docBroker to persist the doc, we check if we were
destroying and in that even remove that last session
and stop the polling thread.
The docBroker instance itself will get cleaned up
in due time.
Change-Id: Ie84e784284e1ec12b0b201d6bf75170b31f66147
So startDestroy must not assume the session
was already added at that point, since addSession
is done when we poll for reading, but not when
the socket has already disconnected.
Change-Id: I7bb7222604269c1cc9f2f4b4dad3ea1054b3e0c9
Without this, it was impossible to connect to an existing session, because we
were trying to send messages to sessions that were not connected yet.
Change-Id: If9260a1f0ac8f5387f492541548724b0065df9d9
Don't block when creating new sessions, instead queue the requests, and handle
them in the DocumentBroker.
Change-Id: I200bbfc740f004c37178fa316d1eb91afdde5d4a
Also dung out a chunk of older code.
FIXME: websocket / ClientSession needs to associate itself with the
DocumentBroker poll loop in place of the original loop.
...and not just when loading a new document.
Change-Id: If5b19e500c59a8d1fcf96666ef244833c05e2b99
Reviewed-on: https://gerrit.libreoffice.org/34118
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
By default snapshots are disabled, since trace recording
is enabled, to avoid unexpectedly flooding the disk.
Change-Id: I6c8728e14801f0a72accde1378455ec0e6046e3e
We no longer tell the clinet "This is embarrassing..."
when we disconnect and unload an idle document. Instead,
the client UI remains greyed out so the user can resume
as if it was inactive (and reload the document in this case).
Also, we now always send the "close: " message prior
to shutting down a client websocket. This is more
reasonable and consistent when we intentionally disconnect,
so clients can rely on it to signal intent and give reason.
Otherwise, a disconnection without this application-level
message should be unexpected and is therefore reasonable
to show the "This is embarrassing..." message.
Change-Id: Ic7439bcc9267be155586ccd5d122e9fe60225516
A race condition between the client socket thread
and the idle-document cleanup caused segfault
on the websocket.
Now the ChildProcess object doesn't reset
the websocket on closing, rather on destruction.
Change-Id: I10d0dfb1ba677a65479df85b7a53de8c5f1b44c3
Each Kit process now reports its own PSS,
which is much more accurate as they share
a significant ratio of their pages with
one another.
Admin tracks the PSS values of the Kits
and reports to the console.
Change-Id: Ifa66d17749c224f0dc211db80c44f7c913f2d6c4
Reviewed-on: https://gerrit.libreoffice.org/33864
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
No need to expose PrisonerSession via ClientSession
when the marshalling of messages can be done by
ClientSession directly.
PrisonerSession can now be removed altogether.
Change-Id: I131e41f5d9ae50be7244fb92a6f391a757502111
Reviewed-on: https://gerrit.libreoffice.org/33431
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
When a client disconnects, we autosave only when there
are no other sessions. The idea being that it'd be
wasteful to save on every client disconnect, so long
that we have a later chance of saving.
This doesn't hold, however, when the only other
remaining session is one that had just connected
and hasn't loaded a view yet. WSD wouldn't
know about this, but when unloading the
disconnecting session, Kit will exit the process
as the last view is gone, losing all unsaved changes.
This patch tracks the sessions in WSD that have
loaded a view and takes that into account when
deciding to autosave on disconnect or not, thereby
fixing this corner case that may result in data loss.
Change-Id: I699721f2d710128ed4db65943b9357dff54380f5
Reviewed-on: https://gerrit.libreoffice.org/33126
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Previously tilecombine had its own version, which is
nonesensical, since it's not really a tile.
Now it passes the version to the tiles when
parsing and serializes version per-tile.
Change-Id: I5db8d94880431e3d2a40b6787c6fe51a05771305
Reviewed-on: https://gerrit.libreoffice.org/32633
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
When saving we need to differentiate between no-op
and failure. The lastSaveTime must always be updated
when saving didn't fail (i.e. no modification or saved).
Change-Id: I0e2455afac22c82f0b623f9441fbc0bca8a7cb83
Reviewed-on: https://gerrit.libreoffice.org/32629
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
While time_t is much simpler, it's too
opaque. The new chrono library is type-safe
and does conversion correctly, as well as
guarantees monotonity and other desirable
properties.
Change-Id: Id41c44c397a31d73e894e8f1715ff18f2b67df53
Reviewed-on: https://gerrit.libreoffice.org/32627
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
And check for save-failed first before finding
the session, as otherwise the session is useless.
Change-Id: I3eb45e54872547eb36765b2c8409e1aa51aee589
Reviewed-on: https://gerrit.libreoffice.org/32611
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
