The switch away from LOOLWebSocket and the use of a websocket
for talking to forkit removes the need for the pipe code.
Change-Id: Ifb0c6c88681289e7a1709d9bc3281532935c7be4
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92033
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
1) Don't actually kill anything with the kill command, otherwise kill(0,
SIGKILL) will kill the fuzzer itself.
2) Don't require a valid signature when authenticating with JWT, since
the private key is generated on each process startup.
3) Log when the JWT would be invalid due to an expired timestamp.
Change-Id: I0da285617e27910329c0e7ed80a6d02e86344ccf
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/91737
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
So that it is easy to find it in the log.
Change-Id: I1b367db9e97d7215c46e824998e99048531acb07
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/91413
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
LOOLWebSocket is used only for tests and a few tools anyway.
Change-Id: I18e04efc2fb3a4cc075f7cefa4ebd0d10fbdb3df
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/90974
Tested-by: Michael Meeks <michael.meeks@collabora.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
This sometimes causes unit-integration to fail and now is consistent
with other member functions of Session that handle missing protocol.
Change-Id: I43c7fcae964cfcb5911ff57d63bd4cb569e6b97c
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/90732
Tested-by: Michael Meeks <michael.meeks@collabora.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
It took both an std::string and a length. Take a char* and a length
instead.
Change-Id: Id37dfa67fe1baae09b69819680848a0a8a1d80ed
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/90552
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Essentially we want to be able to separate low-level socket code
for eg. TCP vs. UDS, from Protocol handling: eg. WebSocketHandler
and client sessions themselves which handle and send messages
which now implement the simple MessageHandlerInterface.
Some helpful renaming too:
s/SocketHandlerInterface/ProtocolHandlerInterface/
Change-Id: I58092b5e0b5792fda47498fb2c875851eada461d
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/90138
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Allows comparing tokens with C strings without a heap allocation. Do the
same when comparing two tokens from two different StringVectors.
And use it at all places where operator ==() has an argument, which is a
StringVector::operator []() result.
Change-Id: Id36eff96767ab99b235ecbd12fb14446a3efa869
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/90201
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
WebSocketHandler handles this complexity for us now, and for the
forseeable future. Simplify to prepare for larger re-factor.
Change-Id: I73b919885adc358cb6502a13492cdac85c34459c
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/90059
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Debian 8 has gcc-4.9, which emits -Werror in case a parameter and a
member function has the same name. Given that we also use -Werror
unconditionally, this breaks the build. Newer gcc/clang versions relaxed
this warning, so this was not a problem in those cases.
Change-Id: I7ad09370d96aa7384b2c117dd8de421644898b50
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/89785
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
This is meant to reduce lots of small allocations and instead have
pointers into the single string for the various tokens instead.
This has a few requirements, though:
1) It's no longer OK to modify the tokens, changing their length would
invalidate the start/length of other tokens. Rework
DocumentBroker::load() to avoid such mutation.
2) The iterators no longer expose zero-terminated strings, so
Poco::cat() doesn't work anymore: add an own cat() instead and use that
in e.g. ChildSession. The own cat() has the benefit that it won't read
past the end of the array if the begin index is out of bounds to add
more safety.
(This nicely works towards killing Poco usage in general.)
3) If zero-terminated strings for all individual tokens is needed, a
copy has to be made, as done in spawnProcess().
(For all of these requirements, the build fails if there are problems.)
Change-Id: Iea40e4400e630b2d669f5c72aea85cb40edf9a2c
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/89711
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
The bulk of this commit just changes std::vector<std::string> to
StringVector when we deal with tokens from a websocket message.
The less boring part of it is the new StringVector class, which is a
wrapper around std::vector<std::string>, and provides the same API,
except that operator[] returns a string, not a string&, and this allows
returning an empty string in case that prevents reading past the end of
the underlying array.
This means in case client code forgets to check size() before invoking
operator[], we don't crash. (See the ~3 previous commits which fixed
such crashes.)
Later the ctor could be changed to take a single underlying string to
avoid lots of tiny allocations, that's not yet done in this commit.
Change-Id: I8a6082143a8ac0b65824f574b32104d7889c184f
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/89687
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Termination flag is a very harsh way of exiting.
It works in most cases, but not when we have a
modified document. What happens is the following:
Unit-test flags for termination.
During session cleanup we have to save the modified doc.
Because save is in progress we don't 'disconnect' the view.
This leaves the view in loaded state until saving is done.
But because of the termination flag we don't wait for saving.
DocBroker sends 'exit' to child to forcefully exit.
This causes at least one assertion due to active LOKWindows (Sidebar).
Instead of the above, we flag for graceful shutdown from unittests,
and after we wait to cleanup all DocBrokers, we flag for termination.
This way, we get clean shutdown and all assertions/validations
pass, while we guarantee never to deadlock the unittests,
in case we end up waiting forever for shutdown to complete.
Change-Id: I7fc34137ea373e329795b1ed0090261c085e955a
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/89308
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
- target ClientSession::_handleInput(), since crashing there would bring
down the whole loolwsd (not just a kit process), and it deals with
input from untrusted users (browsers)
- add a --enable-fuzzers configure switch to build with
-fsanitize=fuzzer (compared to normal sanitizers build, this is the only
special flag needed)
- configuring other sanitizers is not done automatically, either use
--with-sanitizer=... or the environment variables from LODE's sanitizer
config
- run the actual fuzzer like this:
./clientsession_fuzzer -max_len=16384 fuzzer/data/
- note that at least openSUSE Leap 15.1 sadly ships with a clang with
libfuzzer static libs removed from the package, so you need a
self-built clang to run the fuzzer (either manual build or one from
LODE)
- <https://chromium.googlesource.com/chromium/src/testing/libfuzzer/+/refs/heads/master/efficient_fuzzing.md#execution-speed>
suggests that "You should aim for at least 1,000 exec/s from your fuzz
target locally" (i.e. one run should not take more than 1 ms), so try
this minimal approach first. The alternative would be to start from the
existing loolwsd_fuzzer binary, then step by step cut it down to not
fork(), not do any network traffic, etc -- till it's fast enough that
the fuzzer can find interesting input
- the various configurations start to be really complex (the matrix is
just very large), so try to use Util::isFuzzing() for fuzzer-specific
changes (this is what core.git does as well), and only resort to ifdefs
for the Util::isFuzzing() itself
Change-Id: I72dc1193b34c93eacb5d8e39cef42387d42bd72f
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/89226
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
==13901==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000904678 bp 0x7ffdb9e21580 sp 0x7ffdb9e21340 T0)
==13901==The signal is caused by a READ memory access.
==13901==Hint: address points to the zero page.
#0 0x904677 in LOOLProtocol::tokenize[abi:cxx11](char const*, unsigned long, char) common/Protocol.hpp:113:40
#1 0x898c52 in LOOLProtocol::tokenize(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, char) common/Protocol.hpp:141:16
#2 0x18dc2d9 in LOOLProtocol::ParseVersion(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) common/Protocol.cpp:35:51
#3 0x1148824 in ClientSession::_handleInput(char const*, int) wsd/ClientSession.cpp:358:64
#4 0x18efcb8 in Session::handleMessage(bool, WSOpCode, std::vector<char, std::allocator<char> >&) common/Session.cpp:232:13
Next commit will add the actual simple fuzzer that found this.
Change-Id: I8623b4451a57390f6f84c11084c5a1120a11fcc5
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/89225
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
After fixing this single assertion failure, the
./loolwsd_fuzzer --config-file=loolwsd.xml --o:storage.filesystem[@allow]=true --o:logging.level=fatal
invocation works. (It does not really fuzz anything, but it's a single
unpriviliged process at least.)
Change-Id: I45f877e5eb023e3ddfc96a7373c6300e4bb77962
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/89115
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
It can happen that a test succeeds, but right after that, the timeout
thread still fails the test:
09:21:10.411840 [ loolwsd ] INF exitTest: 1. Flagging for termination.| common/Unit.cpp:201
09:21:10.412016 [ loolwsd ] ERR Timed out waiting for unit test to complete| common/Unit.cpp:212
09:21:10.412060 [ loolwsd ] INF exitTest: 2. Flagging for termination.| common/Unit.cpp:201
(1 is TestResult::Ok, 2 is TestResult::TimedOut.)
Fix these cases by just ignoring the timeout invocation, we'll exit soon
anyway.
With this, finally a sanitizers-enabled 'make check' passes agains a
sanitizers-enabled core.git.
Change-Id: Id52c4b6f88a795d4ff2af7929a71d5eb83c56773
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/88999
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
This dates back to 2016 and our pre non-blocking and pre-unipoll state.
It is no longer necessary - a single thread reads all data from the
socket and feeds events into the Kit process; much cleaner.
Change-Id: I46ad6806a1e0cdbb0e5cf4ea5d3e5e5078d3391a
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/88741
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
When trying to open a link normally from help->Online help
nothing happens but the popup is closed.
When trying to open a like forcefully in new tab
from help->online help it crashes the server.
Change-Id: I7e0944ebe521002625a84e155e379ed7e25d2309
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/85466
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
ignoring the segv can lead to not making progress, while churning debug.
Change-Id: I97af266cec3feefe2dcbd9adb8dbf4b13a4d69bd
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/87002
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Use a fully reliable uniqueness check, rather than a hash, and get
simpler ids as a bonus. Fetch view data from the session itself
rather than passing it in too.
Change-Id: Ibcd625156b5a98eb280e35d6537b5c8c026d0197
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/86150
Reviewed-by: Mert Tümer <mert.tumer@collabora.com>
Tested-by: Mert Tümer <mert.tumer@collabora.com>
It is painful to check and search manually the PID to attach the LOKit
process when exists several pre-spawned waiting to load a document.
This patch helps to attach the debugger when the LOKit process is about
to load a document then send the "signal SIGUSR1" to resume it.
Change-Id: I3b15bd522c6ef3ef57dc3453b457dcf91f2661b9
Reviewed-on: https://gerrit.libreoffice.org/85430
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Henry Castro <hcastro@collabora.com>
Reviewed-by: Henry Castro <hcastro@collabora.com>
This is the cleanest way to achieve the goal
of immediately exiting a child. This is used
for cleaning up kit instances when closing
docs, as well as in unit-tests.
Change-Id: I76870234b130a508044044b102419646abe81ac8
Reviewed-on: https://gerrit.libreoffice.org/83699
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
malloc is not signal safe, and must not be called
from signal-safe functions. If malloc itself signals,
calling it in the signal handler can deadlock.
Luckily, we only needed malloc for getting the
backtrace strings. Now we just write directly to
stderr, which is faster, cleaner, and safer.
Change-Id: I54093f45e05f2a0fd3c5cde0cc2104ffe6d81d2a
Reviewed-on: https://gerrit.libreoffice.org/83151
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
There are a few things acceptable/safe in a
signal handler, and taking locks is not one of them.
This replaces the logic with a simple counter that
serves the purpose just as well.
If we get a double signal, we log and ignore.
Change-Id: If589c18492468c120d00c213805467bcbba05d27
Reviewed-on: https://gerrit.libreoffice.org/83150
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
removed use of Poco::StringTokenizer from the common directory
used LOOLProtocol::tokenize and std::vecor<std::string>
regex is used in Authorization.cpp due to limitation of toeknize mathod
regular expression helps to keep the original intention of the code
Change-Id: Ic87597d8b30cb385000f983389a57dc5d2533d98
Reviewed-on: https://gerrit.libreoffice.org/82575
Reviewed-by: Jan Holesovsky <kendy@collabora.com>
Tested-by: Jan Holesovsky <kendy@collabora.com>
