Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#13073 closed patch (fixed)

patch to fix D-Bus introspecction

Reported by: tecknicaltom Owned by: grim
Milestone: 2.7.10 Component: libpurple
Version: 2.7.7 Keywords:
Cc:

Description

The D-Bus spec says that when calling a method, the interface field is optional. Currently, when the Introspect method is called on the PurpleObject?, if the interface is not given, introspection never happens and an empty xml document is sent as a reply. This can be demonstrated with the program dbus-cli available from here.

This patch changes the code to use the provided dbus_message_is_method_call function to test if the message is attempting to call the Introspect method. This function properly handles an omitted interface parameter.

The patch also adds the Introspectable interface to the returned introspection xml and format the xml a bit for the next unfortunate soul that needs to debug at the lowest levels of dbus.

Attachments (1)

dbus-introspection.patch (3.2 KB) - added by tecknicaltom 8 years ago.
patch to fix dbus introspection

Download all attachments as: .zip

Change History (14)

Changed 8 years ago by tecknicaltom

patch to fix dbus introspection

comment:1 Changed 8 years ago by datallah

  • Milestone set to Patches Needing Review

comment:2 Changed 8 years ago by deryni

The main change appears to be correct to me given the wording in the spec. Though I don't know that I like that spec decision. It means that it you ever grow an extra interface on an object introspection calls which used to work will begin either "failing" randomly or throwing errors immediately, but maybe that's not as big of an issue as I think it is, I'm only somewhat familiar with D-Bus.

I can't comment on the formatting changes at the moment and I'd have to look up the details on the introspection interface change more to know what to comment on that.

comment:3 Changed 8 years ago by rekkanoryo

deryni, given your comment, is this patch acceptable as-is?

comment:4 Changed 8 years ago by deryni

I think the last hunk is certainly acceptable, you'd have to compare the introspection xml to confirm that the formatting changes are correct (though I imagine they are fine), and someone more familiar with D-Bus would have to weigh in on whether listing the introspection interface in the introspection data is correct (I don't see anything either way about it in the spec and I'm not at all sure I see what it helps since you can only get it once you've already done introspection) but it doesn't look like it'd hurt anything to me.

I'd love to get someone more familiar with D-Bus to comment on this all.

comment:5 Changed 8 years ago by rekkanoryo

  • Owner set to grim

Let's make grim read a ticket for a change :-P

He's familiar with D-Bus.

comment:6 Changed 8 years ago by rekkanoryo

  • Milestone changed from Patches Needing Review to 2.7.10

comment:7 Changed 8 years ago by nix@…

  • Milestone changed from 2.7.10 to 2.8.0
  • Resolution set to fixed
  • Status changed from new to closed

(In f07e2367652cc664834b1968911b2455b6b0640e):
Add pidgin_make_scrollable and use it. Cleans up a bunch of duplicate code. Net code loss of 180 lines. Fixes #13073.

comment:8 Changed 8 years ago by rekkanoryo@…

(In ecfa76230379acd410497a0402c297294db7302f):
Credit Gabriel for his patch. Refs #13073.

comment:9 Changed 8 years ago by rekkanoryo

  • Resolution fixed deleted
  • Status changed from closed to new

comment:10 follow-up: Changed 8 years ago by rekkanoryo

  • Milestone changed from 2.8.0 to 2.7.10

I'm inclined to commit this patch. tecknicaltom, what name and e-mail should I use to credit you?

Sorry about the noise here. I accidentally referenced the wrong ticket when committing another patch.

comment:11 in reply to: ↑ 10 Changed 8 years ago by tecknicaltom

Replying to rekkanoryo:

I'm inclined to commit this patch. tecknicaltom, what name and e-mail should I use to credit you?

Thanks rekkanoryo. I'm already in the copyright file as Tom Samstag, and you can add the e-mail address pidgin-at-modtwo.com. Thanks again.

comment:12 Changed 8 years ago by pidgin@…

  • Resolution set to fixed
  • Status changed from new to closed

(In bdca8e1103c8ba5e56e72784f743212e91fc91ae):
Fix dbus introspection. Fixes #13073.

comment:13 Changed 8 years ago by rekkanoryo@…

(In 5982603b9ad983ce3c1359e0a967a3e561f645a1):
ChangeLog and credit Tom for his patch. Refs #13073. And it's the right ticket number this time.

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!