Merge pull request #95 from SuperTux88/fix-invalid-signatures-for-rel…

…ayables

Fix relayable signatures for messages with invalid XML characters

lib/diaspora_federation/entity.rb

@@ -277,7 +277,7 @@ def normalize_property(name, value)
 
       case self.class.class_props[name]
       when :string
-        value.to_s
+        value.to_s.gsub(INVALID_XML_REGEX, "\uFFFD")
       when :timestamp
         value.nil? ? "" : value.utc.iso8601
       else
@@ -310,7 +310,7 @@ def add_property_to_xml(doc, root_element, name, value)
     # Create simple node, fill it with text and append to root
     def simple_node(doc, name, value)
       Nokogiri::XML::Element.new(name.to_s, doc).tap do |node|
-        node.content = value.gsub(INVALID_XML_REGEX, "\uFFFD") unless value.empty?
+        node.content = value unless value.empty?
       end
     end
 

spec/lib/diaspora_federation/entities/relayable_spec.rb

@@ -208,6 +208,22 @@ module DiasporaFederation
         expect(verify_signature(parent_pkey, parent_author_signature, signature_data)).to be_truthy
       end
 
+      it "computes correct signatures for the entity with invalid XML characters" do
+        expect_callback(:fetch_private_key, author).and_return(author_pkey)
+        expect_callback(:fetch_private_key, local_parent.author).and_return(parent_pkey)
+
+        invalid_property = "asdfasdf asdf💩asdf\nasdf"
+        signature_data_with_fixed_property = "#{author};#{guid};#{parent_guid};asdf�asdf asdf💩asdf\nasdf"
+
+        xml = Entities::SomeRelayable.new(hash.merge(property: invalid_property)).to_xml
+
+        author_signature = xml.at_xpath("author_signature").text
+        parent_author_signature = xml.at_xpath("parent_author_signature").text
+
+        expect(verify_signature(author_pkey, author_signature, signature_data_with_fixed_property)).to be_truthy
+        expect(verify_signature(parent_pkey, parent_author_signature, signature_data_with_fixed_property)).to be_truthy
+      end
+
       it "computes correct signatures for the entity when the parent is a relayable itself" do
         intermediate_author = Fabricate.sequence(:diaspora_id)
         parent = Fabricate(:related_entity, author: intermediate_author, local: true, parent: local_parent)