This enables writing multiple WS frames
at a time, up to the capacity (rounded
to the next full-frame's size).
Change-Id: I30d25f9ba1e405156d1e44886b2f3ac8017f0f3c
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
When writing to the socket, it's always more efficient
to fill the buffer up to the hardware limit for each
write. This is doubly important for efficiency with
SSL, due to the overhead of encrypting multiple
small buffers instead of one large one.
Currently we don't write more than one message
at a time, primarily due to limitations in
the Poco sockets in the unit-tests, which
have a hard time consuming multiple WS frames
with a single poll (subsequent calls to poll
doesn't enter signalled state until new data
arrives, possibly because the data is read and
buffered internally, making the whole scheme
of using poll unreliable and meaningless).
Change-Id: Ic2e2cf1babfb5ab4116efd93f392977ba234d92b
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
... without copying the token.
And use it in TileDesc::parse(), which is known to be a hot path.
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
Change-Id: I0dcf2eb26c93254cdc6a1c11f9708daf213a825d
And use it in TileDesc::parse(), which is known to be a hot path.
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
Change-Id: I20375d7a1c31f61662446979e4d6799fd45b49d3
In some rare cases Poco throws "Exists" exception.
It's not clear what causes it, so this should
log more of the original exception, hoping that
it would shed some light.
Change-Id: I51723f35c9e062c56608e8f7b80c9a65d3f0488f
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Even if we use http:// locally via a tunnel the new outgoing socket
code needs SSL initialized to connect securely to remote servers.
Change-Id: If64a3838267182757591a8026097bf08d9ba732f
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Used for comments added to documents by the user.
The setting, like templateListURL, can be set through a mobile device
management system.
There is no trivial way on iOS for an app to programmatically find out
the name of the user, and that is good from a privacy point of view.
Fixes https://github.com/CollaboraOnline/online/issues/1843
Change-Id: Ie68fcbacf886ec8f1c74021a71879b38d4180c15
Signed-off-by: Tor Lillqvist <tml@collabora.com>
Per the RFC, the server must set the 'Connection'
header to 'close' to signify that the current
request is the last.
With this patch, we now properly notify all the
clients when we do not intend on resuming communication.
Although, technically, there is no reason why we
shouldn't allow the client to reuse the connection,
we may consider that in the future.
See https://tools.ietf.org/html/rfc7230#section-6.3
Change-Id: Ic6250c0afe8124314a9122452040f6a75728a525
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Since the return value of syncRequest and syncDownload
was derived from Response anyway, there is no reason
to have multiple values for callers to look at.
This simplifies the API.
Change-Id: I0f136e515dd0ef6eda84f6a7cd662b260809d2f1
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Rendering PDF documents to images can consume
large amount of memory. Here we limit the
maximum allowed resolution to 384. This is
about 400% zoom resolution, given that the
100% resolution is 96 dpi.
If this proves limiting, it can be increased
to 480 dpi, which is about 500% zoom scale.
Change-Id: I719118d517bbaf8b204411223f1e26a31b97ca99
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The defined envar is used by Core to override its
default resolution (of 96 dpi), when specified.
Change-Id: Ie7164e78f98990ac88b02dbfe2becbae8c134e9c
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
User-Agent is designed for client-side use only,
in http requests. For servers, the Server header
is designed to announce the server name and version.
This tries to normalize the use and documents
the proper intent and usage.
Change-Id: I42d68d65611cab64c45adf03fe74f9466798b093
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This makes things more consistent in naming
and in namespace.
Change-Id: I2149c9e0b96eddf9fe2ad1dbc80c932fa7582f84
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This moves WebServerPoll and PrisonerPoll
from the global namespace into LOOLWSD
while converting them to unique_ptr. This
is to manage their lifetimes better and
avoid having out-of-order destruction,
especially with regards to Poco and other
statics that are out of our control.
The PrisonerPoll class had to be renamed
to PrisonPoll because the following is
otherwise invalid:
PrisonerPoll = Util::make_unique<PrisonerPoll>();
Change-Id: Id6c50ebf12ad0f313fea130ffba364beff54f686
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
LOOLWSDServer needs to shutdown its
accept_poll SocketPoll in its destructor,
which may be called after Poco's Logging
subsystem has been destroyed.
Instead of managing the lifetime of
accept_poll member of LOOLWSDServer,
it is safer to manage the lifetime
of LOOLWSDServer itself, and destroy
it in the cleanup stage. This makes
sure that its other members, or indeed
LOOLWSDServer itself, can't have any
late-destoryed objects that can cause
trouble.
The stacktrace for this crash:
terminate called after throwing an instance of 'std::bad_alloc'
what(): std::bad_alloc
Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
51 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007ffff613f801 in __GI_abort () at abort.c:79
#2 0x00007ffff6d51957 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3 0x00007ffff6d57ae6 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4 0x00007ffff6d56b49 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5 0x00007ffff6d574b8 in __gxx_personality_v0 () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6 0x00007ffff671f573 in ?? () from /lib/x86_64-linux-gnu/libgcc_s.so.1
#7 0x00007ffff671fad1 in _Unwind_RaiseException () from /lib/x86_64-linux-gnu/libgcc_s.so.1
#8 0x00007ffff6d57d47 in __cxa_throw () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#9 0x00007ffff6d582dc in operator new(unsigned long) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#10 0x00005555556b6127 in void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*, std::forward_iterator_tag) [clone .isra.41] ()
#11 0x0000555555986b94 in Poco::Message::Message(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, Poco::Message::Priority) ()
#12 0x000055555585ea49 in SocketPoll::~SocketPoll (this=0x555555d13490 <srv+16>, __in_chrg=<optimized out>) at net/Socket.cpp:145
#13 0x000055555575d794 in TerminatingPoll::~TerminatingPoll (this=0x555555d13490 <srv+16>, __in_chrg=<optimized out>) at ./net/Socket.hpp:832
#14 LOOLWSDServer::AcceptPoll::~AcceptPoll (this=0x555555d13490 <srv+16>, __in_chrg=<optimized out>) at wsd/LOOLWSD.cpp:3766
#15 LOOLWSDServer::~LOOLWSDServer (this=0x555555d13480 <srv>, __in_chrg=<optimized out>) at wsd/LOOLWSD.cpp:3640
#16 0x00007ffff6142041 in __run_exit_handlers (status=0, listp=0x7ffff64ea718 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true, run_dtors=run_dtors@entry=true) at exit.c:108
#17 0x00007ffff614213a in __GI_exit (status=<optimized out>) at exit.c:139
#18 0x0000555555753658 in LOOLWSD::innerInitialize (this=<optimized out>, self=...) at wsd/LOOLWSD.cpp:1213
#19 0x0000555555789527 in LOOLWSD::initialize (this=<optimized out>, self=...) at wsd/LOOLWSD.hpp:439
#20 0x00005555558e1964 in Poco::Util::Application::run() ()
#21 0x00005555556b6d74 in main (argc=3, argv=0x7fffffffe4f8) at wsd/LOOLWSD.cpp:4286
Change-Id: I28ea6215ce49c752cbb90bc33269ab3b662accf1
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The lifetime management of static objects
is extremely unpredictable and depends on
many variables outside of our control or
even reliable reproducibility.
Complex static objects that own threads
and other objects are doubly problematic
because of their dependency and/or
interaction with other objects.
Here we replace the static DelayPoll
instance with one we control its lifetime
in the LOOLWSD main body, such that it
is destroyed properly.
Specifically, DelayPoll's dtor was
accessing Poco's Logging subsystem out of
order. That is, after Poco had been
destroyed.
Another advantage to this approach is that
we don't even create the DelayPoll at all
if we don't need it. The onus now is on
the user of DelayPoll to make sure they
create a Delay object with a long-enough
lifetime to encompase it use.
For completeness, here is the stacktrace:
terminate called after throwing an instance of 'std::bad_alloc'
what(): std::bad_alloc
Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
51 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007ffff613f801 in __GI_abort () at abort.c:79
#2 0x00007ffff6d51957 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3 0x00007ffff6d57ae6 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4 0x00007ffff6d56b49 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5 0x00007ffff6d574b8 in __gxx_personality_v0 () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6 0x00007ffff671f573 in ?? () from /lib/x86_64-linux-gnu/libgcc_s.so.1
#7 0x00007ffff671fad1 in _Unwind_RaiseException () from /lib/x86_64-linux-gnu/libgcc_s.so.1
#8 0x00007ffff6d57d47 in __cxa_throw () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#9 0x00007ffff6d582dc in operator new(unsigned long) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#10 0x00005555556b5927 in void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*, std::forward_iterator_tag) [clone .isra.41] ()
#11 0x0000555555982a14 in Poco::Message::Message(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, Poco::Message::Priority) ()
#12 0x000055555585a909 in SocketPoll::~SocketPoll (this=0x555555d10f60 <DelayPoll>, __in_chrg=<optimized out>) at net/Socket.cpp:145
#13 0x00007ffff6142041 in __run_exit_handlers (status=0, listp=0x7ffff64ea718 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true, run_dtors=run_dtors@entry=true) at exit.c:108
#14 0x00007ffff614213a in __GI_exit (status=<optimized out>) at exit.c:139
#15 0x0000555555752d78 in LOOLWSD::innerInitialize (this=<optimized out>, self=...) at wsd/LOOLWSD.cpp:1213
#16 0x0000555555788b07 in LOOLWSD::initialize (this=<optimized out>, self=...) at wsd/LOOLWSD.hpp:432
#17 0x00005555558dd7e4 in Poco::Util::Application::run() ()
#18 0x00005555556b6574 in main (argc=2, argv=0x7fffffffe528) at wsd/LOOLWSD.cpp:4276
Change-Id: Ifda55fe869fa6734b9c2490da4497d2551ac21c1
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Makes it accessible to more code.
Change-Id: I73dd1895defe47ae40873b6155203768055206ec
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
"Specifies whether the macro execution is enabled in
general. This will disable Basic, Beanshell, Javascript
and Python scripts. If it is set to false, the
macro_security_level is ignored. If it is set to true,
the mentioned entry specified the level of macro security".
Change-Id: I4bc5b690268a93994d17e2b02b7b60b6398646b7
Signed-off-by: Henry Castro <hcastro@collabora.com>
The clipboard content is handled by the DocBroker,
if it it's available, otherwise, we fallback to the
cached clipboard entries.
When DocBroker has already terminated polling and
is no longer valid, we may end up using it for
the clipboard. In this case, we don't send anything
back, since there is no SocketPoll thread to
process the request or the socket data plumbing.
We avoid this by simply using the fallback (cache)
when we don't have a valid DocBroker.
Change-Id: Idf7f27b453605d173dd8e40d4a5d101f50355468
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Especially error reporting is now better.
Change-Id: I032f6e8403660c2ac24be2d80b53b63831ec1066
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
When loading hasn't completed yet, DocBroker can't
simply send a message to Kit that the session is
disconnected, because there would be no UI or
handlers for events if that were the only session.
Of course letting the document load, only to close
it, is also less than helpful to resource consumption.
Here, we recognize this case and simply terminate the
loading by killing the Kit process altogether to
avoid any potential hangs or resource wastage.
Change-Id: Ia72de715cc6238831c244444bb47b417b9b1e1a4
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Add a new state before load the document,
when Macro Security dialog popup, and avoid
to send the event load timeout.
Change-Id: I5973c5205e90e5447e5478cbab895709a68606f6
Signed-off-by: Henry Castro <hcastro@collabora.com>
This converts Poco to http::Request in sync mode,
thereby not changing functionality. In a follow-up,
this will be converted to async.
Change-Id: Ifbecd44ff599394799c1131470d77f803ed8cc45
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Normally the client session waits for a message
"status:" when the document has loaded to forward
to client side. However, when the document has macros
embedded, and a Macro Warning Security dialog popup
it will never receive the "status:" message.
So it is added "statusindicatorfinish:" to formally set that
the document has loaded.
Change-Id: Id40b853c002403577d7664c4f8206cb5a01403b6
Signed-off-by: Henry Castro <hcastro@collabora.com>
```
wsd/DocumentBroker.cpp:772:57: error: constexpr variable cannot have non-literal type 'const std::chrono::system_clock::time_point' (aka 'const time_point<std::__1::chrono::system_clock>')
constexpr std::chrono::system_clock::time_point Zero;
^
/usr/include/c++/v1/chrono:1355:28: note: 'time_point<std::__1::chrono::system_clock, std::__1::chrono::duration<long long, std::__1::ratio<1, 1000000>>>' is not literal because it is not an aggregate and has no constexpr constructors other than copy or move con
class _LIBCPP_TEMPLATE_VIS time_point
^
```
Signed-off-by: Gleb Popov <6yearold@gmail.com>
Change-Id: I88c5466b8ad104178cf7fa4101f5f5265ccf5e86
Consistent with names elsewhere and less confusing.
Change-Id: Ia72cdef0f6d6cd9589ce54028472e10008cf882c
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The download operation itself is done
after reading the response from the Poco
object. Therefore, the time duration that
was captured for the GetFile operation
was inaccurate. Luckily, we only need
to time the download API of the Storage
object, which actually is simpler, and
more accurate.
Change-Id: I05c94a29fa59d5efae215f2daea17672abc6efc7
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
For some reason, the duration of the CheckFileInfo
request was not set/updated, and therefore it was
always 0. This fixes this oversight and also
moves the duration variable locally, since
the request is synchronous anyway and there is
hardly any reason to store it elsewhere.
Change-Id: I613b24311bc5c8ee9ab98d675057f354990226e8
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
We use download/upload in the storage
and load/save elsewhere, to avoid confusion.
This renames 'save' to 'upload' in the storage
for consistency.
Change-Id: I9ac991c2b52e2586b97c58db02110ff04bfd17d3
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Templates were downloaded by Core
upon loading. This works fine, as
long as there is no special network
setup in loolwsd. However, when
loolwsd has a complex network setup,
such as when using reverse proxies,
Core wouldn't know about the details
and would likely fail to download
the template.
Luckily, there is no reason to rely
on Core for downloading templates.
Instead, we download it in loolwsd,
just like any other document, and
load it in Core as normal. The
remaining post-load saving of
templates remain unchanged.
Change-Id: Ib22ada4ae469863d5e5c8baeee27f667f7cd40ff
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Instead of inheritance we use composition
to preserve the uniqueness of the members
of SaveManager and StorageManager.
Change-Id: Ifad82787e54089d49226085e009ade67bfc7938a
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This will be used to detect whether we need
to upload the document or not.
Change-Id: I2c5f6d058b1a8e0a6ab20c9561b6701413fb5878
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This is to encapsulate and track the
common logic between SaveManager and
StorageManager.
Change-Id: I0c5a59edb8a26b258ba66d65983e2f76198ecbc9
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This is to encapsulate and track the logic
for uploading documents from the Storage.
Change-Id: I5b972151fe9548526755d18063d37cc95949e68f
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
We should only update the save timestamps when we
really save a new version of the document.
Requesting renaming or moving ("save as") in storage
should not be confused with actually saving in Core.
Here we take the first step in this separation,
and we only update the last save response time
when we truely get a response from Core, and
not when we execute a document path/name in storage.
Change-Id: I931c62b306dd3b4906a9e910f844a35fb3f4b6f0
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The SaveManager is responsible for the file on disk.
Change-Id: I92f7843375fdc875bc7d1af3fba387f67a4f0ca5
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The SaveManager is responsible for the document's
save state. It also encapsulates the members
necessary to track the save state and related
data.
Change-Id: I3ed0f1d93f25988b1ad8b1a121a2080288866a53
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
And a couple of const cases.
The removed const is to allow move on return.
Change-Id: I7a81b531e75c39379871f5ffeb82d49ba1110ab1
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
We already access _storage before the assertion,
so it must move before the first use.
Also, we have no use for _tileCache when uploading,
so no point in asserting its validity here.
Change-Id: I4196d0719e6750cfd2dba8d5f449a1b76286bde9
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This details the design of the DocumentBroker
states and state-machines as they are to be
eventually implemented. Not everything described
is current, but the goal is to document what
the design ought to be, not what it is.
Change-Id: I938177812777af058b46c41a396407d0a083cbc4
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Only the last argument (force) was implicit, and only
in one call (of total two calls). Explicit is better.
Change-Id: Ic26f4dd265f48156d1730f1b95bb70145ca47873
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The loaded flag is used to track whether or not
the document was ever loaded. It does *not* reflect
whether or not the document is currently loaded,
rather it is used to save the document, if necessary,
before unloaded.
This is why we need to track it as a separate boolean,
rather than implicitly tracking this state via the
DocumentState enum, which would not be able to
differentiate between a failed-to-load state and
one of a normal end-of-life, after loading.
Change-Id: I1fc3fb09c31cadf7ebb6b0123e462e78dd0af356
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The markToDestroy flag tracks whether or not the
document is in the process of being unloaded.
Change-Id: I8f2339f75ed6d7c5d318eb1d467d6a9cbc1d61c2
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
DocumentState is responsible for tracking
the state of the document, properly documenting
the different stages of the document lifetime
and encapsulating all the relevant members.
DocumentState is still unused in this commit.
Change-Id: Ic2c8de3a9f2d42c5550c5f4ad5f889040f697890
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This prepares for moving this flag into
the upcoming state machine.
Change-Id: I0fa0e26f72a8f4da6813c3be6df5b16a94773591
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
To differentiate between the different stages
of document lifecycle, fetching documents
from the storage is now called 'download',
as opposed to the previous 'load', which is
reserved to loading document in Core.
This is a cosmetic renaming to avoid
confusing usage and intent going forward.
Change-Id: Ib9451e6f73bab19b877a3e6c8fb5b17ba82a06ab
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This code is executed regularly, at every poll.
The config getter internally throws exceptions,
catches and handles, before returning. While
this isn't performance sensitive, it is noisy
to have such poor-behaving code executed
frequently and regularly.
But the real motivation is the pain this exception
causes when debugging an exception. GDB ends up
catching this rogue exception, which is hardly
useful or helpful.
At any rate, we don't need to get a config entry
more than once, as they can't change during the
lifetime of the process.
Change-Id: Ic69fb94674b74b298987131d44d9baa27e9fa4aa
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Add support for configuring the SSL certificates
when running the test binary as well as setting
up the SSL context.
Also adds the SSL socket headers with proper
compile-time guards for when SSL is disabled.
Change-Id: I99992639a66a64871f8ff8a2b2105279ead63ca1
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Technically, the poll interval should be
irrelevant because the tests should wakeupWorld
when they need to break the poll. However, it
turns out that many tests implicitly rely on
the poll interval to be large, and if for
any reason the poll wakes up, the tests either
advance their phase too soon, or assert, causing
random failures and reliability issues.
As the tests should now be more robust (at
least the ones that were non-deterministic
in this way), we can now lower the interval
to catch other cases (either extant or in the
future).
Change-Id: Id0d472e70875db8669bb21c6582b0d5052b19bfb
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
And introduce passTest and failTest to log a reason
and be more informative when reading logs.
Change-Id: I5090793b802a29135de8ea3783a457e189cc7df3
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
While UserFriendlyName is an optional field
in the WOPI protocol, Core needs it for
the Author of the document. When it's blank
the Author is not set and the document fails
to load.
By default we are at least able to load the
document with a sensible placeholder for the
Author. Meanwhile, we log a warning to let
the integrators know of the issue.
Documentation updated.
Change-Id: I4dd2c9d164b4d889f85701a4a27ee8d395bff220
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The development environment is necessary analyse
the logs, so disable setting SAL_LOG instead to
add a parameter to already a long list of options
Change-Id: Id8e4a66e1dcb32c636806e47e1d69270af4c53f5
Signed-off-by: Henry Castro <hcastro@collabora.com>
A minor cleanup of UploadResult to make tidy
it up a little bit and make it less specific.
Single-argument constructors should be explicit
to avoid unexpected conversion and other surprises.
Change-Id: I57599805743dffddac620f501dc6ca79c2217f89
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
In some cases we cannot do a fast bind-mount of the files we want
in our jail since we don't have cap_sys_admin for loolmount inside
eg. docker.
Thus we need to fallback to hard-linking, however various security
systems namespace parts of our tree, such that link() fails with
EXDEV even across the (apparently) same file-system.
As such we need to assemble a copy of what we want to hard-link
close to our jails. However, this needs to be owned by root / the
system to avoid having writable files shared between jails. Hence
we need cap_chown in addition to cap_fowner, to get ownership right
and then hard-link.
Change-Id: Iba0ef46ddbc1c03f3dc7177bc1ec1755624135db
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Most C and Posix API clobber errno. By failing to save
it immediately after invoking an API we risk simply
reporting the result of an arbitrary subsequent API call.
This adds LOG_SYS_ERRNO to take errno explicitly.
This is necessary because sometimes logging is not done
immediately after calling the function for which we
want to report errno. Similarly, log macros that log
errno need to save errno before calling any functions.
This is necessary as the argements might contain calls
that clobber errno.
This also converts some LOG_SYS entries to LOG_ERR
because there can be no relevant errno in that context
(f.e. in a catch clause).
A couple of LOG_ macros have been folded into others,
reducing redundancy.
Finally, both of these log macros append errno to the
log message, so there is little point in ending the
messages with a period.
Change-Id: Iecc656f67115fec78b65cad4e7c17a17623ecf43
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Cleans up some of the conversions and implicity
unit in integral types.
Change-Id: I79f35b92f8f631894e55bdb39851b050870fce96
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Happens when renderfont is called without first loading a document.
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
Change-Id: I1152d1f4b3f610364e22c406cda5494672f20aed
Avoiding some debug code ending up in the product.
Change-Id: If37b2986f134986a33b9dd5b4729b50c56cbc248
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Saving documents to storage also logs and broadcasts
the result to the users. Return values from these
functions are ignored, and anyway not actionable.
Change-Id: Iaf0dab9c6ac8c593e4df292c71fcb30e8b6d7eeb
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Also, makes the logging of units much less error prone.
The overloaded streaming operators are temporary as
they are provided in C++20. The ones here (though
incomplete) are fashioned after the C++20 specs.
Change-Id: Ieb499282ccb6e63fa939ba07bed3e5a4fbef1bd0
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
chrono::system_clock can go back in time.
For time interval measurements, where we don't
care about the local time, a monotonic clock
should be used.
This avoids the server uptime jumping around
with daylight saving (or indeed by regular
synchronization with an atomic clock), among
other cases.
Change-Id: I09f9b24c82d19439348a2e66cad9e9de7d755208
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
While chrono supports double as a datatype, it
is opaque and doesn't lend itself to any obvious
units of time (presumably seconds). Using
chrono::milliseconds is much more readable and
also safe when converting from seconds or any
other units. Ultimately, we typically convert
to milliseconds anyway, mostly for logging.
There is but one exception where we convert
in seconds, and now that case is documented.
Change-Id: Ide98f45f2ad8da8225d41ae870bbc4bc09a2a0b5
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
No advantage in using int when chrono handles
conversion and comparisons transparently for us.
Change-Id: Idc942e7a2557ef979d876f378cf6bb84d3e657cd
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
std::chrono handles unit conversion handsomely
and where there could be logical errors, the
compiler errors out. We only ever need to
use raw integer or double values to interface
C functions and possibly for IO.
Change-Id: I5c2b43c36bd69840f1a4172e9898666c4d68c567
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Now chosen log level is propagated to forkit and kits.
Also, admin console users can filter logs according to their channel names on client side.
Change-Id: Ife15a6148ed87533b81e9d63da252c633e74e559
Signed-off-by: Gökay Şatır <gokaysatir@collabora.com>
A number of call-sites, eg. clipboard, or admin-ws were
writing to sockets assuming they could return all the data
in a single series of writes, without needing to poll. As
such they failed to addSocketToPoll on the new poll - eg.
the docBroker. Unfortunately this meant that on EAGAIN
writes, the socket would be closed and the last parts
of a message lost.
Browsers would give net::ERR_CONTENT_LENGTH_MISMATCH 200 (OK)
The situation is/was intermittent, so painful to debug.
On under-loaded developer machines, socket buffers are larger,
so this was seldom seen.
The re-factor forces a transfer to another SocketPoll via
the disposition, except for a couple of corner cases.
Change-Id: I2f1b2f99f179c4fda84464c9241fe434fa527725
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Using "load" and "save" in the storage was a poor
choice of verbs, in hindsight, because these very
same verbs are also used to describe the loading
and saving of documents in Core.
It is more appropriate to label the storage
operations as download and upload, respectively,
to avoid any confusion. This is especially useful
because when reporting we have for some time now
been reporting the results of each of these
stages separately, there is no longer reason
to label them the same.
We already used "upload" and "download" in
some of the logs, but not all.
Change-Id: I0fac9130032e2c3c6dfb4d671c31130265091f0d
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This is in preparation for asynchronous uploading.
Change-Id: I5c9977107b415efd24cbd99c29599b86cfe32933
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The current session is needed only while loading, as it's not
yet in the sessions container. But while saving, broadcasting
to all sessions includes the current session as well, and
we avoid sending duplicate message to the current session.
We also make the broadcast helper a member of DocumentBroker
which simplifies it.
Change-Id: I3bb37cc808d97ba2b772b88474a8c10f7fdff6b7
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This is in preparation for asynchronous uploading.
Change-Id: Ibd0ff0fa8edfc08ad2755a45227891ed40e09d1c
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
The temporary directories created for convert-to
and insertfile are used only once and should be
cleaned up to avoid clutter.
We also de-poco the temp directory creation as
it doesn't add value and do a bit of cleanup.
Change-Id: Ie1fd5b4749788ff4407f2cc886d405258f65f97a
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
We now download the convert-to files into the
child-root/tmp directory and then move it into
the jail that will convert it. This way ownership
and cleanup become contained within our child-root
and jail subsystems. This reduces the chances of
leaking convert-to files and simplifies the design.
In addition, we avoid an extra file copy and improve
the security of the convert-to API.
Change-Id: I450c24d0d0dc0da447c8072b0701c3b48d07c81b
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
size_t in C and in C++ are not necessarily the same
type. The C++ size_t is in the std namespace. Since
we do include many C headers, and indeed some C++
runtime headers do define size_t for backwards
compatibility, it's easy to mix and match the two
types.
Also, 'using std::size_t;' isn't a great practice,
so removed.
This is not exhaustive, just some low-hanging cases.
Change-Id: I85a36b6fd1acd204274b1869de9bcb94c8b3cf13
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This makes the code self-documenting and avoids accidental
comparison or assignment of Result variables/values.
Change-Id: I84b8e36aa999191c8704938552b73ddc1c3dc3fc
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This replaces Util::getFileTimestamp with
FileUtil::Stat::modifiedTimepoint() and fixes a potential bug:
getFileTimestamp had only 1 second precision (it simply dropped
sub-second data). This could mean that any modifications to a file
within a second could not be detected.
Minor simplifications done where possible and overly long lines
have been reformatted.
This is a non-functional change (except that file modified-time
now supports microsecond precision).
Change-Id: I3606638a86fc3e00c0ad5cb602bdbb2b4651867b
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Confusion arose due to separate creation of session, and watermark
property fetch from CheckFileInfo which happens in DocumentBroker::load
which doesn't do a load. This happens in a subsequent 'load url='
message cf. global.js which can then race vs. the session creation.
This causes mis-ordering of another unhelpfully shared Session,
letting the view canonicalization list to get out of sync between
the two processes.
So instead - tell the view it's canonical id. An example of the
problems of trying to share some unclear subset of the Session
class between kit and wsd perhaps.
Change-Id: I63dc30f9a047e3f889fd339b6aaf392b9fef37b9
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
This reverts commit 4410c51b3879c9291b465dea3a59c2e65e94c361.
Reason: this issue should be fixed in the integration that caused the
problem in the first place.
Change-Id: Id41eb66979e4c08dd3b9fd6499e01b4690526541
This can happen on a 'savetostorage' which is after a failed load.
Change-Id: Iad26bf6415c772c8646a119b0454c202873d6860
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
As the comment describes, this is a temporary workaround.
The issue happens when nocaps is enabled, no doubt due
to the timing difference from the more common (and default)
caps-enabled case.
Change-Id: I742d7b9fd63f0babe8e1d45b968e09026d78debc
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
std::ifstream's tellg() returns -1 on error.
This is handled very poorly and shows up as ULONG_MAX.
Luckily, we have Stat class that does the same
both more safely and more efficiently.
Without opening the file, we now get the necessary
information unambiguously.
Change-Id: I2448bc71e01b0f166a9dd66aa38a88ea97a50cdd
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
And set the LOOL_LOGFILE envar only when
we have a path.
Change-Id: Ide5144c7e5aec5ddb9a5a6c32538e9e0a3a6ed0b
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
When uploading to storage fails, we want to retry on next
save. This works when the document is modified between
the last attempt to upload and the current save.
However, when the document isn't modified (f.e. when unloading
the document) we still need to upload the last version of the
document, so we save the document, but that fails as the
document isn't modified. And so we end up not uploading it.
Actually, it gets worse, because we will keep retrying to
save, because there is nothing else to do when we need to
unload the document (say because it has been idle).
This issue was previously not seen because storage failures
are quite rare. However in certain cases NC with complex
access permission controls does fail fairly frequently,
and this edge-case becomes an issue.
This patch enables forced uploading when the last attempt
had failed, regardless of the state of the current save
result.
Change-Id: I951bf19b62f049547913f490d618be20b6191080
This is useful in situations where you can't get at console.log()
output from the JavaScript, like when you are using the Nextcloud iOS
app installed from the App Store and want to debug the loleaflet
offered by a Collabora Online server you are using. You can add code
like
this._map._socket.sendMessage('DEBUG Foo bar=' + bar);
temporarily to the JavaScript and will see those then in the terminal
where you run 'make run.'
Change-Id: I04c78d86faa283e135dbcb86dac0cabd7b6a0724
If IP address was set as allowed in storage.wopi, but its host
name was given in WOPI URI, the host was not resolved, and was
not accepted.
Change-Id: I20655cee8b435c9645d5cfdd102bdae9033fc1ab
See core.git commit abb6c01519a0318d7165dc9dc5b7d185353f93d6 (replace
usage of whitelist with allowlist, 2020-07-06).
Change-Id: I04646c63df4de14b9aa7dca02a4d5cec10e5eb67
