xmpppy bug

[bb]

<message from='gajim@conference.gajim.org/zinid' type='groupchat' id='1' xml:lang='ru-RU'>\n <x:y/>\n</message>

This is why we need more mods in gajim@.

[asterix]

I don't see anything we can do. it's a parser error that can't be catched.

[misc]

For the record, can someone explain the problem ?

The xml snippet just arrived from the muc componant and it did something wrong to gajim ( crash ? traceback ? error ? )

Has this problem is rated as "critical", it would be nice to have more information, or to close it according to comment 1.

[evgs]

this problem is not directly related to muc - problem is mostly noticeable in muc:

if such message sent to conference gajim can't enter this conference until this message will be moved out from room history delivered at entering conference.

if muc is not used gajim only reconnects silently. but silent reconnect is critical because messages arrived into TCP buffer may be lost.

this bug is xmpppy bug, there are some another apps using xmpppy and affected the same problem.

stanza displayed above is valid, but not well-formed. most parsing library will return warnings if xml is not well-formed. but non-well-formed xml is fatal for xmpppy.

background: non-well-formed stanzas may be sent by j2j transport Jabber-To-Jabber Transport (GTalk, LiveJournal? inside),  http://wiki.JRuDevels.org/index.php/J2J bug is in the deep of used library (Twisted), and can't be resolved without hard tracing.

[tb0hdan@…]

guys, check out this:  http://svn.hypothetic.org/neutron/branches/gh0st-dev/modules/xmpp/dispatcher.py --- WBR, Gh0st, one of the Neutron developers

[asterix]

it works nicely for the described problem, but it break things when we receive big stanza (vcard with avatar for exemple) those stanza are received in 2 (or more) part. and each part is tested and of course rejected as it's not a complete stanza, so they are discarded ...

[bb]

Replying to misc:

For the record, can someone explain the problem ?

Try it out yourself with the XML console, you'll notice the problem soon enough.

Anyway, as Asterix said, it's not a problem of code written by the Gajim team indeed. The faulty code is, however, shipped with Gajim and as such users are going to see this as a Gajim bug, no matter how you put it :/

If you want to fix this, you'll have to work on the parser code, I guesss. If you do not, all gajim clients will be an easy target to shoot down for everybody who wants to be annoying.

[asterix]

here is a patch that doesn't check big packet (or packet for which previous is big ans it's the end of the big stanza, so invalid too)

That's definitly not very good, it's why I don't commit it

[anonymous]

<presence><caps:c/></presence>

[steve-e]

See  http://trac.gajim.org/ticket/2634

[js]

(In [cd4762f3423f1bc139fce64afed7447ac427da1c]) Don't disconnect on unbound prefixes. Fixes #3083. I can't believe this was unfixed for so long, as it's really a rather easy fix.

[bct]

 http://tools.ietf.org/html/draft-saintandre-rfc3920bis-05#section-12.3

A XMPP entity MUST NOT accept XML data from another entity if that data is not well-formed in accordance with both the definition of "well-formed" in Section 2.1 of [XML] and the definition of "namespace-well-formed" in Section 7 of [XML-NAMES]. In an XMPP entity receives XML data that is not so well-formed, it MUST return an <xml-not-well-formed/> stream error and close the stream over which the data was sent.

This message is not "namespace-well-formed", so we're doing the right thing according to the spec.

It may make sense for us to handle this specific case if the server isn't following the spec (but IMO handling arbitrary XML-like strings is a path we don't want to go down).

[bct]

(reopened because js reverted his fix)

[steve-e]

Furtheremore the problem hasn't only to do with namespaces. Any malformed xml can cause the stream to drop.

[js]

Sorry bct and steve-e, but your both wrong: The problem is not malformed XML. The server never routes that, it is required to drop that according to XMPP Core and all servers do so. But unbound namespaces MAY be routed, and ejabberd decides to do so. On malformed XML, we SHOULD disconnect and we do so, but on unbound namespaces we SHOULD just ignore that.

