From bb40d6190d4ed05126ac1b8260f10edd25c1bb99 Mon Sep 17 00:00:00 2001
From: Benjamin Neff <benjamin@coding4coffee.ch>
Date: Sat, 29 Jul 2017 22:26:10 +0200
Subject: [PATCH] Only add optional nil values to relayable XML when needed for
 signature order

---
 lib/diaspora_federation/entities/relayable.rb |  4 +-
 spec/entities.rb                              |  2 +-
 .../entities/relayable_spec.rb                | 37 +++++++++++++++++++
 3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/lib/diaspora_federation/entities/relayable.rb b/lib/diaspora_federation/entities/relayable.rb
index 5397258..a61a467 100644
--- a/lib/diaspora_federation/entities/relayable.rb
+++ b/lib/diaspora_federation/entities/relayable.rb
@@ -100,7 +100,9 @@ module DiasporaFederation
       # The order for signing
       # @return [Array]
       def signature_order
-        @signature_order || self.class.class_props.keys - %i[author_signature parent_author_signature parent]
+        @signature_order || self.class.class_props.keys.reject {|key|
+          self.class.optional_props.include?(key) && public_send(key).nil?
+        } - %i[author_signature parent_author_signature parent]
       end
 
       private
diff --git a/spec/entities.rb b/spec/entities.rb
index 846c4cd..5576939 100644
--- a/spec/entities.rb
+++ b/spec/entities.rb
@@ -74,7 +74,7 @@ module DiasporaFederation
 
       include Entities::Relayable
 
-      property :property, :string
+      property :property, :string, optional: true
     end
   end
 
diff --git a/spec/lib/diaspora_federation/entities/relayable_spec.rb b/spec/lib/diaspora_federation/entities/relayable_spec.rb
index b237fcf..bbac670 100644
--- a/spec/lib/diaspora_federation/entities/relayable_spec.rb
+++ b/spec/lib/diaspora_federation/entities/relayable_spec.rb
@@ -148,6 +148,43 @@ XML
         expect(xml.at_xpath("new_property").text).to be_empty
       end
 
+      it "adds nil properties to xml when needed for signature_order" do
+        expected_xml = <<-XML
+<some_relayable>
+  <author>#{author}</author>
+  <guid>#{guid}</guid>
+  <parent_guid>#{parent_guid}</parent_guid>
+  <property/>
+  <new_property>#{new_property}</new_property>
+  <author_signature>aa</author_signature>
+  <parent_author_signature>bb</parent_author_signature>
+</some_relayable>
+XML
+
+        signature_order = [:author, :guid, :parent_guid, :property, "new_property"]
+        xml = Entities::SomeRelayable.new(
+          hash_with_fake_signatures.merge(property: nil), signature_order, "new_property" => new_property
+        ).to_xml
+
+        expect(xml.to_s.strip).to eq(expected_xml.strip)
+      end
+
+      it "does not add nil properties to xml when not needed for signature_order" do
+        expected_xml = <<-XML
+<some_relayable>
+  <author>#{author}</author>
+  <guid>#{guid}</guid>
+  <parent_guid>#{parent_guid}</parent_guid>
+  <author_signature>aa</author_signature>
+  <parent_author_signature>bb</parent_author_signature>
+</some_relayable>
+XML
+
+        xml = Entities::SomeRelayable.new(hash_with_fake_signatures.merge(property: nil)).to_xml
+
+        expect(xml.to_s.strip).to eq(expected_xml.strip)
+      end
+
       it "computes correct signatures for the entity" do
         expect_callback(:fetch_private_key, author).and_return(author_pkey)
         expect_callback(:fetch_private_key, local_parent.author).and_return(parent_pkey)