Bug 19723

Summary: Patch to expose Marshal and Demarshal in the Python API
Product: dbus Reporter: Ben Schwartz <bens>
Component: pythonAssignee: Simon McVittie <smcv>
Status: RESOLVED MOVED QA Contact: John (J5) Palmieri <johnp>
Severity: normal    
Priority: medium CC: robert, will
Version: unspecifiedKeywords: patch
Hardware: All   
OS: All   
Whiteboard: review- due to error handling
i915 platform: i915 features:
Bug Depends on: 22141, 23215    
Bug Blocks:    
Attachments: A patch to message.c to add marshal() and demarshal() methods
A patch to message.c to add marshal() and demarshal() methods

Description Ben Schwartz 2009-01-24 09:10:26 UTC
Created attachment 22205 [details] [review]
A patch to message.c to add marshal() and demarshal() methods

I would like to have dbus_message_marshal and dbus_message_demarshal available through dbus-python.  As such, I attempted to write a patch.  The patch is attached.  Marshaling seems to work, but demarshaling does not.  Here's an example python session:

>>> import dbus
>>> x = dbus.lowlevel.SignalMessage('/a/b','c.d','e')
>>> x.append('a test string')
>>> s = x.marshal()
>>> s
'l\x04\x01\x01\x00\x00\x00\x00\x00\x00\x00\x007\x00\x00\x00\x01\x01o\x00\x04\x00\x00\x00/a/b\x00\x00\x00\x00\x02\x01s\x00\x03\x00\x00\x00c.d\x00\x00\x00\x00\x00\x03\x01s\x00\x01\x00\x00\x00e\x00\x00\x00\x00\x00\x00\x00\x08\x01g\x00\x01s\x00\x00\r\x00\x00\x00a test string\x00'
>>> dbus.lowlevel.Message.demarshal(s)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: Message is corrupted

I have tested extensively to ensure that the string being produced by dbus_message_marshal is precisely identical to the one being provided to dbus_message_demarshal.  Therefore, I have no idea what the problem is.  My current estimation is that, possibly, dbus_message_demarshal has stricter validation requirements than dbus_message_marshal, and the message "x" in this example meets the marshaling requirements but not the demarshaling requirements.

Anyway, please advise.  I would like to get this patch tested, working, submitted, and included.
Comment 1 Ben Schwartz 2009-01-24 09:37:35 UTC
Created attachment 22206 [details] [review]
A patch to message.c to add marshal() and demarshal() methods

Removed #include<stdio.h>, which was being used for debugging.
Comment 2 Will Thompson 2009-06-07 09:37:50 UTC
As discussed on IRC, the problem is that dbus_message_marshal() does not fill in the message's 'length' header (which is done in _dbus_message_lock) before marshalling it.
Comment 3 Ben Schwartz 2009-06-15 20:16:58 UTC
I attempted to backport the patch from #22141 to my installation of libdbus-1.2.3.  Unfortunately, this did not solve the problem.  However, it did change the behavior.  A sample python session:

>>> import dbus
>>> x = dbus.lowlevel.SignalMessage('/a/b','c.d','e')
>>> x.append('a test string')
>>> s = x.marshal()
>>> s
'l\x04\x01\x01\x12\x00\x00\x00\x00\x00\x00\x007\x00\x00\x00\x01\x01o\x00\x04\x00\x00\x00/a/b\x00\x00\x00\x00\x02\x01s\x00\x03\x00\x00\x00c.d\x00\x00\x00\x00\x00\x03\x01s\x00\x01\x00\x00\x00e\x00\x00\x00\x00\x00\x00\x00\x08\x01g\x00\x01s\x00\x00\r\x00\x00\x00a test string\x00'
>>> y = dbus.lowlevel.Message.demarshal(s)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: Message is corrupted

The 4th byte, which was previously \x00, is now \x12.  This seems to reflect the length of the contents, which was added by the call to dbus_message_lock().