Quoting XMPP core: Except as noted with regard to 'to' and 'from' addresses for stanzas within the 'jabber:server' namespace, a server is not responsible for validating the XML elements forwarded to a client or another server; an implementation MAY choose to provide only validated data elements but this is OPTIONAL (although an implementation MUST NOT accept XML that is not well-formed). Clients SHOULD NOT rely on the ability to send data which does not conform to the schemas, and SHOULD ignore any non-conformant elements or attributes on the incoming XML stream. Validation of XML streams and stanzas is OPTIONAL, and schemas are included herein for descriptive purposes only.

This means we can receive invalid XML (for example, unbound namespace, which is the problem here), but NEVER malformed XML. Thus we handle malformed XML correctly, but don't ignore non-conformant like we should.

[bct]

js, the stanza is *not* namespace well-formed.

 http://www.w3.org/TR/REC-xml-names/

The Prefix provides the namespace prefix part of the qualified name, and MUST be associated with a namespace URI reference in a namespace declaration.

...

Definition: A document is namespace-well-formed if it conforms to this specification.

[js]

By well-formed, one usually refers to the XML being not malformed, whereas invalid means something makes it non-conformant, for example namespaces like in this case.

[bct]

"well-formed" = follows the XML spec "namespace-well-formed" = follows the XML namespaces spec

As I quoted in  http://trac.gajim.org/ticket/3083#comment:11 , the revised RFC specifically says to close the stream if it isn't namespace-well-formed.

[js]

The revised does? Well, that'd be a bad change, or will jabber servers filter it then? Can you please quote that part or give a link? If the server should filter it in the revised RFC, I'll talk to the ejabberd devs again.

[bct]

 http://tools.ietf.org/html/draft-saintandre-rfc3920bis-05#section-12.3

A XMPP entity MUST NOT accept XML data from another entity if that data is not well-formed in accordance with both the definition of "well-formed" in Section 2.1 of [XML] and the definition of "namespace-well-formed" in Section 7 of [XML-NAMES]. In an XMPP entity receives XML data that is not so well-formed, it MUST return an <xml-not-well-formed/> stream error and close the stream over which the data was sent.

So the server should be dropping it. I logged it as a bug against ejabberd:  https://support.process-one.net/browse/EJAB-680

[bct]

I wrote a test for this issue: source:trunk/test/test_dispatcher_nb.py

It should help shorten the debugging cycle. Run it with python test/test_dispatcher_nb.py

[js]

bct, the problem is that this is only for the revised RFC, not for the current RFC :(. For the current RFC, we do it wrong whereas with the revised RFC, ejabberd does it wrong. So, this still is a issue on our end.

[bct]

The current RFC isn't specific about what it means by "well-formedness". It's not wrong to drop the message, it's just using a strict interpretation. It makes sense for the server to reject it anyways IMO, there's no reason not to.

Anyhow, we have to deal with it so I've attached my patch attempt. It buffers the stream until it thinks it's reached the end of the bad stanza, drops the whole thing and starts parsing again.

I don't think dropped messages will even end up in the XML console, so it needs to be tested carefully.

[bct]

Did some digging in pyexpat's internals. It looks like we can turn off expat's namespace processing by removing the namespace_separator=' ' from line 297 of simplexml.py. I don't know what side effects this would have.

[js]

That does not work as all our code relies on a namespace aware expat, I already tried it. Most of simplexml.py would need to be rewritten, I already had a few tries with it.

[asterix]

we choosed to live with this big for the moment. That's no SO critical as that closes stream and we don't lloose anything.

[anonymous]

Replying to tb0hdan@gmail.com:

guys, check out this:  http://svn.hypothetic.org/neutron/branches/gh0st-dev/modules/xmpp/dispatcher.py --- WBR, Gh0st, one of the Neutron developers

URL changed:  http://code.google.com/p/neutron/source/browse/branches/gh0st-dev/modules/xmpp/dispatcher.py

[dwd]

Seems to now be 100% working. Returns two bogus namespaces for undeclared namespaces. Running for personal use quite happily.

[dwd]

Last patch seems to be working fine for both me and a couple of other folk who've tried it. I'll happily fix anything that comes up, though.

[asterix]

execute commands don't work anymore with the patch :/

[asterix]

(In [bf58ce53946b430e9b5a1a73567faa029d40130f]) [dwd] fix reconnection when we get wrong XML with undeclared namespaces. Fixes #3083