sending error on corrupt data
Havoc Pennington
hp at redhat.com
Mon Nov 19 15:37:22 PST 2007
Hi,
I investigated a bit what was involved here; I thought it might be easy
to just fix, but it wasn't _that_ easy, so I'll write down what would be
involved in case someone has more time.
Problem Statement
===
Corruption (non-well-formedness) currently results in dropping the
socket without sending any kind of error.
What would be better: attempt to send an error "blind" - i.e. without
blocking (give up if buffer is full), and without doing any reading off
the socket. Then disconnect as before.
In theory, virtually all disconnects here could be avoided through
_dbus_return_if_fail() in libdbus. However, one or two of them perhaps
can't be, and the libdbus checks don't help people reimplementing the
protocol.
Code
===
The relevant code includes:
dbus-message.c:load_message() parses the message and sets the
DBusMessageLoader::corrupted flag if the message is corrupted.
dbus-marshal-header.c:_dbus_header_load() parses the header
specifically, and is most likely to encounter some kind of corruption
problem. The message body can only be corrupt if someone's marshaling
code is truly broken, while the header has other "higher level"
well-formedness requirements like the presence of certain fields.
dbus-transport.c:_dbus_transport_queue_messages() has the logic that
calls _dbus_transport_disconnect() if the loader is corrupted.
Problems
===
1) Sending an error requires a message serial to reply to. Otherwise,
nobody will get the error. But the message was corrupted.
Possible solution: only send an error if the corruption is after we read
the reply serial, in _dbus_header_load(). There are various possible
corruptions before that point, but I don't know how to send errors for
those.
2) Currently, all the code for writing out messages relies on pulling
messages from the outgoing queue. It's an ugly layering violation to
call _dbus_connection_send_unlocked_no_update() from dbus-transport.c,
but I don't really have a better idea. It would be a mess to try to
support writing out a message directly instead of putting it in the
queue first.
3) Corruption errors are currently just an enum, so we'd need a strerror
type thing for the enum.
Possible Alternate Solutions
===
- have some kind of new signal (or message type?) for "disconnect
reason" so we don't need a reply serial - i.e. send a generic
disconnect reason message instead of a reply to the bad message. This
would solve problem 1)
- print a warning to stderr instead of trying to talk to the
remote app (I don't like this idea much)
Havoc
More information about the dbus
mailing list