# HG changeset patch # User Ralph Meijer # Date 1157546327 0 # Node ID 6fe78048baf985aed839a0944acb6214e8b39cc1 # Parent b2149e448465d54e39bb8892ba9ded3313510738 Rework error handling, depend on Twisted Words 0.4.0. Twisted Words 0.4.0 introduced support for stanza error handling, much better than the custom error handling in Idavoll. Also, all protocol-level errors were examined and brought up to date with version 1.8 of JEP-0060. As a result of the error examination, the retrieval of default configuration options using is now supported properly. diff -r b2149e448465 -r 6fe78048baf9 INSTALL --- a/INSTALL Thu Jun 18 11:52:01 2009 +0000 +++ b/INSTALL Wed Sep 06 12:38:47 2006 +0000 @@ -2,7 +2,7 @@ ============ - Twisted Core >= 2.0.0 -- Twisted Words >= 0.3.0 +- Twisted Words >= 0.4.0 - uuid.py (http://ofxsuite.berlios.de/uuid.py) - A jabber server that supports the component protocol (JEP-0114) diff -r b2149e448465 -r 6fe78048baf9 idavoll/backend.py --- a/idavoll/backend.py Thu Jun 18 11:52:01 2009 +0000 +++ b/idavoll/backend.py Wed Sep 06 12:38:47 2006 +0000 @@ -10,21 +10,18 @@ def __str__(self): return self.msg -class NotAuthorized(Error): +class Forbidden(Error): pass -class PayloadExpected(Error): - msg = 'Payload expected' +class ItemForbidden(Error): + pass -class NoPayloadAllowed(Error): - msg = 'No payload allowed' +class ItemRequired(Error): + pass class NoInstantNodes(Error): pass -class NotImplemented(Error): - pass - class NotSubscribed(Error): pass @@ -34,6 +31,12 @@ class InvalidConfigurationValue(Error): msg = 'Bad configuration value' +class NodeNotPersistent(Error): + pass + +class NoRootNode(Error): + pass + class IBackendService(Interface): """ Interface to a backend service of a pubsub service. """ diff -r b2149e448465 -r 6fe78048baf9 idavoll/generic_backend.py --- a/idavoll/generic_backend.py Thu Jun 18 11:52:01 2009 +0000 +++ b/idavoll/generic_backend.py Wed Sep 06 12:38:47 2006 +0000 @@ -78,7 +78,7 @@ def _check_auth(self, node, requestor): def check(affiliation, node): if affiliation not in ['owner', 'publisher']: - raise backend.NotAuthorized + raise backend.Forbidden return node d = node.get_affiliation(requestor) @@ -97,9 +97,9 @@ deliver_payloads = configuration["pubsub#deliver_payloads"] if items and not persist_items and not deliver_payloads: - raise backend.NoPayloadAllowed + raise backend.ItemForbidden elif not items and (persist_items or deliver_payloads): - raise backend.PayloadExpected + raise backend.ItemRequired if persist_items or deliver_payloads: for item in items: @@ -149,7 +149,7 @@ def subscribe(self, node_id, subscriber, requestor): subscriber_entity = subscriber.userhostJID() if subscriber_entity != requestor: - return defer.fail(backend.NotAuthorized()) + return defer.fail(backend.Forbidden()) d = self.parent.storage.get_node(node_id) d.addCallback(_get_affiliation, subscriber_entity) @@ -160,7 +160,7 @@ node, affiliation = result if affiliation == 'outcast': - raise backend.NotAuthorized + raise backend.Forbidden d = node.add_subscription(subscriber, 'subscribed') d.addCallback(lambda _: 'subscribed') @@ -177,7 +177,7 @@ def unsubscribe(self, node_id, subscriber, requestor): if subscriber.userhostJID() != requestor: - raise backend.NotAuthorized + return defer.fail(backend.Forbidden()) d = self.parent.storage.get_node(node_id) d.addCallback(lambda node: node.remove_subscription(subscriber)) @@ -200,13 +200,17 @@ d.addCallback(lambda _: node_id) return d + def get_default_configuration(self): + d = defer.succeed(self.parent.default_config) + d.addCallback(self._make_config) + return d + def get_node_configuration(self, node_id): - if node_id: - d = self.parent.storage.get_node(node_id) - d.addCallback(lambda node: node.get_configuration()) - else: - # XXX: this is disabled in pubsub.py - d = defer.succeed(self.parent.default_config) + if not node_id: + raise backend.NoRootNode + + d = self.parent.storage.get_node(node_id) + d.addCallback(lambda node: node.get_configuration()) d.addCallback(self._make_config) return d @@ -223,6 +227,9 @@ return options def set_node_configuration(self, node_id, options, requestor): + if not node_id: + raise backend.NoRootNode + for key, value in options.iteritems(): if not self.parent.options.has_key(key): raise backend.InvalidConfigurationOption @@ -241,7 +248,7 @@ node, affiliation = result if affiliation != 'owner': - raise backend.NotAuthorized + raise backend.Forbidden return node.set_configuration(options) @@ -271,7 +278,7 @@ node, subscribed = result if not subscribed: - raise backend.NotAuthorized + raise backend.NotSubscribed if item_ids: return node.get_items_by_id(item_ids) @@ -293,7 +300,7 @@ persist_items = node.get_configuration()["pubsub#persist_items"] if affiliation not in ['owner', 'publisher']: - raise backend.NotAuthorized + raise backend.Forbidden if not persist_items: raise backend.NodeNotPersistent @@ -317,7 +324,7 @@ persist_items = node.get_configuration()["pubsub#persist_items"] if affiliation != 'owner': - raise backend.NotAuthorized + raise backend.Forbidden if not persist_items: raise backend.NodeNotPersistent @@ -354,7 +361,7 @@ node, affiliation = result if affiliation != 'owner': - raise backend.NotAuthorized + raise backend.Forbidden d = defer.DeferredList([cb(node.id) for cb in self._callback_list], consumeErrors=1) diff -r b2149e448465 -r 6fe78048baf9 idavoll/idavoll.py --- a/idavoll/idavoll.py Thu Jun 18 11:52:01 2009 +0000 +++ b/idavoll/idavoll.py Wed Sep 06 12:38:47 2006 +0000 @@ -1,12 +1,11 @@ # Copyright (c) 2003-2006 Ralph Meijer # See LICENSE for details. -from twisted.words.protocols.jabber import component +from twisted.words.protocols.jabber import component, error from twisted.application import service from twisted.internet import defer import backend import pubsub -import xmpp_error import disco import sys @@ -69,7 +68,7 @@ info.extend(i[1]) if node and not info: - return xmpp_error.error_from_iq(iq, 'item-not-found') + return error.StanzaError('item-not-found').toResponse(iq) else: iq.swapAttributeValues("to", "from") iq["type"] = "result" @@ -85,7 +84,7 @@ def _error(self, result, iq): print "Got error on index %d:" % result.value[1] result.value[0].printBriefTraceback() - return xmpp_error.error_from_iq(iq, 'internal-server-error') + return error.StanzaError('internal-server-error').toResponse(iq) def onDiscoItems(self, iq): dl = [] @@ -118,7 +117,7 @@ if iq.handled == True: return - self.send(xmpp_error.error_from_iq(iq, 'service-unavailable')) + self.send(error.StanzaError('service-unavailable').toResponse(iq)) class LogService(component.Service): diff -r b2149e448465 -r 6fe78048baf9 idavoll/implementation_issues.txt --- a/idavoll/implementation_issues.txt Thu Jun 18 11:52:01 2009 +0000 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,49 +0,0 @@ -- What happens when a node creation request is accompanied by a configure - node, but node configuration is not implemented? - - Solution: - - Reply with a not-acceptable stanza error, with a node-not-configurable - error attached. - -- Node purging with many items in storage and notification of retraction enabled -could produce a large number of notifications. Suggestion: instead send a - - -- Examples 58 and 62 are the same - -- What error to return for outcast user requests? - - What if some admin requested it? - -- Why not return forbidden when JIDs do not match on subscription? - -- What to do when receiving an unsubscription request with a subid, when - subids are not supported? Ignore or reply with bad-request? - -- unsubscription: why check for the requestor being subscribed. You want to - check if the jid is subscribed. An not-authorized can be sent if the - requestor and jid don't match. Otherwise, unexpected-request might - be appropriate when the jid is not subscribed along with a special - child element of not-subscribed in the pubsub#error namespace. - -- 8.1.7 states: - If subscription identifiers are supported by the service and the entity's - subscription is not NONE or OUTCAST, the 'subid' attribute MUST be present - as well as the 'jid' attribute. - There is no such thing as a subscription of 'OUTCAST'. Also, I'm not sure - why NONE is capitalized. - -- On meta-data: - http://mail.jabber.org/pipermail/standards-jig/2004-July/005783.html - Assumed meta-data as per JEP-0128 is indeed optional, advertised with the - pubsub#meta-data disco feature. Also, how do you actually set this meta-data - information? - -- on items attribute max_items. xs:string?? - -- no disco feature for retracting items, purging nodes, deleting nodes, other - owner use cases? - -- no pubsub#error for bad node configuration options? - -- configure in pubsub#owner namespace?? diff -r b2149e448465 -r 6fe78048baf9 idavoll/pubsub.py --- a/idavoll/pubsub.py Thu Jun 18 11:52:01 2009 +0000 +++ b/idavoll/pubsub.py Wed Sep 06 12:38:47 2006 +0000 @@ -1,7 +1,7 @@ # Copyright (c) 2003-2006 Ralph Meijer # See LICENSE for details. -from twisted.words.protocols.jabber import component,jid +from twisted.words.protocols.jabber import component, jid, error from twisted.words.xish import utility, domish from twisted.python import components from twisted.internet import defer @@ -9,7 +9,6 @@ import backend import storage -import xmpp_error import disco import data_form @@ -40,6 +39,7 @@ PUBSUB_UNSUBSCRIBE = PUBSUB_SET + '/unsubscribe' PUBSUB_OPTIONS_GET = PUBSUB_GET + '/options' PUBSUB_OPTIONS_SET = PUBSUB_SET + '/options' +PUBSUB_DEFAULT = PUBSUB_OWNER_GET + '/default' PUBSUB_CONFIGURE_GET = PUBSUB_OWNER_GET + '/configure' PUBSUB_CONFIGURE_SET = PUBSUB_OWNER_SET + '/configure' PUBSUB_SUBSCRIPTIONS = PUBSUB_GET + '/subscriptions' @@ -49,37 +49,39 @@ PUBSUB_PURGE = PUBSUB_OWNER_SET + '/purge' PUBSUB_DELETE = PUBSUB_OWNER_SET + '/delete' -class Error(Exception): - pubsub_error = None - stanza_error = None - msg = '' - -class NotImplemented(Error): - stanza_error = 'feature-not-implemented' +class BadRequest(error.StanzaError): + def __init__(self): + error.StanzaError.__init__(self, 'bad-request') -class BadRequest(Error): - stanza_error = 'bad-request' +class PubSubError(error.StanzaError): + def __init__(self, condition, pubsubCondition, feature=None, text=None): + appCondition = domish.Element((NS_PUBSUB_ERRORS, pubsubCondition)) + if feature: + appCondition['feature'] = feature + error.StanzaError.__init__(self, condition, + text=text, + appCondition=appCondition) -class OptionsUnavailable(Error): - stanza_error = 'feature-not-implemented' - pubsub_error = 'subscription-options-unavailable' - -class NodeNotConfigurable(Error): - stanza_error = 'feature-not-implemented' - pubsub_error = 'node-not-configurable' +class OptionsUnavailable(PubSubError): + def __init__(self): + PubSubError.__init__(self, 'feature-not-implemented', + 'unsupported', + 'subscription-options-unavailable') error_map = { - storage.NodeNotFound: ('item-not-found', None), - storage.NodeExists: ('conflict', None), - storage.SubscriptionNotFound: ('not-authorized', - 'not-subscribed'), - backend.NotAuthorized: ('not-authorized', None), - backend.NoPayloadAllowed: ('bad-request', None), - backend.PayloadExpected: ('bad-request', None), - backend.NoInstantNodes: ('not-acceptable', None), - backend.NotImplemented: ('feature-not-implemented', None), - backend.InvalidConfigurationOption: ('not-acceptable', None), - backend.InvalidConfigurationValue: ('not-acceptable', None), + storage.NodeNotFound: ('item-not-found', None, None), + storage.NodeExists: ('conflict', None, None), + storage.SubscriptionNotFound: ('not-authorized', 'not-subscribed', None), + backend.Forbidden: ('forbidden', None, None), + backend.ItemForbidden: ('bad-request', 'item-forbidden', None), + backend.ItemRequired: ('bad-request', 'item-required', None), + backend.NoInstantNodes: ('not-acceptable', 'unsupported', 'instant-nodes'), + backend.NotSubscribed: ('not-authorized', 'not-subscribed', None), + backend.InvalidConfigurationOption: ('not-acceptable', None, None), + backend.InvalidConfigurationValue: ('not-acceptable', None, None), + backend.NodeNotPersistent: ('feature-not-implemented', 'unsupported', + 'persistent-node'), + backend.NoRootNode: ('bad-request', None, None), } class Service(component.Service): @@ -91,24 +93,23 @@ def error(self, failure, iq): try: - e = failure.trap(Error, *error_map.keys()) + e = failure.trap(error.StanzaError, *error_map.keys()) except: failure.printBriefTraceback() - xmpp_error.error_from_iq(iq, 'internal-server-error') - return iq + return error.StanzaError('internal-server-error').toResponse(iq) else: - if e == Error: - stanza_error = failure.value.stanza_error - pubsub_error = failure.value.pubsub_error - msg = '' + if e == error.StanzaError: + exc = failure.value else: - stanza_error, pubsub_error = error_map[e] + condition, pubsubCondition, feature = error_map[e] msg = failure.value.msg - xmpp_error.error_from_iq(iq, stanza_error, msg) - if pubsub_error: - iq.error.addElement((NS_PUBSUB_ERRORS, pubsub_error)) - return iq + if pubsubCondition: + exc = PubSubError(condition, pubsubCondition, feature, msg) + else: + exc = error.StanzaError(condition, text=msg) + + return exc.toResponse(iq) def success(self, result, iq): iq.swapAttributeValues("to", "from") @@ -362,6 +363,7 @@ def componentConnected(self, xmlstream): xmlstream.addObserver(PUBSUB_CREATE, self.onCreate) + xmlstream.addObserver(PUBSUB_DEFAULT, self.onDefault) xmlstream.addObserver(PUBSUB_CONFIGURE_GET, self.onConfigureGet) xmlstream.addObserver(PUBSUB_CONFIGURE_SET, self.onConfigureSet) @@ -371,6 +373,7 @@ if not node: info.append(disco.Feature(NS_PUBSUB + "#create-nodes")) info.append(disco.Feature(NS_PUBSUB + "#config-node")) + info.append(disco.Feature(NS_PUBSUB + "#retrieve-default")) if self.backend.supports_instant_nodes(): info.append(disco.Feature(NS_PUBSUB + "#instant-nodes")) @@ -378,6 +381,7 @@ return defer.succeed(info) def onCreate(self, iq): + print "onCreate" self.handler_wrapper(self._onCreate, iq) def _onCreate(self, iq): @@ -397,14 +401,26 @@ entity['node'] = result return [reply] + def onDefault(self, iq): + self.handler_wrapper(self._onDefault, iq) + + def _onDefault(self, iq): + d = self.backend.get_default_configuration() + d.addCallback(self._return_default_response) + return d + + def _return_default_response(self, options): + reply = domish.Element((NS_PUBSUB_OWNER, "pubsub")) + default = reply.addElement("default") + default.addChild(self._form_from_configuration(options)) + + return [reply] + def onConfigureGet(self, iq): self.handler_wrapper(self._onConfigureGet, iq) def _onConfigureGet(self, iq): - try: - node_id = iq.pubsub.configure["node"] - except KeyError: - raise NodeNotConfigurable + node_id = iq.pubsub.configure.getAttribute("node") d = self.backend.get_node_configuration(node_id) d.addCallback(self._return_configuration_response, node_id) @@ -415,26 +431,25 @@ configure = reply.addElement("configure") if node_id: configure["node"] = node_id + configure.addChild(self._form_from_configuration(options)) + + return [reply] + + def _form_from_configuration(self, options): form = data_form.Form(type="form", form_type=NS_PUBSUB + "#node_config") for option in options: form.add_field(**option) - form.parent = configure - configure.addChild(form) - - return [reply] + return form def onConfigureSet(self, iq): + print "onConfigureSet" self.handler_wrapper(self._onConfigureSet, iq) def _onConfigureSet(self, iq): - try: - node_id = iq.pubsub.configure["node"] - except KeyError: - raise BadRequest - + node_id = iq.pubsub.configure["node"] requestor = jid.internJID(iq["from"]).userhostJID() for element in iq.pubsub.configure.elements(): diff -r b2149e448465 -r 6fe78048baf9 idavoll/xmpp_error.py --- a/idavoll/xmpp_error.py Thu Jun 18 11:52:01 2009 +0000 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,35 +0,0 @@ -# Copyright (c) 2003-2006 Ralph Meijer -# See LICENSE for details. - -NS_XMPP_STANZAS = "urn:ietf:params:xml:ns:xmpp-stanzas" - -conditions = { - 'bad-request': {'code': '400', 'type': 'modify'}, - 'not-authorized': {'code': '401', 'type': 'cancel'}, - 'item-not-found': {'code': '404', 'type': 'cancel'}, - 'not-acceptable': {'code': '406', 'type': 'modify'}, - 'conflict': {'code': '409', 'type': 'cancel'}, - 'internal-server-error': {'code': '500', 'type': 'wait'}, - 'feature-not-implemented': {'code': '501', 'type': 'cancel'}, - 'service-unavailable': {'code': '503', 'type': 'cancel'}, -} - -def error_from_iq(iq, condition, text = '', type = None): - iq.swapAttributeValues("to", "from") - iq["type"] = 'error' - e = iq.addElement("error") - - c = e.addElement((NS_XMPP_STANZAS, condition), NS_XMPP_STANZAS) - - if type == None: - type = conditions[condition]['type'] - - code = conditions[condition]['code'] - - e["code"] = code - e["type"] = type - - if text: - t = e.addElement((NS_XMPP_STANZAS, "text"), NS_XMPP_STANZAS, text) - - return iq