Opened 6 years ago

Closed 6 years ago

#3407 enhancement closed fixed (fixed)

twistd xmpp-router command for running an xmpp router

Reported by: glyph Owned by:
Priority: high Milestone:
Component: words Keywords:
Cc: glyph, oubiwann, exarkun, twonds, therve Branch: branches/xmpp-router-3407-2
(diff, github, buildbot, log)
Author: ralphm Launchpad Bug:

Description

XMPP does things other than chat, so there should be a general XMPP router in Twisted.

There are a variety of other potential twistd commands for jabber, so perhaps this should actually be 'twistd xmpp router'; but this is the simplest and should come first.

Attachments (2)

xmpp-router.patch (44.3 KB) - added by ralphm 6 years ago.
Patch bomb resulting in a XMPP router plugin for twistd.
test.py (2.3 KB) - added by ralphm 6 years ago.
Test script for XMPP router.

Download all attachments as: .zip

Change History (26)

comment:1 Changed 6 years ago by glyph

#2328 is still a very important feature, but I have now been educated as to the various components involved in a jabber server speaking to chat clients, and it is clear that there should be some other, lower-level parts of XMPP exposed as tools that developers can use without necessarily coding.

comment:2 Changed 6 years ago by oubiwann

  • Cc oubiwann added

Here are some notes that I took from the discussion that generated this ticket:

http://twistedmatrix.com/trac/wiki/XMPPServerArchitecture

And here is a log of the raw IRC transcript:

http://twistedmatrix.com/trac/wiki/XMPPIRCTranscript20080903

comment:3 Changed 6 years ago by ralphm

A router in a traditionally designed XMPP server accepts TCP connections to establish bidirectional XML streams. Other entities (usually called XMPP server-side components) connect to the router and trust it to forward XML Stanzas on their behalf. The whole of the router and its connected components is generally known as an XMPP server.

Each component is responsible for, and binds to, a particular domain name. These might be externally routable domains, but also domains that can only be reached within the server. Examples of the latter would be component providing server-to-server connectivity (s2s) or client-to-server connectivity (c2s).

There are two protocols that detail how components connect to a server (i.e. its router):

  • Jabber Component Protocol (XEP-0114): This is the most widely implemented connection protocol that dates back the first Jabber server implementation.
  • Component Connections (XEP-0225): A new, yet experimental, protocol that is meant to be a replacement of XEP-0114 that resolves a number of drawbacks in that protocol. It brings it in line with XMPP 1.0, providing TLS, SASL and the announcement of stream features, and allows for binding more than one domain to a component.

XEP-0225 will likely be the basis of additional in-server protocols, for example to provide access to a session manager to enable a separate component to implement Personal Eventing. However, it is not finished, yet.

An additional requirement is to allow for components that connect to the router from within the same process as the router, i.e. without using TCP connections.

Some server implementations provide an additional <route/> stanza to envelope messages sent between components, that either come from, or destined to, external entities (like clients or other servers). I don't think we need to implement that at first.

I suggest this ticket addresses the conception of router that implements XEP-0114 as well as a way to do in-process routing, leaving XEP-0225 and other extensions to be addressed later.

comment:4 Changed 6 years ago by ralphm

Ok, I have it all working. It requires a bit of rework of some existing things, and some new helper functionality that I need to split across some tickets, probably. The code is currently in my local mercurial repository, as mercurial queues patches. For a first look, I attach a big patch that includes all, including a twistd plugin.

Also attached a test script that depends on wokkel, but shows how to do internal connects to the router. I intend to put the dependencies of that script and the new classes defined there in Twisted, too.

Changed 6 years ago by ralphm

Patch bomb resulting in a XMPP router plugin for twistd.

Changed 6 years ago by ralphm

Test script for XMPP router.

comment:5 Changed 6 years ago by oubiwann

  • author set to oubiwann
  • Branch set to branches/xmpp-router-3407

(In [24830]) Branching to 'xmpp-router-3407'

comment:6 Changed 6 years ago by exarkun

  • Cc exarkun added

The patch doesn't include xmpproutertap.py