Unfortunately, it appears that more problems still remain in the marshalling or demarshalling code.
Comment 4 Will Thompson 2009-08-08 05:23:19 UTC
(In reply to comment #3)
> Unfortunately, it appears that more problems still remain in the marshalling or
> demarshalling code.

I see the same results when I do the same kind of thing with raw libdbus. :(

Comment 5 Will Thompson 2009-08-08 06:06:19 UTC
A-ha!

s[8:12] are the message's serial number, and are all zeroes. dbus_message_demarshal doesn't accept messages that have a zero serial number.

The spec never actually says that serials need to be non-zero, but given that this implies that the daemon will throw away messages unless the serial is non-zero, I propose amending the specification to match the implementation. I guess you'll need to wrap dbus_message_set_serial () in the Python binding, which makes me kind of sad.
Comment 6 Will Thompson 2009-08-08 06:13:24 UTC
(In reply to comment #5)
> I
> guess you'll need to wrap dbus_message_set_serial () in the Python binding,
> which makes me kind of sad.

I should explain my sadness: dbus_message_set_serial () will assert if you call it twice, so you'll probably have to wrap it to check if the message has a serial, and if it does throw a Python assertion rather than calling it again.
Comment 7 Ben Schwartz 2009-08-08 09:04:32 UTC
(In reply to comment #5)
> A-ha!
> 
> s[8:12] are the message's serial number, and are all zeroes.
> dbus_message_demarshal doesn't accept messages that have a zero serial number.
> 
> The spec never actually says that serials need to be non-zero, but given that
> this implies that the daemon will throw away messages unless the serial is
> non-zero, I propose amending the specification to match the implementation.

I'm not convinced.

You see, the reason I (personally) want to marshal and demarshal messages is to serialize Python D-Bus objects to disk (efficiently), and later reconstitute them back into D-Bus objects.  The marshaled messages never have to go through the daemon, so I don't mind that the daemon wouldn't accept them.  Therefore, I think a better solution would be to make dbus_message_demarshal accept messages with a zero serial number, since serial number is perfectly irrelevant to me.

An even better solution might be to do this and also add python bindings for set_serial, so that users can use dbus_message_demarshal without setting a serial number, but (hypothetical) users who _do_ need to set a serial number from python can.  However, as I'm the only person I know of who's requested this feature, I doubt a python binding for set_serial would get used.
Comment 8 Simon McVittie 2009-08-11 06:05:04 UTC
(In reply to comment #7)
> You see, the reason I (personally) want to marshal and demarshal messages is to
> serialize Python D-Bus objects to disk (efficiently), and later reconstitute
> them back into D-Bus objects.  The marshaled messages never have to go through
> the daemon, so I don't mind that the daemon wouldn't accept them.  Therefore, I
> think a better solution would be to make dbus_message_demarshal accept messages
> with a zero serial number, since serial number is perfectly irrelevant to me.

I'd like it to be like this: the message is syntactically well-formed, even if the daemon would never have accepted it.

However, since people are frequently (and correctly?) reluctant to update their libdbus, I think in the short-to-medium term, you'll also need to set a stub serial number when doing your serialization.

> An even better solution might be to do this and also add python bindings for
> set_serial, so that users can use dbus_message_demarshal without setting a
> serial number, but (hypothetical) users who _do_ need to set a serial number
> from python can.  However, as I'm the only person I know of who's requested
> this feature, I doubt a python binding for set_serial would get used.

Since set_serial is a "call no more than once" thing, I think I'd rather bind it as an optional kwarg to the constructor:

    m = dbus.lowlevel.SignalMessage(..., serial=1)
    m.append(*payload, signature='...')
    f.write(m.marshal())

If it's exposed as a method, it must raise a suitable exception (I'd use either ValueError or a DBusException subtype) rather than letting libdbus warn (which is fatal on approximately everything except Debian).
Comment 9 GitLab Migration User 2018-08-22 22:03:32 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus-python/issues/6.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.