From faf48e1dd408288b37d5ec2ab28421931abb6890 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Fri, 1 Sep 2017 02:33:21 +0200 Subject: [PATCH 1/5] Extract allowed chars for GUIDs to constant Also: disallow special chars at the end of a GUID --- docs/federation/types.md | 2 ++ lib/diaspora_federation/validators/rules/guid.rb | 6 +++++- .../lib/diaspora_federation/validators/rules/guid_spec.rb | 8 ++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/federation/types.md b/docs/federation/types.md index 4d1dcd5..2771bff 100644 --- a/docs/federation/types.md +++ b/docs/federation/types.md @@ -31,6 +31,8 @@ A network-wide, unique identifier. A random string of at least 16 and at most 25 * Numbers: `0-9` * Special chars: `-`, `_`, `@`, `.` and `:` + Special chars aren't allowed at the end. + Example: `298962a0b8dc0133e40d406c8f31e210` ## String diff --git a/lib/diaspora_federation/validators/rules/guid.rb b/lib/diaspora_federation/validators/rules/guid.rb index 1405e1c..88ab8e6 100644 --- a/lib/diaspora_federation/validators/rules/guid.rb +++ b/lib/diaspora_federation/validators/rules/guid.rb @@ -6,7 +6,11 @@ module Validation # * Letters: a-z # * Numbers: 0-9 # * Special chars: '-', '_', '@', '.' and ':' + # 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 + # The error key for this rule # @return [Symbol] error key def error_key @@ -15,7 +19,7 @@ module Validation # Determines if value is a valid +GUID+ def valid_value?(value) - value.is_a?(String) && value.downcase =~ /\A[0-9a-z\-_@.:]{16,255}\z/ + value.is_a?(String) && value =~ /\A#{VALID_CHARS}\z/ end # This rule has no params. diff --git a/spec/lib/diaspora_federation/validators/rules/guid_spec.rb b/spec/lib/diaspora_federation/validators/rules/guid_spec.rb index d8954dc..3a71404 100644 --- a/spec/lib/diaspora_federation/validators/rules/guid_spec.rb +++ b/spec/lib/diaspora_federation/validators/rules/guid_spec.rb @@ -45,6 +45,14 @@ describe Validation::Rule::Guid do expect(validator.errors).to include(:guid) end + it "fails if the string contains special chars at the end" do + validator = Validation::Validator.new(OpenStruct.new(guid: "abcdef0123456789.")) + validator.rule(:guid, :guid) + + expect(validator).not_to be_valid + expect(validator.errors).to include(:guid) + end + it "fails if the string contains invalid chars" do validator = Validation::Validator.new(OpenStruct.new(guid: "ghijklmnopqrstuvwxyz++")) validator.rule(:guid, :guid) From 457f06d1da848bfe99c3c42f52bf93427c8dfc89 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Mon, 4 Sep 2017 01:41:32 +0200 Subject: [PATCH 2/5] Extract regex for entity names --- lib/diaspora_federation/entity.rb | 5 ++++- lib/diaspora_federation/federation/fetcher.rb | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/diaspora_federation/entity.rb b/lib/diaspora_federation/entity.rb index 2ba9aa6..30549b3 100644 --- a/lib/diaspora_federation/entity.rb +++ b/lib/diaspora_federation/entity.rb @@ -40,6 +40,9 @@ module DiasporaFederation # @see https://www.w3.org/TR/REC-xml/#charsets "Extensible Markup Language (XML) 1.0" INVALID_XML_REGEX = /[^\x09\x0A\x0D\x20-\uD7FF\uE000-\uFFFD\u{10000}-\u{10FFFF}]/ + # Regex to validate and find entity names + ENTITY_NAME_REGEX = "[a-z]*(?:_[a-z]*)*".freeze + # Initializes the Entity with the given attribute hash and freezes the created # instance it returns. # @@ -145,7 +148,7 @@ module DiasporaFederation # @param [String] entity_name "snake_case" class name # @return [Class] entity class def self.entity_class(entity_name) - raise InvalidEntityName, "'#{entity_name}' is invalid" unless entity_name =~ /\A[a-z]*(_[a-z]*)*\z/ + raise InvalidEntityName, "'#{entity_name}' is invalid" unless entity_name =~ /\A#{ENTITY_NAME_REGEX}\z/ class_name = entity_name.sub(/\A[a-z]/, &:upcase) class_name.gsub!(/_([a-z])/) { Regexp.last_match[1].upcase } diff --git a/lib/diaspora_federation/federation/fetcher.rb b/lib/diaspora_federation/federation/fetcher.rb index bce97df..76cf530 100644 --- a/lib/diaspora_federation/federation/fetcher.rb +++ b/lib/diaspora_federation/federation/fetcher.rb @@ -21,7 +21,7 @@ module DiasporaFederation end private_class_method def self.entity_name(class_name) - return class_name if class_name =~ /\A[a-z]*(_[a-z]*)*\z/ + return class_name if class_name =~ /\A#{Entity::ENTITY_NAME_REGEX}\z/ raise DiasporaFederation::Entity::UnknownEntity, class_name unless Entities.const_defined?(class_name) From 0b927290e33913ae055d7e05c4f3ab0f69b9204f Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 2 Sep 2017 04:18:31 +0200 Subject: [PATCH 3/5] Add DiasporaUrlParser to extract diaspora:// URLs from texts --- lib/diaspora_federation/federation.rb | 1 + .../federation/diaspora_url_parser.rb | 29 ++++++++ .../federation/diaspora_url_parser_spec.rb | 68 +++++++++++++++++++ 3 files changed, 98 insertions(+) create mode 100644 lib/diaspora_federation/federation/diaspora_url_parser.rb create mode 100644 spec/lib/diaspora_federation/federation/diaspora_url_parser_spec.rb diff --git a/lib/diaspora_federation/federation.rb b/lib/diaspora_federation/federation.rb index a55eed4..b279cfd 100644 --- a/lib/diaspora_federation/federation.rb +++ b/lib/diaspora_federation/federation.rb @@ -4,6 +4,7 @@ module DiasporaFederation end end +require "diaspora_federation/federation/diaspora_url_parser" require "diaspora_federation/federation/fetcher" require "diaspora_federation/federation/receiver" require "diaspora_federation/federation/sender" diff --git a/lib/diaspora_federation/federation/diaspora_url_parser.rb b/lib/diaspora_federation/federation/diaspora_url_parser.rb new file mode 100644 index 0000000..3fb4300 --- /dev/null +++ b/lib/diaspora_federation/federation/diaspora_url_parser.rb @@ -0,0 +1,29 @@ +module DiasporaFederation + module Federation + # This module is for parsing and fetching linked entities. + module DiasporaUrlParser + include Logging + + # Regex to find diaspora:// URLs + DIASPORA_URL_REGEX = %r{diaspora://(#{Entity::ENTITY_NAME_REGEX})/(#{Validation::Rule::Guid::VALID_CHARS})} + + # 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) + end + end + + private_class_method def self.fetch_entity(sender, 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) + rescue => e + logger.error "Failed to fetch linked entity #{type}:#{guid}: #{e.class}: #{e.message}" + end + 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 new file mode 100644 index 0000000..bce2b2d --- /dev/null +++ b/spec/lib/diaspora_federation/federation/diaspora_url_parser_spec.rb @@ -0,0 +1,68 @@ +module DiasporaFederation + describe Federation::DiasporaUrlParser do + let(:sender) { Fabricate.sequence(:diaspora_id) } + let(:guid) { Fabricate.sequence(:guid) } + + describe ".fetch_linked_entities" do + it "parses linked posts from the text" do + guid2 = Fabricate.sequence(:guid) + guid3 = Fabricate.sequence(:guid) + expect_callback(:fetch_related_entity, "Post", guid).and_return(double) + 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}." + + Federation::DiasporaUrlParser.fetch_linked_entities(sender, 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." + + Federation::DiasporaUrlParser.fetch_linked_entities(sender, 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}." + + Federation::DiasporaUrlParser.fetch_linked_entities(sender, 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}." + + Federation::DiasporaUrlParser.fetch_linked_entities(sender, 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) + + text = "This is a link to a post: diaspora://post/#{guid}." + + Federation::DiasporaUrlParser.fetch_linked_entities(sender, 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 + ).and_raise(Federation::Fetcher::NotFetchable, "Something went wrong!") + + text = "This is a link to a post: diaspora://post/#{guid}." + + expect { + Federation::DiasporaUrlParser.fetch_linked_entities(sender, text) + }.not_to raise_error + end + end + end +end From add5e16abfdc3242e2c362ebd5468b10b6e33d9d Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Mon, 4 Sep 2017 03:20:32 +0200 Subject: [PATCH 4/5] Fetch linked entities from received entities with text --- lib/diaspora_federation/entities/event.rb | 2 +- lib/diaspora_federation/entities/profile.rb | 5 +++- .../federation/receiver/abstract_receiver.rb | 5 ++++ .../entities/account_migration_spec.rb | 2 +- .../entities/person_spec.rb | 2 +- .../entities/profile_spec.rb | 4 +-- .../federation/receiver/private_spec.rb | 30 +++++++++++++++++++ .../federation/receiver/public_spec.rb | 30 +++++++++++++++++++ 8 files changed, 74 insertions(+), 6 deletions(-) diff --git a/lib/diaspora_federation/entities/event.rb b/lib/diaspora_federation/entities/event.rb index 1568f0c..0eda632 100644 --- a/lib/diaspora_federation/entities/event.rb +++ b/lib/diaspora_federation/entities/event.rb @@ -24,7 +24,7 @@ module DiasporaFederation # @!attribute [r] description # Description of the event # @return [String] event description - property :description, :string, optional: true + property :description, :string, alias: :text, optional: true # @!attribute [r] start # The start time of the event diff --git a/lib/diaspora_federation/entities/profile.rb b/lib/diaspora_federation/entities/profile.rb index 8fa2736..8caebf7 100644 --- a/lib/diaspora_federation/entities/profile.rb +++ b/lib/diaspora_federation/entities/profile.rb @@ -43,9 +43,12 @@ module DiasporaFederation # @return [String] url to the small avatar (50x50) property :image_url_small, :string, optional: true + # @!attribute [r] bio + # @return [String] bio of the person + property :bio, :string, alias: :text, optional: true + property :birthday, :string, optional: true property :gender, :string, optional: true - property :bio, :string, optional: true property :location, :string, optional: true # @!attribute [r] searchable diff --git a/lib/diaspora_federation/federation/receiver/abstract_receiver.rb b/lib/diaspora_federation/federation/receiver/abstract_receiver.rb index 1d23a1c..d63ba6e 100644 --- a/lib/diaspora_federation/federation/receiver/abstract_receiver.rb +++ b/lib/diaspora_federation/federation/receiver/abstract_receiver.rb @@ -30,6 +30,7 @@ module DiasporaFederation validate DiasporaFederation.callbacks.trigger(:receive_entity, entity, sender, recipient_id) logger.info "successfully received #{entity} from person #{sender}#{" for #{recipient_id}" if recipient_id}" + fetch_linked_entities_from_text end def validate @@ -43,6 +44,10 @@ module DiasporaFederation sender == entity.author end end + + def fetch_linked_entities_from_text + DiasporaUrlParser.fetch_linked_entities(sender, entity.text) if entity.respond_to?(:text) && entity.text + end end end end diff --git a/spec/lib/diaspora_federation/entities/account_migration_spec.rb b/spec/lib/diaspora_federation/entities/account_migration_spec.rb index 928ff8b..debc3d0 100644 --- a/spec/lib/diaspora_federation/entities/account_migration_spec.rb +++ b/spec/lib/diaspora_federation/entities/account_migration_spec.rb @@ -24,9 +24,9 @@ module DiasporaFederation #{data[:profile].image_url} #{data[:profile].image_url} #{data[:profile].image_url} + #{data[:profile].bio} #{data[:profile].birthday} #{data[:profile].gender} - #{data[:profile].bio} #{data[:profile].location} #{data[:profile].searchable} #{data[:profile].public} diff --git a/spec/lib/diaspora_federation/entities/person_spec.rb b/spec/lib/diaspora_federation/entities/person_spec.rb index 435aac6..6344827 100644 --- a/spec/lib/diaspora_federation/entities/person_spec.rb +++ b/spec/lib/diaspora_federation/entities/person_spec.rb @@ -13,9 +13,9 @@ module DiasporaFederation #{data[:profile].image_url} #{data[:profile].image_url} #{data[:profile].image_url} + #{data[:profile].bio} #{data[:profile].birthday} #{data[:profile].gender} - #{data[:profile].bio} #{data[:profile].location} #{data[:profile].searchable} #{data[:profile].public} diff --git a/spec/lib/diaspora_federation/entities/profile_spec.rb b/spec/lib/diaspora_federation/entities/profile_spec.rb index d162556..30b1aa1 100644 --- a/spec/lib/diaspora_federation/entities/profile_spec.rb +++ b/spec/lib/diaspora_federation/entities/profile_spec.rb @@ -9,9 +9,9 @@ module DiasporaFederation #{data[:image_url]} #{data[:image_url]} #{data[:image_url]} + #{data[:bio]} #{data[:birthday]} #{data[:gender]} - #{data[:bio]} #{data[:location]} #{data[:searchable]} #{data[:public]} @@ -29,9 +29,9 @@ XML "image_url": "#{data[:image_url]}", "image_url_medium": "#{data[:image_url]}", "image_url_small": "#{data[:image_url]}", + "bio": "#{data[:bio]}", "birthday": "#{data[:birthday]}", "gender": "#{data[:gender]}", - "bio": "#{data[:bio]}", "location": "#{data[:location]}", "searchable": #{data[:searchable]}, "public": #{data[:public]}, diff --git a/spec/lib/diaspora_federation/federation/receiver/private_spec.rb b/spec/lib/diaspora_federation/federation/receiver/private_spec.rb index 2deda9d..5635e91 100644 --- a/spec/lib/diaspora_federation/federation/receiver/private_spec.rb +++ b/spec/lib/diaspora_federation/federation/receiver/private_spec.rb @@ -112,6 +112,36 @@ module DiasporaFederation end end end + + context "with text" do + before do + expect(DiasporaFederation.callbacks).to receive(:trigger) + 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) + + described_class.new(magic_env, recipient).receive + end + + it "fetches linked entities for the profile bio" do + 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) + + described_class.new(magic_env, recipient).receive + end + + it "doesn't try to fetch linked entities when the text is nil" do + photo = Fabricate(:photo_entity, public: false, text: nil) + magic_env = Salmon::MagicEnvelope.new(photo, photo.author) + + expect(Federation::DiasporaUrlParser).not_to receive(:fetch_linked_entities) + + described_class.new(magic_env, recipient).receive + end + end end end end diff --git a/spec/lib/diaspora_federation/federation/receiver/public_spec.rb b/spec/lib/diaspora_federation/federation/receiver/public_spec.rb index 0234487..04f2108 100644 --- a/spec/lib/diaspora_federation/federation/receiver/public_spec.rb +++ b/spec/lib/diaspora_federation/federation/receiver/public_spec.rb @@ -134,6 +134,36 @@ module DiasporaFederation described_class.new(magic_env).receive end end + + context "with text" do + before do + expect(DiasporaFederation.callbacks).to receive(:trigger) + 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) + + described_class.new(magic_env).receive + end + + it "fetches linked entities for the profile bio" do + 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) + + described_class.new(magic_env).receive + end + + it "doesn't try to fetch linked entities when the text is nil" do + photo = Fabricate(:photo_entity, text: nil) + magic_env = Salmon::MagicEnvelope.new(photo, photo.author) + + expect(Federation::DiasporaUrlParser).not_to receive(:fetch_linked_entities) + + described_class.new(magic_env).receive + end + end end end end From b1a5c13a083564083ce54e7522328c99a6b85b50 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Mon, 4 Sep 2017 03:51:04 +0200 Subject: [PATCH 5/5] Add documentation for the diaspora:// URI scheme --- docs/_includes/federation_tree.html | 3 ++- docs/federation/diaspora_scheme.md | 32 +++++++++++++++++++++++++++++ docs/federation/types.md | 4 +++- 3 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 docs/federation/diaspora_scheme.md diff --git a/docs/_includes/federation_tree.html b/docs/_includes/federation_tree.html index 989a659..88bfa62 100644 --- a/docs/_includes/federation_tree.html +++ b/docs/_includes/federation_tree.html @@ -1,9 +1,10 @@ diff --git a/docs/federation/diaspora_scheme.md b/docs/federation/diaspora_scheme.md new file mode 100644 index 0000000..86a945c --- /dev/null +++ b/docs/federation/diaspora_scheme.md @@ -0,0 +1,32 @@ +--- +title: diaspora:// URI scheme +--- + +## Server and software independent links + +A `diaspora://` URL is used if a user wants to link to another post. It doesn't +contain a server hostname so it is independent of the senders server. And it +isn't software specific, it is thought to be compatible with every software +that is compatible with the protocol, so the receiving software can display +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. + +### Format + +`diaspora://:type/:guid` + +#### Parameters + +| Name | Description | +| ------ | ---------------------------------------------- | +| `type` | The type of the linked entity in `snake_case`. | +| `guid` | The [GUID][guid] of the linked entity. | + +#### Example + +`diaspora://post/17faf230675101350d995254001bd39e` + +[fetching]: {{ site.baseurl }}/federation/fetching.html +[guid]: {{ site.baseurl }}/federation/types.html#guid diff --git a/docs/federation/types.md b/docs/federation/types.md index 2771bff..3d3be4b 100644 --- a/docs/federation/types.md +++ b/docs/federation/types.md @@ -57,7 +57,9 @@ Example: `12.3456` Text formatted with markdown using the [CommonMark spec][commonmark]. -Example: `Some *Text* with **markdown**.` +It can also contain [diaspora:// URLs][diaspora_scheme]. + +Example: `Some *Text* with **markdown** and a [link](diaspora://post/298962a0b8dc0133e40d406c8f31e210).` ## URL