comment:7 Changed 6 years ago by ralphm

(In [24851]) Add missing tap file. Re #3407.

comment:8 Changed 6 years ago by ralphm

(In [24852]) Commit missing XmlPipe and tests. Re #3407.

comment:9 Changed 6 years ago by ralphm

I split some changes off to #3435.

comment:10 Changed 6 years ago by twonds

  • Cc twonds added

comment:11 Changed 6 years ago by ralphm, oubiwann

  • author changed from oubiwann to ralphm, oubiwann
  • Branch changed from branches/xmpp-router-3407 to branches/xmpp-router-3407-2

(In [24962]) Branching to 'xmpp-router-3407-2'

comment:12 Changed 6 years ago by ralphm

  • author changed from ralphm, oubiwann to ralphm
  • Cc glyph added
  • Keywords review added
  • Owner ralphm deleted

Now #3435 has been closed, I merged forward. This should be a lot easier to review.

comment:13 follow-up: Changed 6 years ago by therve

  • Keywords review removed
  • Owner set to ralphm

Here we go!

  • the new tap file is not tested
  • ListenComponentAuthenticator is missing docstrings
  • the FIXME in the line self.xmlstream.sid = 'random' makes me uncomfortable. What are the implications of this?
  • the "not-authorized" part of ListenComponentAuthenticator.onElement is not tested
  • same for the self.xmlstream.dispatch in ListenComponentAuthenticator.onHandshake
  • if not list(stanza.elements()): in RouterService.route is not tested
  • ComponentServer.startService is not tested

Note that it may have been easier to create a separate ticket to create the router functionality, and then create the twistd plugin (small branches are great, smaller branches are better).

Everything else looks good, I have to catch up on XMPP to actually understand the code though :).

comment:14 in reply to: ↑ 13 Changed 6 years ago by ralphm

  • Cc therve added
  • Keywords review added

Replying to therve:

Here we go!

Yay!

  • the new tap file is not tested

Fixed.

  • ListenComponentAuthenticator is missing docstrings

Fixed.

  • the FIXME in the line self.xmlstream.sid = 'random' makes me uncomfortable. What are the implications of this?

I changed the base class ListenAuthenticator to generate the stream ID, since it really belongs there, using twisted.python.randbytes.

  • the "not-authorized" part of ListenComponentAuthenticator.onElement is not tested

Fixed. This actually revealed a bug. Thanks.

  • same for the self.xmlstream.dispatch in ListenComponentAuthenticator.onHandshake

Fixed.

  • if not list(stanza.elements()): in RouterService.route is not tested

This really shouldn't have been in there, so I removed that check entirely.

  • ComponentServer.startService is not tested

This shouldn't have been a service to begin with, so I made ComponentServer into a XMPPComponentServerFactory that then should be used with TCPServer to listen for connections. I also changed RouterService into Router which is no longer a service.

Note that it may have been easier to create a separate ticket to create the router functionality, and then create the twistd plugin (small branches are great, smaller branches are better).

Actually I have all this stuff in 6 Mercurial Queues patches, but since most of them are interdepenent, that makes getting them all reviewed maybe even take longer? I haven't figured out the best strategy here :-)

Everything else looks good, I have to catch up on XMPP to actually understand the code though :).

Heh. Please review again.

comment:15 Changed 6 years ago by ralphm

  • Owner ralphm deleted

comment:16 follow-up: Changed 6 years ago by therve

  • Keywords review removed
  • Owner set to ralphm

OK, that looks really good. Still a few comments:

  • the method names of XMPPComponentServerFactory are not really clear. makeConnection is used on protocol in Twisted, so another name would be welcome. connectionInitialized is not used, but it looks familiar. Maybe put a prefix/suffix to that, to make clear that they are methods called upon XMPP events?
  • is XMLPipe intended to be used outside tests? It's not really clear for now, so if it's only to be used in tests, please make it explicit in the docstring.
  • instead of passing a numeric port in the tap file, you could used strports.service (the main benefit being the opportunity to set the listening interface).

That's it!

comment:17 Changed 6 years ago by ralphm

(In [25133]) Address review comments.

