The extra "mustAuthenticate" parameter to the Session::sync() method
is replaced with temporarily setting username/password to empty
via the Session::m_syncFilter.
To catch undesired prototype changes of D-Bus API calls (as it
happened when adding that parameter), all these methods now come with
a comment that links them to the corresponding D-Bus API call.
test-dbus.py tests cover the different credential checks:
- TestConnection.testStartSync: invalid MD5 creds, no authorization
necessary => client accepted
- TestConnection.testCredentialsWrong: invalid MD5 creds, authorization
necessary => client rejected
- TestConnection.testCredentialsRight: correct basic creds, authorization
necessary => client accepted
The server was always asking clients to authenticate with
the more secure MD5+Nonce method. For clients which don't
support that, there was no way to tell the server to accept
them.
Changed the configuration so that basic authentication is
accepted. The server will still send a Nonce and ask for MD5 for
failed logins, so clients can (and should) use MD5+Nonce.
The motivation for this change was not a specific client, but
testing (will be committed later): sending valid credentials
in a preformatted message is easier with basic authentication,
exactly because it is susceptible to replay attacks.
Lukas pointed towards <logininitscript>return TRUE</logininitscript>
as a better way of accepting all kinds of sessions, even those with
invalid credentials. For good measure, requestedauth/requiredauth
are still set to none, even though that shouldn't matter now.
The Server.GetConfig() is stricly read-only. When looking at the
error message for NoSuchConfig I noticed that it creates the config
from a template with the name if the config doesn't exist yet. This
was discussed before and should have been removed, but this instance
must have slipped in.
Removed and adapted test for error message accordingly.
Reading a non-existant configuration should result in
org.syncevolution.NoSuchConfig (MB#6548).
Running a sync without a valid configuration should
result in a bad status code.
Both tests passes.
The server now checks whether it has other sessions active or in the
queue with the same device ID and kills the older sessions when adding
a new one. Because it does this on each Process(), it is unlikely
(impossible?!) to to have more than one older session, but because the
session might be running or still inactive in the queue, both are
checked.
As part of testing with test-dbus.py it was observed that a connection
that was to be aborted was also closed normally first. That was because
the sync engine did not set an error status in this case. Better check
for abort requests explicitly before calling agent->shutdown(). However,
this should be fixed as part of the server progress handling (MB#7709).
It was implied, but not explicitly specified and not implemented like
this: with this patch, when a connection is aborted, the "Abort"
signal is sent and no further "Reply", not even the one which is
normally used to close the connection.
Define a DBusCxxException class in gdbus cxx bridge.
It only has 2 virtual functions: getName, getMessage,
which provide the information necessary to generate
a D-Bus error.
Then define a class DBusSyncException deriving from
DBusCxxException and SyncEvo::Exception.
Also 3 classes NoSuchConfig, NoSuchSource,
InvalidCall are based on DBusSyncException.
Turn some errors into these 3 specific errors.
Define a macro to let user have an opportunity to write
its own exception handling function. The SyncEvoHandleException()
logs the error via Exception::handle() and then overrides
the conversion of all exceptions so that they result in
an "org.syncevolution.Exception", unless specified otherwise
when throwing a DBusCXXException or dbus_error.
This patch fixes signal handling and shutdown of syncevo-dbus-server
when signals were received. This problems were found in combination
with automated tests via test-dbus.py.
Signals handled by the signal handlers in the core had no effect on
syncevo-dbus-server, which called its own event loop once the
suspended or aborted sync returned. Signals received while outside
of a sync and outside of an event loop also had no effect.
Now signals are always caught by the niam() function in
syncevo-dbus-server. It uses the new SyncContext::handleSignal() API
to tell the core engine about it and in addition, remembers that
syncevo-dbus-server is meant to shut down (suspendRequested). This is
then checked by the syncevo-dbus-server event loop (DBusServer::run())
before blocking again.
The implementation of Session.Abort() and Session.Suspend() didn't
work. isSuspend() and isAbort() mixed up the state. They also did not
wake up DBusTransportAgent, so the syncevo-dbus-server might get stuck
although an abort was already requested. Partially fixed by adding
g_main_loop_quit(). It's not entirely sure whether g_main_loop_quit()
is reentrant (see below). There is other code which uses it in signal
handlers, but that doesn't mean that this is correct. The right long
term solution would be:
- avoid entering and leaving the event loop
- feed signals into the event loop as normal events
SoupTransportAgent solves this by polling for a signal once per second.
This also should be improved.
Because such code was used in several places for a connection, that common
code was moved into Connection::wakeupSession().
Things which went in unnoticed when the original signal handling was merged:
- SyncContext::getSuspendFlags() should not allow the caller to modify
the SyncContext state. Made the returned SuspendFlags const.
- A signal handler may only call re-entrant functions (Stevens, "Advanced
Programming in a Unix Environment", 10.6).
All printing therefore has to be done outside of the signal handlers.
Now the signal handlers just set a message and the regular code
checking for abort/suspend prints it. There's a slight race condition
(a message might get overwritten before it is printed), but printing
just the last message might actually make more sense. There might be
a slight delay between receiving the signal and printing now.
Follow up the previous behavior of syncevolution core
about CTRL-C/SIGINT handling.
Also add SIGTERM handler to abort sync immediately.
currently dbus server checks abort/suspend requests
not only from client but also from 'CTRL-C'/SIGINT
signals.
The location of Boost specified or found in the configure script was
not used during compilation. Now it is part of the global
BACKEND_CPPFLAGS variable which is used in all parts of SyncEvolution.
When the syncevo-dbus-server receives a SyncML message as initial
data from a transport stub, it peeks into the data with the help
of the Synthesis server engine and our SyncEvolution_Session_CheckDevice()
and extracts the LocURI = device ID of the client. This value is
guaranteed to be in the initial SyncML message.
Then it searches for a peer configuration (still called "server
configuration" in the current code) which has the same ID in its
remoteDeviceID property and uses that configuration.
Instantiating SyncContext and Synthesis engine multiple times
is a bit tricky. The context for the SynthesisDBPlugin currently
has to be passed via a global variable, SyncContext::m_activeContext.
SyncContext::analyzeSyncMLMessage() temporarily replaces that
with the help of a new sentinal class SwapContext, which ensures
that the previous context is restored when leaving the function.
Some common code was moved around for this (SyncContext::initEngine()).
The "default config" parameter in syncevo-http-server.py was
removed because it is no longer needed. The possibility to
select some kind of config context via the path below the
sync URL still exists. This is passed to syncevo-dbus-server via
the "config" peer attribute. It has no effect there at the
moment.
TestConnection.testStartSync() in test-dbus.py covers this kind of
config selection. To run it, a peer configuration with remoteDeviceID
"sc-api-nat" must exist.
The Synthesis engine calls some of the functions which depend
on a context even if creating the context failed. Normally
that does not happen, but better protect against it by
checking for NULL.
The Session::sync() prototype was changed without taking into
account that the method has to match the Session.Sync() signature.
Automaticly generated D-Bus bindings are a double-edged sword...
This was caught by the test-dbus.py TestDBusSync test. We really
need to run that more often and eventually nightly.
The new TestConnection suite covers some Connection API related
aspects:
- creating a connection and closing it
- sending an invalid message
- sending a valid message
Right now these tests assume that a "syncevolution_server" config
exists. They depend on getting one Abort signal per connection
to shut down each test.
The signal recording code from TestDBusSync was moved into DBusUtil
so that the different tests can share this utility code.
Some sleeps are necessary after sync sessions because SIGTERM might
get caught without properly shutting down synevo-dbus-server (MB#7555).
First, kill all running syncevo-dbus-server instances. Having
one running was one way how the tests could have failed because
it wasn't running with the right environment.
Second, use files for output instead of in-memory pipes. The latter
might have caused deadlocks when a process writes large amount of
data.
syncevo.log and dbus.log in the current directory are used. The
additional benefit is that these files can be read while the test
runs, for example when it is deadlocked for some other reason.
Third, when syncevo-dbus-server returns with a non-zero error code,
give dbus-monitor time to settle down before stopping it. Otherwise
the final D-Bus traffic might not get recorded.
The API spec now guarantees that the signal is only raised once for
each connection. This was unspecified earlier. This change was made
to simplify clients and make the server more deterministic.
A new test in the test-dbus.py will depend on this behavior.
In a transport which anyone can send messages to (like in a HTTP
server), a random session ID protects a bit against someone
inserting unwanted messages into a regular session.
This patch uses the usec part of gettimeofday() to initialize
the libc random number generator. Better (and more complex)
approaches are of course possible...
The reason for implementing this now is that automated testing via
test-dbus.py also had a problem with non-unique IDs: when the
server was restarted, separate runs ended up using the same
session ID because they were initialized with the current system time
in seconds. This confused signal listeners which checked the ID.
Running the script failed on Debian Lenny failed because Python
and python-gobject are too old. Python 2.6 was required because
of subprocess.terminate(), replaced with os.kill(). python-gobject
2.16 was needed for glib, which is used in some tests to run
the event loop for a short while. These tests now fail if glib
is not available, while the rest of the tests can be run normally.
Due to a typo in the src/syncevolution -> src/syncevo transition,
unit tests inside libsyncevolution.a were not included in the
client-test executable. The check of libsyncevolution.a searched
for the library in synccevo instead of syncevo.
The "mustAuthenticate" parameter in the D-Bus Server.Connect()
call was ignored, valid credentials were always required unless
username and password in the server configuration were empty.
Now this parameter is passed through and used
- to configure the Synthesis engine correctly
- to suppress asking for our password if not needed
Note that we rely on a patch that turns <requiredauth>none</requiredauth>
into "accept invalid credentials". Without that patch, the client would
have to provide no credentials at all (which SyncEvolution doesn't support).
The "must authenticate" parameter for Server.Connect() had no
effect so far. Netherless, HTTP clients should have been required
to authenticate from the beginning.
Starting with this commit, SyncEvolution implements the
Synthesis session authentication and device administration
itself. The goal is to have better control over these steps
and become independent of the SDK_textdb which implemented
this before.
The SDK_textdb works, but it has several limitations, among
them the inability to choose a password. It is also called
"demo" in the source code, which raises some doubts about
its stability.
Because the Synthesis DB interface is now used both for database
access and for session handling, the Module_CreateContext() function
can not always select a source as context. When used as session
plugin, the global context pointer is NULL.
The Synthesis engine calls us at various points to save information.
Because it is not exactly clear how long we can buffer this
information in memory, it is always flushed to disk immediately.
The ! sign was used as escape character and thus has to be escaped
itself. In the less strict mode this wasn't done. Shouldn't affect
SyncEvolution 0.9.1 where the code was used in strict mode for
change tracking nodes, but broke the new nonce saving.
Part of the D-Bus API testing is checking of the GetConfigs()
call for templates. So when adding a new server template, that
test must also be updated.
Calculate progress information according to progress
event from synthesis library.
Considering the most common scenario and profiling data,
current implementation will partition a sync session into
3 *big* steps: SYNC_INIT, SYNC_DATA, SYNC_UNINIT
Typically each step contains one message send and receive,
thus occupies a specified proportion.
By default, 5 data items are supposed in calculating
proportions, which affect SYNC_DATA and SYNC_UNINIT.
The main idea of dynamically adjustment is that based on
the common scenario, if there are extra operations,
such as message send/receive or many data items,
adjust proportions by re-calculate operations of each
remaining step.
Typcially possible extra operations are more than one-time
message send/receive in a step and the number
of data items is not 5. Then we re-calculate proportions
for remaing steps according to future's operations
with the remaining proportion.
--with-synthesis-src requires a parameter. Without it, "yes"
was used as value, leading to a pretty obscure error message
about "no configure script in yes".
That error message should mention --with-synthesis-src. It now
says:
error: --with-synthesis-src=/tmp: no Synthesis configure script found in that directory
The recently introduced enableServerMode() and serverModeEnabled()
calls must be implemented by all sync sources not derived from
TrackingSyncSource. This patch adds the calls to the SQLiteContactSource,
using the default mechanism provided for it in SyncSourceAdmin.
The "GConf Error: Failed to contact configuration server..." error
message was assigned the Logger::DEV severity and thus never shown to
users. At low loglevel settings it probably also never made it into a
log file, if one was written at all.
This patch raises the severity of all output lines which contain the
word "error". Clearly this only works for non-localized error
messages, but hopefully that's the majority of the relevant error
messages on stderr.
The active sources must be set before calling sync(), doing it in
prepare() is too late. The right way to do it with the revised
API is via source filters: disable all sources with a filter that
applies to all sources, then enable the desired one(s) with the
right mode with a more specific filter.
The environment with XDG_ redirected into a local directory
also needs to be passed to gdb when debugging interactively.
Otherwise the tests would not run as without gdb.
Use 5 enum values to represent sync statuses,
SYNC_IDLE, SYNC_RUNNING, SYNC_ABORTING,
SYNC_SUSPENDING, SYNC_DONE
So we could use one enum variable instead of
previously 3 boolean values
Add test script in test/dbus-server-config.py to test
getConfigs, usage:
python dbus-server-config.py [servername] --getconfigs
[null or any characters]
Hook up checkForSuspend and checkForAbort in dbus server
and implement sleep function but not suspend and
abort when CTRL-C is entered for often sync dbus server
runs as a daemonease enter the commit message for your changes.
For status and progress, we need a timer to record when
the last signal was triggered.
Here implement a 'Timer' class and add 2 timers in session
to keep time track of emitting signals for status and progress
Previous impl removes necessary meta information, such
as global synthesis and source meta.
This patch is only to remove un-set sources and don't remove
meta information for global synthesis and sources which aren't be
removed
