From 15a82c7bce196bed1ddc89794bbe890420678ec1 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 17 Nov 2020 15:45:08 +0100 Subject: [PATCH] [Metadata] Fix actors not sanitizing their description and refactor Signed-off-by: Thomas Citharel --- lib/service/formatter/html.ex | 2 +- lib/service/metadata/actor.ex | 30 +++++++++++------ lib/service/metadata/instance.ex | 15 ++++----- lib/service/metadata/metadata.ex | 10 ++++-- lib/service/metadata/utils.ex | 55 ++++++++++++++++++++++++++------ 5 files changed, 80 insertions(+), 32 deletions(-) diff --git a/lib/service/formatter/html.ex b/lib/service/formatter/html.ex index 926d3fee2..ef4cdd6a4 100644 --- a/lib/service/formatter/html.ex +++ b/lib/service/formatter/html.ex @@ -32,7 +32,7 @@ defmodule Mobilizon.Service.Formatter.HTML do @spec strip_tags_and_insert_spaces(String.t()) :: String.t() def strip_tags_and_insert_spaces(html) when is_binary(html) do html - |> String.replace(" String.replace("><", "> <") |> strip_tags() end diff --git a/lib/service/metadata/actor.ex b/lib/service/metadata/actor.ex index 5677edf8e..fd3988ddb 100644 --- a/lib/service/metadata/actor.ex +++ b/lib/service/metadata/actor.ex @@ -4,20 +4,32 @@ defimpl Mobilizon.Service.Metadata, for: Mobilizon.Actors.Actor do alias Mobilizon.Actors.Actor alias Mobilizon.Web.JsonLD.ObjectView alias Mobilizon.Web.MediaProxy + import Mobilizon.Service.Metadata.Utils, only: [process_description: 2, default_description: 1] + + def build_tags(_actor, _locale \\ "en") + + def build_tags(%Actor{type: :Group} = group, locale) do + default_desc = default_description(locale) + + group = + Map.update(group, :summary, default_desc, fn summary -> + process_description(summary, locale) + end) - def build_tags(%Actor{} = actor, _locale \\ "en") do [ - Tag.tag(:meta, property: "og:title", content: Actor.display_name_and_username(actor)), - Tag.tag(:meta, property: "og:url", content: actor.url), - Tag.tag(:meta, property: "og:description", content: actor.summary), + Tag.tag(:meta, property: "og:title", content: Actor.display_name_and_username(group)), + Tag.tag(:meta, property: "og:url", content: group.url), + Tag.tag(:meta, property: "og:description", content: group.summary), Tag.tag(:meta, property: "og:type", content: "profile"), - Tag.tag(:meta, property: "profile:username", content: actor.preferred_username), + Tag.tag(:meta, property: "profile:username", content: group.preferred_username), Tag.tag(:meta, property: "twitter:card", content: "summary") ] - |> maybe_add_avatar(actor) - |> maybe_add_group_schema(actor) + |> maybe_add_avatar(group) + |> add_group_schema(group) end + def build_tags(%Actor{} = _actor, _locale), do: [] + @spec maybe_add_avatar(list(Tag.t()), Actor.t()) :: list(Tag.t()) defp maybe_add_avatar(tags, actor) do if is_nil(actor.avatar) do @@ -28,12 +40,10 @@ defimpl Mobilizon.Service.Metadata, for: Mobilizon.Actors.Actor do end end - defp maybe_add_group_schema(tags, %Actor{type: :Group} = group) do + defp add_group_schema(tags, %Actor{} = group) do tags ++ [~s{} |> HTML.raw()] end - defp maybe_add_group_schema(tags, _), do: tags - # Insert JSON-LD schema by hand because Tag.content_tag wants to escape it defp json(%Actor{} = group) do "group.json" diff --git a/lib/service/metadata/instance.ex b/lib/service/metadata/instance.ex index aef15f862..5c88e4478 100644 --- a/lib/service/metadata/instance.ex +++ b/lib/service/metadata/instance.ex @@ -7,11 +7,15 @@ defmodule Mobilizon.Service.Metadata.Instance do alias Phoenix.HTML.Tag alias Mobilizon.Config - alias Mobilizon.Service.Formatter.HTML, as: HTMLFormatter + alias Mobilizon.Service.Metadata.Utils alias Mobilizon.Web.Endpoint + @doc """ + Build the list of tags for the instance + """ + @spec build_tags() :: list(Phoenix.HTML.safe()) def build_tags do - description = process_description(Config.instance_description()) + description = Utils.process_description(Config.instance_description()) title = "#{Config.instance_name()} - Mobilizon" instance_json_ld = """ @@ -38,11 +42,4 @@ defmodule Mobilizon.Service.Metadata.Instance do HTML.raw(instance_json_ld) ] end - - defp process_description(description) do - description - |> HTMLFormatter.strip_tags() - |> String.slice(0..200) - |> (&"#{&1}…").() - end end diff --git a/lib/service/metadata/metadata.ex b/lib/service/metadata/metadata.ex index eb58edb70..01def810f 100644 --- a/lib/service/metadata/metadata.ex +++ b/lib/service/metadata/metadata.ex @@ -1,7 +1,13 @@ defprotocol Mobilizon.Service.Metadata do - @doc """ - Build tags + @moduledoc """ + Service that allows producing metadata HTML tags about content """ + @doc """ + Build tags for an entity. Returns a list of `t:Phoenix.HTML.safe/0` tags. + + Locale can be provided to generate fallback descriptions. + """ + @spec build_tags(any(), String.t()) :: list(Phoenix.HTML.safe()) def build_tags(entity, locale \\ "en") end diff --git a/lib/service/metadata/utils.ex b/lib/service/metadata/utils.ex index f2c25bdf4..1fddcba1f 100644 --- a/lib/service/metadata/utils.ex +++ b/lib/service/metadata/utils.ex @@ -9,28 +9,63 @@ defmodule Mobilizon.Service.Metadata.Utils do @slice_limit 200 - @spec stringify_tags(Enum.t()) :: String.t() + @doc """ + Converts list of tags, containing either `t:Phoenix.HTML.safe/0` or strings, to a concatenated string listing the tags + """ + @spec stringify_tags(list(Phoenix.HTML.safe() | String.t())) :: String.t() def stringify_tags(tags), do: Enum.reduce(tags, "", &stringify_tag/2) - defp stringify_tag(tag, acc) when is_tuple(tag), do: acc <> HTML.safe_to_string(tag) - defp stringify_tag(tag, acc) when is_binary(tag), do: acc <> tag - + @doc """ + Removes the HTML tags from a text + """ @spec strip_tags(String.t()) :: String.t() - def strip_tags(text), do: HTMLFormatter.strip_tags(text) + def strip_tags(text), do: HTMLFormatter.strip_tags_and_insert_spaces(text) + @doc """ + Processes a text and limits it. + + * Removes the HTML tags from a text + * Slices it to a limit and add an ellipsis character + * Returns a default description if text is empty + """ @spec process_description(String.t(), String.t(), integer()) :: String.t() def process_description(description, locale \\ "en", limit \\ @slice_limit) def process_description(nil, locale, limit), do: process_description("", locale, limit) def process_description("", locale, _limit) do - Gettext.put_locale(locale) - gettext("The event organizer didn't add any description.") + default_description(locale) end def process_description(description, _locale, limit) do description - |> HTMLFormatter.strip_tags() - |> String.slice(0..limit) - |> (&"#{&1}…").() + |> HTMLFormatter.strip_tags_and_insert_spaces() + |> String.trim() + |> maybe_slice(limit) end + + @doc """ + Returns the default description for a text + """ + @spec default_description(String.t()) :: String.t() + def default_description(locale \\ "en") do + Gettext.put_locale(locale) + gettext("The event organizer didn't add any description.") + end + + defp maybe_slice(description, limit) do + if String.length(description) > limit do + description + |> String.slice(0..limit) + |> String.trim() + |> (&"#{&1}…").() + else + description + end + end + + @spec stringify_tag(Phoenix.HTML.safe(), String.t()) :: String.t() + defp stringify_tag(tag, acc) when is_tuple(tag), do: acc <> HTML.safe_to_string(tag) + + @spec stringify_tag(String.t(), String.t()) :: String.t() + defp stringify_tag(tag, acc) when is_binary(tag), do: acc <> tag end