To avoid writing (saving) while reading (uploading) or
any other undesirable racse on the document file,
we atomically rename the file after saving on disk
to pass ownership.
After saving in Core, we rename the document in the
jail to .upload (by appending it). DocumentBroker
looks for the file with that extension and atomically
renames it to .uploading (by appening the 'ing' suffix).
This way, the Kit only renames from the original to
.upload and DocBroker renames only .upload to .uploading.
This guarantees that we never rename the same file
concurrently.
Uploading decision is strictly based on the modified
timestamp of the .uploading file, compared to the
timestamp of the last file we uploaded successfully.
Change-Id: I03520cd8c87605f6dad417e7a978204f76fc0c38
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Now we can handle the response of the storage
after the async upload is complete (or timed out).
Change-Id: I29d450646bddb07f02bb17d257e7e0fa372ce357
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
Modelled on how it is done in core. ProfileZone is derived from
NamedEvent which is derived from TraceEvent. Here we don't keep any
separate ProfileZone.hpp, though.
This was needed to introduce generation of "instant" events here, too.
Signed-off-by: Tor Lillqvist <tml@collabora.com>
Change-Id: I6583134e96001641c50339deb4197fca6ab7d5d5
This was already worked on iOS since commit
75a3ab02ca (Add an iOS setting for the
user's name or nickname, 2021-04-07), start using the same API on
Android as well.
Signed-off-by: Miklos Vajna <vmiklos@collabora.com>
Change-Id: I37825d143b0ed92bd84f9a1512313e51ff5e761c
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>
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>
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>
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>
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>
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>
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>
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>
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>
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: 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>
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>
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
The default Poco connection timeout is 60 seconds,
which is probably excessive. The current configurable
default is a more reasonable 30 seconds.
Currently we set this timeout on Storage connections
going out (i.e. WOPI connections).
Change-Id: Ie80a9141ca9bf721addc74baf94e62e0ad72fdd2
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/98913
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Ashod Nakashian <ash@collabora.com>
Integrator currently gets no message when loading the document
from WOPI host fails.
Similiar to Action_Save_Resp, introduce Action_Load_Resp with
the result of the load action.
Change-Id: I3b0f9ee691a1c5d58e9f833d511435a0b25a465f
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/98299
Tested-by: Jenkins
Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt@cib.de>
665b1629de was not correct, as it reported back
the save result of the internal save (which usually succeeds).
Instead we want to know the save result of the remote storage (WOPI/Webdav).
So report that back instead.
Change-Id: Iaaa42b8c817a19c2c77935a6f81c1951fdf2216c
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/97637
Tested-by: Jenkins
Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt@cib.de>
This is to defend the sneaking of extra http-headers
in the access_header URI param that was recently fixed.
Change-Id: Ic28cf58854847ac278bed8043f398b107f7992b3
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/96862
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Without the content-length, LOCK requests result in 411 response with
the message: "The request must be chunked or have a content length".
Ref: https://forums.iis.net/t/1119456.aspx
Change-Id: Ieceb2bcf478c5f6baf97ee6b89d37622da168df5
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/97524
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
This hoists the common parts of the HTTPRequest
for all WOPI requests to avoid errors when changing them.
Change-Id: Ia02ef657a43b7a7d2fc13be3da012836fa0d7650
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/96372
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Tested-by: Jenkins
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
