From 92dc8b027770f0b9056c6b489f2e7e869e90cc9a Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 12 Sep 2017 21:24:50 +0200 Subject: [PATCH 1/5] Fix GUID regex --- lib/diaspora_federation/validators/rules/guid.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/diaspora_federation/validators/rules/guid.rb b/lib/diaspora_federation/validators/rules/guid.rb index 88ab8e6..3e30874 100644 --- a/lib/diaspora_federation/validators/rules/guid.rb +++ b/lib/diaspora_federation/validators/rules/guid.rb @@ -9,7 +9,7 @@ module Validation # Special chars aren't allowed at the end. class Guid # Allowed chars to validate a GUID with a regex - VALID_CHARS = "[0-9A-Za-z\\-_@.:]{15,254}[0-9a-z]".freeze + VALID_CHARS = "[0-9A-Za-z\\-_@.:]{15,254}[0-9A-Za-z]".freeze # The error key for this rule # @return [Symbol] error key From 5e3f510a880882236623f54fc4c15f70a16d6958 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 12 Sep 2017 21:52:45 +0200 Subject: [PATCH 2/5] Refactor diaspora ID regex to be used in diaspora:// URL regex --- .../validators/rules/diaspora_id.rb | 14 ++++++++------ .../validators/rules/diaspora_id_spec.rb | 6 +++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/diaspora_federation/validators/rules/diaspora_id.rb b/lib/diaspora_federation/validators/rules/diaspora_id.rb index 001face..944a9b0 100644 --- a/lib/diaspora_federation/validators/rules/diaspora_id.rb +++ b/lib/diaspora_federation/validators/rules/diaspora_id.rb @@ -5,23 +5,25 @@ module Validation # A simple rule to validate the base structure of diaspora* IDs. class DiasporaId # The Regex for a valid diaspora* ID - DIASPORA_ID = begin + DIASPORA_ID_REGEX = begin letter = "a-zA-Z" digit = "0-9" hexadecimal = "[a-fA-F#{digit}]" username = "[#{letter}#{digit}\\-\\_\\.]+" hostname_part = "[#{letter}#{digit}\\-]" - hostname = "#{hostname_part}+([.]#{hostname_part}*)*" + hostname = "#{hostname_part}+(?:[.]#{hostname_part}*)*" ipv4 = "(?:[#{digit}]{1,3}\\.){3}[#{digit}]{1,3}" ipv6 = "\\[(?:#{hexadecimal}{0,4}:){0,7}#{hexadecimal}{1,4}\\]" ip_addr = "(?:#{ipv4}|#{ipv6})" domain = "(?:#{hostname}|#{ip_addr})" - port = "(:[#{digit}]+)?" - addr_spec = "(#{username}\\@#{domain}#{port})?" + port = "(?::[#{digit}]+)?" - /\A#{addr_spec}\z/u + "#{username}\\@#{domain}#{port}" end + # The Regex for validating a full diaspora* ID + DIASPORA_ID = /\A#{DIASPORA_ID_REGEX}\z/u + # The error key for this rule # @return [Symbol] error key def error_key @@ -30,7 +32,7 @@ module Validation # Determines if value is a valid diaspora* ID def valid_value?(value) - value.nil? || !DIASPORA_ID.match(value).nil? + value.is_a?(String) && value =~ DIASPORA_ID end # This rule has no params. diff --git a/spec/lib/diaspora_federation/validators/rules/diaspora_id_spec.rb b/spec/lib/diaspora_federation/validators/rules/diaspora_id_spec.rb index 0e9006a..178b190 100644 --- a/spec/lib/diaspora_federation/validators/rules/diaspora_id_spec.rb +++ b/spec/lib/diaspora_federation/validators/rules/diaspora_id_spec.rb @@ -83,13 +83,13 @@ describe Validation::Rule::DiasporaId do expect(validator.errors).to include(:diaspora_id) end - it "allows nil and empty" do + it "fails for nil and empty" do [nil, ""].each do |val| validator = Validation::Validator.new(OpenStruct.new(diaspora_id: val)) validator.rule(:diaspora_id, :diaspora_id) - expect(validator).to be_valid - expect(validator.errors).to be_empty + expect(validator).not_to be_valid + expect(validator.errors).to include(:diaspora_id) end end end From e663a65c7e274af2fdb496154a66d5d6ad2fdd1f Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 12 Sep 2017 22:09:06 +0200 Subject: [PATCH 3/5] Remove not_nil for diaspora IDs from validators The diaspora ID validator now is not nil by default. Also: * mark root_author as optional for reshares. * make author for profiles mandatory (I don't remember why this was optional, we never generate a profile without author and we wouldn't receive a profile without author anyway, because we validate that the author is the sender). * refactor validator specs for diaspora IDs --- lib/diaspora_federation/entities/reshare.rb | 2 +- .../validators/account_deletion_validator.rb | 2 +- .../validators/account_migration_validator.rb | 2 +- .../validators/contact_validator.rb | 4 ++-- .../validators/conversation_validator.rb | 2 +- .../validators/event_validator.rb | 2 +- .../validators/message_validator.rb | 2 +- .../validators/participation_validator.rb | 2 +- .../validators/person_validator.rb | 2 +- .../validators/photo_validator.rb | 2 +- .../validators/related_entity_validator.rb | 2 +- .../validators/relayable_validator.rb | 2 +- .../validators/reshare_validator.rb | 2 +- .../validators/retraction_validator.rb | 2 +- .../validators/status_message_validator.rb | 2 +- .../account_deletion_validator_spec.rb | 1 - .../account_migration_validator_spec.rb | 1 - .../validators/contact_validator_spec.rb | 1 - .../validators/conversation_validator_spec.rb | 1 - .../validators/event_validator_spec.rb | 1 - .../validators/message_validator_spec.rb | 1 - .../validators/participation_validator_spec.rb | 1 - .../validators/person_validator_spec.rb | 1 - .../validators/photo_validator_spec.rb | 1 - .../validators/profile_validator_spec.rb | 1 - .../validators/related_entity_validator_spec.rb | 1 - .../validators/reshare_validator_spec.rb | 6 +++--- .../validators/retraction_validator_spec.rb | 1 - .../validators/status_message_validator_spec.rb | 1 - spec/support/shared_validator_specs.rb | 17 +++++++++-------- 30 files changed, 28 insertions(+), 40 deletions(-) diff --git a/lib/diaspora_federation/entities/reshare.rb b/lib/diaspora_federation/entities/reshare.rb index 96ae593..ff235fa 100644 --- a/lib/diaspora_federation/entities/reshare.rb +++ b/lib/diaspora_federation/entities/reshare.rb @@ -10,7 +10,7 @@ module DiasporaFederation # The diaspora* ID of the person who posted the original post # @see Person#author # @return [String] diaspora* ID - property :root_author, :string, xml_name: :root_diaspora_id + property :root_author, :string, optional: true, xml_name: :root_diaspora_id # @!attribute [r] root_guid # Guid of the original post diff --git a/lib/diaspora_federation/validators/account_deletion_validator.rb b/lib/diaspora_federation/validators/account_deletion_validator.rb index 872472b..60a32cf 100644 --- a/lib/diaspora_federation/validators/account_deletion_validator.rb +++ b/lib/diaspora_federation/validators/account_deletion_validator.rb @@ -4,7 +4,7 @@ module DiasporaFederation class AccountDeletionValidator < OptionalAwareValidator include Validation - rule :author, %i[not_empty diaspora_id] + rule :author, :diaspora_id end end end diff --git a/lib/diaspora_federation/validators/account_migration_validator.rb b/lib/diaspora_federation/validators/account_migration_validator.rb index f0dd336..1c11b25 100644 --- a/lib/diaspora_federation/validators/account_migration_validator.rb +++ b/lib/diaspora_federation/validators/account_migration_validator.rb @@ -4,7 +4,7 @@ module DiasporaFederation class AccountMigrationValidator < OptionalAwareValidator include Validation - rule :author, %i[not_empty diaspora_id] + rule :author, :diaspora_id rule :profile, :not_nil end diff --git a/lib/diaspora_federation/validators/contact_validator.rb b/lib/diaspora_federation/validators/contact_validator.rb index c22ecc7..3c88805 100644 --- a/lib/diaspora_federation/validators/contact_validator.rb +++ b/lib/diaspora_federation/validators/contact_validator.rb @@ -4,8 +4,8 @@ module DiasporaFederation class ContactValidator < OptionalAwareValidator include Validation - rule :author, %i[not_empty diaspora_id] - rule :recipient, %i[not_empty diaspora_id] + rule :author, :diaspora_id + rule :recipient, :diaspora_id rule :following, :boolean rule :sharing, :boolean end diff --git a/lib/diaspora_federation/validators/conversation_validator.rb b/lib/diaspora_federation/validators/conversation_validator.rb index e8c84ee..43e68cb 100644 --- a/lib/diaspora_federation/validators/conversation_validator.rb +++ b/lib/diaspora_federation/validators/conversation_validator.rb @@ -4,7 +4,7 @@ module DiasporaFederation class ConversationValidator < OptionalAwareValidator include Validation - rule :author, %i[not_empty diaspora_id] + rule :author, :diaspora_id rule :guid, :guid rule :subject, [:not_empty, length: {maximum: 255}] diff --git a/lib/diaspora_federation/validators/event_validator.rb b/lib/diaspora_federation/validators/event_validator.rb index ab926a4..a6c0aa6 100644 --- a/lib/diaspora_federation/validators/event_validator.rb +++ b/lib/diaspora_federation/validators/event_validator.rb @@ -4,7 +4,7 @@ module DiasporaFederation class EventValidator < OptionalAwareValidator include Validation - rule :author, %i[not_empty diaspora_id] + rule :author, :diaspora_id rule :guid, :guid diff --git a/lib/diaspora_federation/validators/message_validator.rb b/lib/diaspora_federation/validators/message_validator.rb index 1e757f6..6ded966 100644 --- a/lib/diaspora_federation/validators/message_validator.rb +++ b/lib/diaspora_federation/validators/message_validator.rb @@ -4,7 +4,7 @@ module DiasporaFederation class MessageValidator < OptionalAwareValidator include Validation - rule :author, %i[not_empty diaspora_id] + rule :author, :diaspora_id rule :guid, :guid rule :conversation_guid, :guid diff --git a/lib/diaspora_federation/validators/participation_validator.rb b/lib/diaspora_federation/validators/participation_validator.rb index 13b8c1e..c3c4917 100644 --- a/lib/diaspora_federation/validators/participation_validator.rb +++ b/lib/diaspora_federation/validators/participation_validator.rb @@ -4,7 +4,7 @@ module DiasporaFederation class ParticipationValidator < OptionalAwareValidator include Validation - rule :author, %i[not_empty diaspora_id] + rule :author, :diaspora_id rule :guid, :guid rule :parent_guid, :guid rule :parent_type, [:not_empty, regular_expression: {regex: /\APost\z/}] diff --git a/lib/diaspora_federation/validators/person_validator.rb b/lib/diaspora_federation/validators/person_validator.rb index 62cac9d..ddfad99 100644 --- a/lib/diaspora_federation/validators/person_validator.rb +++ b/lib/diaspora_federation/validators/person_validator.rb @@ -6,7 +6,7 @@ module DiasporaFederation rule :guid, :guid - rule :author, %i[not_empty diaspora_id] + rule :author, :diaspora_id rule :url, %i[not_nil URI] diff --git a/lib/diaspora_federation/validators/photo_validator.rb b/lib/diaspora_federation/validators/photo_validator.rb index 48e0d73..7c18107 100644 --- a/lib/diaspora_federation/validators/photo_validator.rb +++ b/lib/diaspora_federation/validators/photo_validator.rb @@ -6,7 +6,7 @@ module DiasporaFederation rule :guid, :guid - rule :author, %i[not_empty diaspora_id] + rule :author, :diaspora_id rule :public, :boolean diff --git a/lib/diaspora_federation/validators/related_entity_validator.rb b/lib/diaspora_federation/validators/related_entity_validator.rb index 1f2ffe3..17c2726 100644 --- a/lib/diaspora_federation/validators/related_entity_validator.rb +++ b/lib/diaspora_federation/validators/related_entity_validator.rb @@ -4,7 +4,7 @@ module DiasporaFederation class RelatedEntityValidator < Validation::Validator include Validation - rule :author, %i[not_empty diaspora_id] + rule :author, :diaspora_id rule :local, :boolean rule :public, :boolean end diff --git a/lib/diaspora_federation/validators/relayable_validator.rb b/lib/diaspora_federation/validators/relayable_validator.rb index f32b384..618f3ce 100644 --- a/lib/diaspora_federation/validators/relayable_validator.rb +++ b/lib/diaspora_federation/validators/relayable_validator.rb @@ -6,7 +6,7 @@ module DiasporaFederation # @param [Validation::Validator] validator the validator in which it is included def self.included(validator) validator.class_eval do - rule :author, %i[not_empty diaspora_id] + rule :author, :diaspora_id rule :guid, :guid rule :parent_guid, :guid rule :parent, :not_nil diff --git a/lib/diaspora_federation/validators/reshare_validator.rb b/lib/diaspora_federation/validators/reshare_validator.rb index db8858d..b28621d 100644 --- a/lib/diaspora_federation/validators/reshare_validator.rb +++ b/lib/diaspora_federation/validators/reshare_validator.rb @@ -8,7 +8,7 @@ module DiasporaFederation rule :root_guid, :guid - rule :author, %i[not_empty diaspora_id] + rule :author, :diaspora_id rule :guid, :guid diff --git a/lib/diaspora_federation/validators/retraction_validator.rb b/lib/diaspora_federation/validators/retraction_validator.rb index de70bc8..922b44f 100644 --- a/lib/diaspora_federation/validators/retraction_validator.rb +++ b/lib/diaspora_federation/validators/retraction_validator.rb @@ -4,7 +4,7 @@ module DiasporaFederation class RetractionValidator < OptionalAwareValidator include Validation - rule :author, %i[not_empty diaspora_id] + rule :author, :diaspora_id rule :target_guid, :guid rule :target_type, :not_empty diff --git a/lib/diaspora_federation/validators/status_message_validator.rb b/lib/diaspora_federation/validators/status_message_validator.rb index 43a13be..62eee98 100644 --- a/lib/diaspora_federation/validators/status_message_validator.rb +++ b/lib/diaspora_federation/validators/status_message_validator.rb @@ -4,7 +4,7 @@ module DiasporaFederation class StatusMessageValidator < OptionalAwareValidator include Validation - rule :author, %i[not_empty diaspora_id] + rule :author, :diaspora_id rule :guid, :guid diff --git a/spec/lib/diaspora_federation/validators/account_deletion_validator_spec.rb b/spec/lib/diaspora_federation/validators/account_deletion_validator_spec.rb index 7d0b542..2c76af6 100644 --- a/spec/lib/diaspora_federation/validators/account_deletion_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/account_deletion_validator_spec.rb @@ -6,7 +6,6 @@ module DiasporaFederation it_behaves_like "a diaspora* ID validator" do let(:property) { :author } - let(:mandatory) { true } end end end diff --git a/spec/lib/diaspora_federation/validators/account_migration_validator_spec.rb b/spec/lib/diaspora_federation/validators/account_migration_validator_spec.rb index eca0e5d..7d95768 100644 --- a/spec/lib/diaspora_federation/validators/account_migration_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/account_migration_validator_spec.rb @@ -6,7 +6,6 @@ module DiasporaFederation it_behaves_like "a diaspora* ID validator" do let(:property) { :author } - let(:mandatory) { true } end describe "#person" do diff --git a/spec/lib/diaspora_federation/validators/contact_validator_spec.rb b/spec/lib/diaspora_federation/validators/contact_validator_spec.rb index 476d859..27ab84a 100644 --- a/spec/lib/diaspora_federation/validators/contact_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/contact_validator_spec.rb @@ -8,7 +8,6 @@ module DiasporaFederation describe "##{prop}" do it_behaves_like "a diaspora* ID validator" do let(:property) { prop } - let(:mandatory) { true } end end end diff --git a/spec/lib/diaspora_federation/validators/conversation_validator_spec.rb b/spec/lib/diaspora_federation/validators/conversation_validator_spec.rb index 7891159..49670ff 100644 --- a/spec/lib/diaspora_federation/validators/conversation_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/conversation_validator_spec.rb @@ -6,7 +6,6 @@ module DiasporaFederation it_behaves_like "a diaspora* ID validator" do let(:property) { :author } - let(:mandatory) { true } end it_behaves_like "a guid validator" do diff --git a/spec/lib/diaspora_federation/validators/event_validator_spec.rb b/spec/lib/diaspora_federation/validators/event_validator_spec.rb index 1bd3864..c516a95 100644 --- a/spec/lib/diaspora_federation/validators/event_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/event_validator_spec.rb @@ -6,7 +6,6 @@ module DiasporaFederation it_behaves_like "a diaspora* ID validator" do let(:property) { :author } - let(:mandatory) { true } end it_behaves_like "a guid validator" do diff --git a/spec/lib/diaspora_federation/validators/message_validator_spec.rb b/spec/lib/diaspora_federation/validators/message_validator_spec.rb index 7347b9b..0889cbb 100644 --- a/spec/lib/diaspora_federation/validators/message_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/message_validator_spec.rb @@ -5,7 +5,6 @@ module DiasporaFederation it_behaves_like "a diaspora* ID validator" do let(:property) { :author } - let(:mandatory) { true } end describe "#guid" do diff --git a/spec/lib/diaspora_federation/validators/participation_validator_spec.rb b/spec/lib/diaspora_federation/validators/participation_validator_spec.rb index cd7cadd..7ffdcfc 100644 --- a/spec/lib/diaspora_federation/validators/participation_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/participation_validator_spec.rb @@ -6,7 +6,6 @@ module DiasporaFederation it_behaves_like "a diaspora* ID validator" do let(:property) { :author } - let(:mandatory) { true } end describe "#guid" do diff --git a/spec/lib/diaspora_federation/validators/person_validator_spec.rb b/spec/lib/diaspora_federation/validators/person_validator_spec.rb index e4f7726..6b7d61d 100644 --- a/spec/lib/diaspora_federation/validators/person_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/person_validator_spec.rb @@ -6,7 +6,6 @@ module DiasporaFederation it_behaves_like "a diaspora* ID validator" do let(:property) { :author } - let(:mandatory) { true } end it_behaves_like "a guid validator" do diff --git a/spec/lib/diaspora_federation/validators/photo_validator_spec.rb b/spec/lib/diaspora_federation/validators/photo_validator_spec.rb index 97682cc..342ebac 100644 --- a/spec/lib/diaspora_federation/validators/photo_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/photo_validator_spec.rb @@ -6,7 +6,6 @@ module DiasporaFederation it_behaves_like "a diaspora* ID validator" do let(:property) { :author } - let(:mandatory) { true } end describe "#guid" do diff --git a/spec/lib/diaspora_federation/validators/profile_validator_spec.rb b/spec/lib/diaspora_federation/validators/profile_validator_spec.rb index d93b17a..5f8b52c 100644 --- a/spec/lib/diaspora_federation/validators/profile_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/profile_validator_spec.rb @@ -6,7 +6,6 @@ module DiasporaFederation it_behaves_like "a diaspora* ID validator" do let(:property) { :author } - let(:mandatory) { false } end %i[first_name last_name].each do |prop| diff --git a/spec/lib/diaspora_federation/validators/related_entity_validator_spec.rb b/spec/lib/diaspora_federation/validators/related_entity_validator_spec.rb index d42d3fc..56bf669 100644 --- a/spec/lib/diaspora_federation/validators/related_entity_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/related_entity_validator_spec.rb @@ -6,7 +6,6 @@ module DiasporaFederation it_behaves_like "a diaspora* ID validator" do let(:property) { :author } - let(:mandatory) { true } end %i[local public].each do |prop| diff --git a/spec/lib/diaspora_federation/validators/reshare_validator_spec.rb b/spec/lib/diaspora_federation/validators/reshare_validator_spec.rb index e78905d..2f50ef5 100644 --- a/spec/lib/diaspora_federation/validators/reshare_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/reshare_validator_spec.rb @@ -6,7 +6,6 @@ module DiasporaFederation describe "#author" do it_behaves_like "a diaspora* ID validator" do let(:property) { :author } - let(:mandatory) { true } end end @@ -23,9 +22,10 @@ module DiasporaFederation end describe "#root_author" do - it_behaves_like "a diaspora* ID validator" do + it_behaves_like "a property with a value validation/restriction" do let(:property) { :root_author } - let(:mandatory) { false } + let(:wrong_values) { ["i am a weird diaspora* ID @@@ ### 12345"] } + let(:correct_values) { [nil, "alice@example.org"] } end end diff --git a/spec/lib/diaspora_federation/validators/retraction_validator_spec.rb b/spec/lib/diaspora_federation/validators/retraction_validator_spec.rb index a0bb5c8..b19294c 100644 --- a/spec/lib/diaspora_federation/validators/retraction_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/retraction_validator_spec.rb @@ -9,7 +9,6 @@ module DiasporaFederation it_behaves_like "a diaspora* ID validator" do let(:property) { :author } - let(:mandatory) { true } end describe "#target_type" do diff --git a/spec/lib/diaspora_federation/validators/status_message_validator_spec.rb b/spec/lib/diaspora_federation/validators/status_message_validator_spec.rb index 2fb5873..0f8bd4b 100644 --- a/spec/lib/diaspora_federation/validators/status_message_validator_spec.rb +++ b/spec/lib/diaspora_federation/validators/status_message_validator_spec.rb @@ -6,7 +6,6 @@ module DiasporaFederation it_behaves_like "a diaspora* ID validator" do let(:property) { :author } - let(:mandatory) { true } end it_behaves_like "a guid validator" do diff --git a/spec/support/shared_validator_specs.rb b/spec/support/shared_validator_specs.rb index 58a5c0a..16def0c 100644 --- a/spec/support/shared_validator_specs.rb +++ b/spec/support/shared_validator_specs.rb @@ -20,7 +20,6 @@ end shared_examples "a relayable validator" do it_behaves_like "a diaspora* ID validator" do let(:property) { :author } - let(:mandatory) { true } end describe "#guid" do @@ -74,16 +73,18 @@ shared_examples "a diaspora* ID validator" do [nil, ""].each do |val| validator = described_class.new(entity_stub(entity, property => val)) - if mandatory - expect(validator).not_to be_valid - expect(validator.errors).to include(property) - else - expect(validator).to be_valid - expect(validator.errors).to be_empty - end + expect(validator).not_to be_valid + expect(validator.errors).to include(property) end end + it "validates a well-formed diaspora* ID" do + validator = described_class.new(entity_stub(entity, property => "alice@example.org")) + + expect(validator).to be_valid + expect(validator.errors).to be_empty + end + it "must be a valid diaspora* ID" do validator = described_class.new(entity_stub(entity, property => "i am a weird diaspora* ID @@@ ### 12345")) From b6ec405e55e7f4ef1373c22e9c5168f1e45f9960 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 12 Sep 2017 23:08:13 +0200 Subject: [PATCH 4/5] Add author to the diaspora:// URL parser --- .../federation/diaspora_url_parser.rb | 17 ++++++---- .../federation/receiver/abstract_receiver.rb | 2 +- .../federation/diaspora_url_parser_spec.rb | 34 +++++++++---------- .../federation/receiver/private_spec.rb | 4 +-- .../federation/receiver/public_spec.rb | 4 +-- 5 files changed, 33 insertions(+), 28 deletions(-) diff --git a/lib/diaspora_federation/federation/diaspora_url_parser.rb b/lib/diaspora_federation/federation/diaspora_url_parser.rb index 3fb4300..dad4b41 100644 --- a/lib/diaspora_federation/federation/diaspora_url_parser.rb +++ b/lib/diaspora_federation/federation/diaspora_url_parser.rb @@ -5,22 +5,27 @@ module DiasporaFederation include Logging # Regex to find diaspora:// URLs - DIASPORA_URL_REGEX = %r{diaspora://(#{Entity::ENTITY_NAME_REGEX})/(#{Validation::Rule::Guid::VALID_CHARS})} + DIASPORA_URL_REGEX = %r{ + diaspora:// + (#{Validation::Rule::DiasporaId::DIASPORA_ID_REGEX})/ + (#{Entity::ENTITY_NAME_REGEX})/ + (#{Validation::Rule::Guid::VALID_CHARS}) + }ux # Parses all diaspora:// URLs from the text and fetches the entities from # the remote server if needed. # @param [String] sender the diaspora* ID of the sender of the entity # @param [String] text text with diaspora:// URLs to fetch - def self.fetch_linked_entities(sender, text) - text.scan(DIASPORA_URL_REGEX).each do |type, guid| - fetch_entity(sender, type, guid) + def self.fetch_linked_entities(text) + text.scan(DIASPORA_URL_REGEX).each do |author, type, guid| + fetch_entity(author, type, guid) end end - private_class_method def self.fetch_entity(sender, type, guid) + private_class_method def self.fetch_entity(author, type, guid) class_name = Entity.entity_class(type).to_s.rpartition("::").last return if DiasporaFederation.callbacks.trigger(:fetch_related_entity, class_name, guid) - Fetcher.fetch_public(sender, type, guid) + Fetcher.fetch_public(author, type, guid) rescue => e logger.error "Failed to fetch linked entity #{type}:#{guid}: #{e.class}: #{e.message}" end diff --git a/lib/diaspora_federation/federation/receiver/abstract_receiver.rb b/lib/diaspora_federation/federation/receiver/abstract_receiver.rb index d63ba6e..30a203e 100644 --- a/lib/diaspora_federation/federation/receiver/abstract_receiver.rb +++ b/lib/diaspora_federation/federation/receiver/abstract_receiver.rb @@ -46,7 +46,7 @@ module DiasporaFederation end def fetch_linked_entities_from_text - DiasporaUrlParser.fetch_linked_entities(sender, entity.text) if entity.respond_to?(:text) && entity.text + DiasporaUrlParser.fetch_linked_entities(entity.text) if entity.respond_to?(:text) && entity.text end end end diff --git a/spec/lib/diaspora_federation/federation/diaspora_url_parser_spec.rb b/spec/lib/diaspora_federation/federation/diaspora_url_parser_spec.rb index bce2b2d..0bd5628 100644 --- a/spec/lib/diaspora_federation/federation/diaspora_url_parser_spec.rb +++ b/spec/lib/diaspora_federation/federation/diaspora_url_parser_spec.rb @@ -1,6 +1,6 @@ module DiasporaFederation describe Federation::DiasporaUrlParser do - let(:sender) { Fabricate.sequence(:diaspora_id) } + let(:author) { Fabricate.sequence(:diaspora_id) } let(:guid) { Fabricate.sequence(:guid) } describe ".fetch_linked_entities" do @@ -11,56 +11,56 @@ module DiasporaFederation expect_callback(:fetch_related_entity, "Post", guid2).and_return(double) expect_callback(:fetch_related_entity, "Post", guid3).and_return(double) - text = "This is a [link to a post with markdown](diaspora://post/#{guid}) and one without " \ - "diaspora://post/#{guid2} and finally a last one diaspora://post/#{guid3}." + text = "This is a [link to a post with markdown](diaspora://#{author}/post/#{guid}) and one without " \ + "diaspora://#{author}/post/#{guid2} and finally a last one diaspora://#{author}/post/#{guid3}." - Federation::DiasporaUrlParser.fetch_linked_entities(sender, text) + Federation::DiasporaUrlParser.fetch_linked_entities(text) end it "ignores invalid diaspora:// urls" do expect(DiasporaFederation.callbacks).not_to receive(:trigger) - text = "This is an invalid link diaspora://Post/#{guid}) and another one: " \ - "diaspora://post/abcd." + text = "This is an invalid link diaspora://#{author}/Post/#{guid} and another one " \ + "diaspora://#{author}/post/abcd and last one: diaspora://example.org/post/#{guid}." - Federation::DiasporaUrlParser.fetch_linked_entities(sender, text) + Federation::DiasporaUrlParser.fetch_linked_entities(text) end it "allows to link other entities" do expect_callback(:fetch_related_entity, "Event", guid).and_return(double) - text = "This is a link to an event diaspora://event/#{guid}." + text = "This is a link to an event diaspora://#{author}/event/#{guid}." - Federation::DiasporaUrlParser.fetch_linked_entities(sender, text) + Federation::DiasporaUrlParser.fetch_linked_entities(text) end it "handles unknown entities gracefully" do expect(DiasporaFederation.callbacks).not_to receive(:trigger) - text = "This is a link to an event diaspora://unknown/#{guid}." + text = "This is a link to an event diaspora://#{author}/unknown/#{guid}." - Federation::DiasporaUrlParser.fetch_linked_entities(sender, text) + Federation::DiasporaUrlParser.fetch_linked_entities(text) end it "fetches entities from sender when not found locally" do expect_callback(:fetch_related_entity, "Post", guid).and_return(nil) - expect(Federation::Fetcher).to receive(:fetch_public).with(sender, "post", guid) + expect(Federation::Fetcher).to receive(:fetch_public).with(author, "post", guid) - text = "This is a link to a post: diaspora://post/#{guid}." + text = "This is a link to a post: diaspora://#{author}/post/#{guid}." - Federation::DiasporaUrlParser.fetch_linked_entities(sender, text) + Federation::DiasporaUrlParser.fetch_linked_entities(text) end it "handles fetch errors gracefully" do expect_callback(:fetch_related_entity, "Post", guid).and_return(nil) expect(Federation::Fetcher).to receive(:fetch_public).with( - sender, "post", guid + author, "post", guid ).and_raise(Federation::Fetcher::NotFetchable, "Something went wrong!") - text = "This is a link to a post: diaspora://post/#{guid}." + text = "This is a link to a post: diaspora://#{author}/post/#{guid}." expect { - Federation::DiasporaUrlParser.fetch_linked_entities(sender, text) + Federation::DiasporaUrlParser.fetch_linked_entities(text) }.not_to raise_error end end diff --git a/spec/lib/diaspora_federation/federation/receiver/private_spec.rb b/spec/lib/diaspora_federation/federation/receiver/private_spec.rb index 5635e91..e0fbb37 100644 --- a/spec/lib/diaspora_federation/federation/receiver/private_spec.rb +++ b/spec/lib/diaspora_federation/federation/receiver/private_spec.rb @@ -119,7 +119,7 @@ module DiasporaFederation end it "fetches linked entities when the received entity has a text property" do - expect(Federation::DiasporaUrlParser).to receive(:fetch_linked_entities).with(post.author, post.text) + expect(Federation::DiasporaUrlParser).to receive(:fetch_linked_entities).with(post.text) described_class.new(magic_env, recipient).receive end @@ -128,7 +128,7 @@ module DiasporaFederation profile = Fabricate(:profile_entity) magic_env = Salmon::MagicEnvelope.new(profile, profile.author) - expect(Federation::DiasporaUrlParser).to receive(:fetch_linked_entities).with(profile.author, profile.bio) + expect(Federation::DiasporaUrlParser).to receive(:fetch_linked_entities).with(profile.bio) described_class.new(magic_env, recipient).receive end diff --git a/spec/lib/diaspora_federation/federation/receiver/public_spec.rb b/spec/lib/diaspora_federation/federation/receiver/public_spec.rb index 04f2108..0096120 100644 --- a/spec/lib/diaspora_federation/federation/receiver/public_spec.rb +++ b/spec/lib/diaspora_federation/federation/receiver/public_spec.rb @@ -141,7 +141,7 @@ module DiasporaFederation end it "fetches linked entities when the received entity has a text property" do - expect(Federation::DiasporaUrlParser).to receive(:fetch_linked_entities).with(post.author, post.text) + expect(Federation::DiasporaUrlParser).to receive(:fetch_linked_entities).with(post.text) described_class.new(magic_env).receive end @@ -150,7 +150,7 @@ module DiasporaFederation profile = Fabricate(:profile_entity, public: true) magic_env = Salmon::MagicEnvelope.new(profile, profile.author) - expect(Federation::DiasporaUrlParser).to receive(:fetch_linked_entities).with(profile.author, profile.bio) + expect(Federation::DiasporaUrlParser).to receive(:fetch_linked_entities).with(profile.bio) described_class.new(magic_env).receive end From 67a2ba7449055f204cd9d3f442304b0e6f329930 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 12 Sep 2017 23:33:56 +0200 Subject: [PATCH 5/5] Add author parameter for diaspora:// URLs to the documentation --- docs/federation/diaspora_scheme.md | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/docs/federation/diaspora_scheme.md b/docs/federation/diaspora_scheme.md index 86a945c..80ee32e 100644 --- a/docs/federation/diaspora_scheme.md +++ b/docs/federation/diaspora_scheme.md @@ -13,20 +13,25 @@ it as software specific URL. The format is similar to the route used for [fetching][fetching], so if the receiving server doesn't know the linked entity yet, it can just be fetched. +When the entity with that `guid` is already available locally, the recipient +should validate that it's from `author` before linking to it. + ### Format -`diaspora://:type/:guid` +`diaspora://:author/:type/:guid` #### Parameters -| Name | Description | -| ------ | ---------------------------------------------- | -| `type` | The type of the linked entity in `snake_case`. | -| `guid` | The [GUID][guid] of the linked entity. | +| Name | Description | +| -------- | -------------------------------------------------------------------- | +| `author` | The [diaspora\* ID][diaspora-id] of the author of the linked entity. | +| `type` | The type of the linked entity in `snake_case`. | +| `guid` | The [GUID][guid] of the linked entity. | #### Example -`diaspora://post/17faf230675101350d995254001bd39e` +`diaspora://alice@example.org/post/17faf230675101350d995254001bd39e` [fetching]: {{ site.baseurl }}/federation/fetching.html +[diaspora-id]: {{ site.baseurl }}/federation/types.html#diaspora-id [guid]: {{ site.baseurl }}/federation/types.html#guid