Merge pull request #41 from jaywink/drop-urlencoding

Fixes to Diaspora protocol handling

CHANGELOG.md

@@ -1,3 +1,11 @@
+## [unreleased]
+
+## Fixes
+
+- Don't quote/encode `Protocol.build_send` payload. It was doing it wrongly in the first place and also it's not necessary since Diaspora 0.6 protocol changes. [#41](https://github.com/jaywink/social-federation/pull/41)
+- Fix identification of Diaspora protocol messages. This was not working in the case that the attributes in the tag were in different order. [#41](https://github.com/jaywink/social-federation/pull/41)
+
+
 ## [0.4.0] - 2016-07-24
 
 ## Breaking changes

federation/protocols/diaspora/protocol.py

@@ -2,7 +2,7 @@
 import logging
 from base64 import b64decode, urlsafe_b64decode, b64encode, urlsafe_b64encode
 from json import loads, dumps
-from urllib.parse import unquote_plus, quote_plus, urlencode
+from urllib.parse import unquote_plus
 
 from Crypto.Cipher import AES, PKCS1_v1_5
 from Crypto.Hash import SHA256
@@ -23,7 +23,7 @@ PROTOCOL_NS = "https://joindiaspora.com/protocol"
 def identify_payload(payload):
     try:
         xml = unquote_plus(payload)
-        return xml.find('<diaspora xmlns="%s"' % PROTOCOL_NS) > -1
+        return xml.find('xmlns="%s"' % PROTOCOL_NS) > -1
     except Exception:
         return False
 
@@ -186,12 +186,8 @@ class Protocol(BaseProtocol):
         """Build POST data for sending out to remotes."""
         xml = entity.to_xml()
         self.init_message(xml, from_user.handle, from_user.private_key)
-        xml = quote_plus(
-            self.create_salmon_envelope(to_user.key))
-        data = urlencode({
-            'xml': xml
-        })
-        return data
+        xml = self.create_salmon_envelope(to_user.key)
+        return {'xml': xml}
 
     def init_message(self, message, author_username, private_key):
         """

federation/tests/protocols/diaspora/test_diaspora.py

@@ -1,6 +1,6 @@
 # -*- coding: utf-8 -*-
 from base64 import urlsafe_b64decode
-from unittest.mock import Mock
+from unittest.mock import Mock, patch
 from xml.etree.ElementTree import ElementTree
 
 from lxml import etree
@@ -8,6 +8,7 @@ import pytest
 
 from federation.exceptions import EncryptedMessageError, NoSenderKeyFoundError, NoHeaderInMessageError
 from federation.protocols.diaspora.protocol import Protocol, identify_payload
+from federation.tests.factories.entities import DiasporaPostFactory
 from federation.tests.fixtures.payloads import ENCRYPTED_DIASPORA_PAYLOAD, UNENCRYPTED_DIASPORA_PAYLOAD
 
 
@@ -140,3 +141,15 @@ class TestDiasporaProtocol(DiasporaTestBase):
         protocol.header = ElementTree()
         protocol.content = "<content><handle>bob@example.com</handle></content>"
         assert protocol.get_sender() == None
+
+    @patch.object(Protocol, "init_message")
+    @patch.object(Protocol, "create_salmon_envelope")
+    def test_build_send(self, mock_create_salmon, mock_init_message):
+        mock_create_salmon.return_value = "xmldata"
+        protocol = self.init_protocol()
+        mock_entity_xml = Mock()
+        entity = Mock(to_xml=Mock(return_value=mock_entity_xml))
+        from_user = Mock(handle="foobar", private_key="barfoo")
+        data = protocol.build_send(from_user, Mock(), entity)
+        mock_init_message.assert_called_once_with(mock_entity_xml, from_user.handle, from_user.private_key)
+        assert data == {"xml": "xmldata"}

federation/tests/test_outbound.py

@@ -8,16 +8,15 @@ from federation.outbound import handle_create_payload
 
 
 class TestHandleCreatePayloadBuildsAPayload(object):
-    def test_handle_create_payload_builds_an_xml(self):
-        from_user = Mock(private_key=RSA.generate(2048), handle="foobar@domain.tld")
-        to_user = Mock(key=RSA.generate(2048).publickey())
+    @patch("federation.outbound.Protocol")
+    def test_handle_create_payload_builds_an_xml(self, mock_protocol_class):
+        mock_protocol = Mock()
+        mock_protocol_class.return_value = mock_protocol
+        from_user = Mock()
+        to_user = Mock()
         entity = DiasporaPost()
-        data = handle_create_payload(from_user, to_user, entity)
-        assert len(data) > 0
-        parts = data.split("=")
-        assert len(parts) == 2
-        assert parts[0] == "xml"
-        assert len(parts[1]) > 0
+        handle_create_payload(from_user, to_user, entity)
+        mock_protocol.build_send.assert_called_once_with(from_user=from_user, to_user=to_user, entity=entity)
 
     @patch("federation.outbound.get_outbound_entity")
     def test_handle_create_payload_calls_get_outbound_entity(self, mock_get_outbound_entity):