Adium

Ticket #6317 (closed defect: fixed)

Opened 3 years ago

Last modified 2 years ago

Sends invalid XML when doing link-local messaging (XEP-0174)

Reported by: sjoerd Owned by: nobody
Milestone: Adium 1.2 Component: Service/Bonjour
Version: 1.0.2 Severity: normal
Keywords: xml bonjour link-local messaging Cc: christian@…
Patch Status:

Description

When checking why salut (telepathy xep-0174 CM) didn't work with Adium. We noticed that the XML Adium sends is actually invalid.

The received message was (note the FONT element): 

<message type="chat" to="192.87.65.58">
  <body>blablabla</body>
  <html xmlns="http://www.w3.org/1999/xhtml">
    <body ichattextcolor="#000000">
     <FONT FACE="Lucida Grande" ABSZ=11 SIZE=3>
       blablabla
     </FONT>
    </body>
  </html>
</message>

Futhermore the "to" attribute in the message element is a raw ip address, while this should contain the logical local address and the "from" attribute is missing. See XEP-0174 for more info... :)

Change History

  Changed 3 years ago by cbarrett

Ideally, we would just like to use Telepathy. But there is currently no good way of working with dbus on Mac OS X :\ If I had enough free energy, I would work on improving that situation, or on just smoking the dbus raw and writing a protocol plugin for Adium that way.

Anyway, our Bonjour implementation is quite stale. Since you know a lot about the spec, let me ask you: How well does Gaim's implementation do? It's possible we could use that.

  Changed 3 years ago by proton

The HTML sent is generated by Adium and should be passed to libezv as valid XML. This will need to be fixed in Adium -- there needs to be a way to generate valid HTML not AIM-style.

As to not matching the XEP spec this is because libezv was based on iChat's implementation long before the XEP existed. We should probably attempt to match iChat and the XEP spec where possible, however I think iChat compatibility should come before the XEP spec if there are any conflicts.

The "to" section is based on iChat 1.0, and may be possible that it should be handled differently for iChat 1.0 and iChat 2.0 and later clients.

Gaim's implementation can not be used on Mac OS X as it requires a third party mDNSResponder.

  Changed 3 years ago by am

It's possible to generate XEP-complying XHTML with the Adium code. You might want to look into [source:branches/smack/Plugins/Smack%20Service/SmackXMPPAccount.m@HEAD#L532 SmackXMPPAccount.m].

  Changed 3 years ago by proton

Unfortunately the valid XHTML created by Adium is not correctly interpreted by iChat. iChat doesn't take any notice of span tags with CSS styling, but uses a font tag instead.

iChat will use something like this: 

<font face="Helvetica" ABSZ="12">text here</font>

Whereas Adium's XHTML generator will create: 

<div><span style="font-family: Helvetica; font-size: 12pt;">text here</span></div>

It looks like a bit more custom HTML generation is needed to handle this properly for iChat compatibility which is pretty much essential for Bonjour.

  Changed 3 years ago by sjoerd

I've tested salut against iChat 2.0 and that just worked fine.. In this case salut is strictly adhering to the spec for what it's sending out, but being permissive enough on the input side to work with iChat. Which mostly means that it allows message stanza to have a wrong to attribute and no from attribute. I didn't test with iChat 1.0 though..

The main issue in this bug is obviously the invalid XML though. If that's fixed it will be nicely compatible with salut. Actually adhering to the standard is mostly just for extra bonus points in most practical situations :)

follow-up: ↓ 9   Changed 3 years ago by am

Since <font/> is a violation of  XEP-0071: XHTML-IM, maybe Adium should detect whether the other side is using iChat or a proper XMPP-client (via a jabber:iq:version query) and adjust the messages accordingly. A good XHTML-IM implementation should *always* filter out non-conforming XHTML and not just send it verbatim to a generic html rendering engine, since this could lead to malicious messages taking over the client's message view (via javascript:-urls for example -- since the message view's html is generated locally, the security settings probably aren't as strict as when rendering a remote page).

  Changed 3 years ago by sjoerd

Just to prevent confusion. It doesn't matter for me if the xhtml/html that's generated is valid as long as the document is well-formed XML. (Sorry if calling it invalid XML caused any confustion). Thus if in the example given in my report 

  <FONT FACE="Lucida Grande" ABSZ=11 SIZE=3>

would be replaced by 

  <FONT FACE="Lucida Grande" ABSZ="11" SIZE="3">

all would be just fine from salut's perspective :)

  Changed 3 years ago by cbarrett

I would recommend running tests to see if iChat can accept quotes.

Hopefully we can get away with just adding another knob to AIHTMLDecoder (ugh).

in reply to: ↑ 6   Changed 3 years ago by tick

Replying to am:

Since <font/> is a violation of  XEP-0071: XHTML-IM, maybe Adium should detect whether the other side is using iChat or a proper XMPP-client (via a jabber:iq:version query) and adjust the messages accordingly.

Is this going to be viable for all xmpp clients/old style jabber clients?

follow-up: ↓ 12   Changed 3 years ago by cbarrett

"This" being? You're very unclear there.

  Changed 3 years ago by proton

iChat does not support jabber:iq:version queries (at least not over Bonjour).

cbarrett is right, this would be best (if you can describe it that way) to implement using another knob in AIHTMLDecoder.

in reply to: ↑ 10   Changed 3 years ago by tick

Replying to cbarrett:

"This" being? You're very unclear there.

I thought what I typed was clear, but since it's not, let me quote for context:

Replying to tick:

Replying to am:

Since <font/> is a violation of  XEP-0071: XHTML-IM, maybe Adium should detect whether the other side is using iChat or a proper XMPP-client (via a jabber:iq:version query) and adjust the messages accordingly.

Is this going to be viable for all xmpp clients/old style jabber clients?

am is suggesting functionality which would allow Adium to "detect whether the other side is using ichat or a proper XMPP-client". That is what I meant by "this".

  Changed 3 years ago by evands

It'll need to be optional whether quotes are used around the ABSZ and SIZE arguments. AIM for Mac (and possibly AIM for Windows) does *not* understand quoted size arguments in font tags (see #5274, fixed by [17423] which removed the quotes).

  Changed 3 years ago by jas8522

  • patch_status set to None
  • version changed from 1.0 to 1.0.2
  • milestone set to Needs dev review

  Changed 3 years ago by jas8522

  • keywords xml bonjour link-local messaging added

  Changed 2 years ago by jas8522

  • pending set to 1
  • milestone changed from Needs dev review to Needs feedback from users

Is this fixed for 1.2 with our new Bonjour implementation? Check the AdiumBeta.

  Changed 2 years ago by islandsvinur

I tested the 1.2 release against salut on maemo and can confirm it works!

  Changed 2 years ago by evands

  • status changed from new to closed
  • resolution set to fixed
  • milestone changed from Needs feedback from users to Adium X 1.2

Thanks!

Note: See TracTickets for help on using tickets.