[Metadata] Fix actors not sanitizing their description and refactor

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

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
 

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:title", content: Actor.display_name_and_username(group)),
-      Tag.tag(:meta, property: "og:url", content: actor.url),
+      Tag.tag(:meta, property: "og:url", content: group.url),
-      Tag.tag(:meta, property: "og:description", content: actor.summary),
+      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_avatar(group)
-    |> maybe_add_group_schema(actor)
+    |> 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{<script type="application/ld+json">#{json(group)}</script>} |> 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"

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

lib/service/metadata/metadata.ex

@@ -1,7 +1,13 @@
 defprotocol Mobilizon.Service.Metadata do
-  @doc """
+  @moduledoc """
-  Build tags
+  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

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)
+  @doc """
-  defp stringify_tag(tag, acc) when is_binary(tag), do: acc <> tag
+  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)
+    default_description(locale)
-    gettext("The event organizer didn't add any description.")
   end
 
   def process_description(description, _locale, limit) do
     description
-    |> HTMLFormatter.strip_tags()
+    |> 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