Re #3407.

comment:18 in reply to: ↑ 16 Changed 6 years ago by ralphm

  • Keywords review added
  • Owner ralphm deleted

Replying to therve:

OK, that looks really good. Still a few comments:

  • the method names of XMPPComponentServerFactory are not really clear. makeConnection is used on protocol in Twisted, so another name would be welcome. connectionInitialized is not used, but it looks familiar. Maybe put a prefix/suffix to that, to make clear that they are methods called upon XMPP events?

The names were chosen to coincide with the method names of an XMPPHandler, but you have a point. I changed the method names and made them start with on, as is done in other parts of the Jabber related code in Twisted.

  • is XMLPipe intended to be used outside tests? It's not really clear for now, so if it's only to be used in tests, please make it explicit in the docstring.

No, it is meant as a generically usable thing for connecting two things that want to exchange XML Stanzas without actually establishing a full blown connection and XmlStream. That's why I put it in t.w.xish.utility. One use case is tests, but I also use it for connecting in-process parts of a Jabber server to the router. These uses are not yet in Twisted, but can be found in the Wokkel code base. Examples are server-side components and the server-to-server code that I am working on.

  • instead of passing a numeric port in the tap file, you could used strports.service (the main benefit being the opportunity to set the listening interface).

Yeah, I wondered about that, so thanks for suggesting this. I made the default to only listen on localhost because this is a typical configuration for Jabber servers. You in general don't want components to be able to connect from anywhere, without ensuring a properly firewalled environment.

That's it!

Thanks!

Please review.

comment:19 follow-up: Changed 6 years ago by therve

  • Keywords review removed
  • Owner set to ralphm

Last thing: please add '@since: 8.2' in the docstrings of the new classes (I think Router, XMPPComponentServerFactory, and XmlPipe).

Oh, and the description of the xmpp router command says 'An XMPP Router server', it should be 'A XMPP'.

Then you can merge.

comment:20 in reply to: ↑ 19 Changed 6 years ago by ralphm

Replying to therve:

Last thing: please add '@since: 8.2' in the docstrings of the new classes (I think Router, XMPPComponentServerFactory, and XmlPipe).

Done.

Oh, and the description of the xmpp router command says 'An XMPP Router server', it should be 'A XMPP'.

XMPP is usually pronounced 'axe-em-pee-pee' and as such takes 'an' as the indefinite article.

Then you can merge.

Thanks!

comment:21 Changed 6 years ago by ralphm

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

(In [25269]) Add XMPP router functionality and twistd plugin 'xmpp-router'.

Author: ralphm.
Reviewer: therve.
Fixes #3407.

An XMPP router connects parts of a Jabber server known as 'components' and
routes XML Stanzas between components based on addressing on stanzas.
Components usually connect over a TCP socket, so this change adds a factory to
listen for incoming component connections that are tied to the router. Using
the new XmlPipe, components can also be connected to the router within the same
process without using a socket. Finally, a random stream ID is generated for
all incoming XML Streams.

The new xmpp-router twistd plugin allows for easy deployment of a stand-alone
XMPP router.

comment:22 Changed 6 years ago by ralphm

  • Resolution fixed deleted
  • Status changed from closed to reopened

(In [25270]) Revert r25269, api-documentation errors.

Reopens #3407.

comment:23 Changed 6 years ago by ralphm

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

(In [25272]) Add XMPP router functionality and twistd plugin 'xmpp-router'.

Author: ralphm.
Reviewer: therve.
Fixes #3407.

An XMPP router connects parts of a Jabber server known as 'components' and
routes XML Stanzas between components based on addressing on stanzas.
Components usually connect over a TCP socket, so this change adds a factory to
listen for incoming component connections that are tied to the router. Using
the new XmlPipe, components can also be connected to the router within the same
process without using a socket. Finally, a random stream ID is generated for
all incoming XML Streams.

The new xmpp-router twistd plugin allows for easy deployment of a stand-alone
XMPP router.

comment:24 Changed 3 years ago by <automation>

  • Owner ralphm deleted
Note: See TracTickets for help on using tickets.