Improve RSS compliance

The core feed reader issue was that RSS 2.0 <link> is a simple text
element and can only appear once per <item>.
By having multiple <link> tags with rel attributes, parsers were
getting confused and resolving entries to the wrong URL.
Now each item has exactly one <link>: the post URL. Everything else
uses valid <atom:link> or dedicated extension elements where
appropriate.

changelog.d/rss.fix

@@ -0,0 +1 @@
+Fix RSS feeds to be W3C compliant and render better in RSS readers

lib/pleroma/web/feed/feed_view.ex

@@ -97,6 +97,64 @@ defmodule Pleroma.Web.Feed.FeedView do
     end
   end
 
+  def rss_content_encoded(data) do
+    data
+    |> feed_content_encoded()
+    |> String.replace("]]>", "]]>")
+  end
+
+  def atom_content_encoded(data), do: feed_content_encoded(data)
+
+  defp feed_content_encoded(data) do
+    base_content =
+      case activity_content(data) do
+        "" ->
+          data["summary"] || data["type"] ||
+            ""
+            |> escape()
+
+        content ->
+          content
+      end
+
+    attachments_html =
+      (data["attachment"] || [])
+      |> Enum.map(&rss_attachment_preview/1)
+      |> Enum.reject(&(&1 == ""))
+      |> Enum.join("<br/><br/>")
+
+    [base_content, attachments_html]
+    |> Enum.reject(&(&1 == ""))
+    |> Enum.join("<br/><br/>")
+  end
+
+  defp rss_attachment_preview(attachment) do
+    href = attachment_href(attachment)
+
+    if is_binary(href) and href != "" do
+      media_type = attachment_type(attachment) || "application/octet-stream"
+      escaped_href = escape(href)
+      name = attachment["name"] || "Attachment"
+      escaped_name = escape(name)
+
+      cond do
+        String.starts_with?(media_type, "image/") ->
+          ~s(<p><img src="#{escaped_href}" alt="#{escaped_name}" loading="lazy"/></p>)
+
+        String.starts_with?(media_type, "video/") ->
+          ~s(<p><video controls preload="metadata" src="#{escaped_href}"></video></p><p><a href="#{escaped_href}">#{escaped_name}</a></p>)
+
+        String.starts_with?(media_type, "audio/") ->
+          ~s(<p><audio controls preload="metadata" src="#{escaped_href}"></audio></p><p><a href="#{escaped_href}">#{escaped_name}</a></p>)
+
+        true ->
+          ~s(<p><a href="#{escaped_href}">#{escaped_name}</a></p>)
+      end
+    else
+      ""
+    end
+  end
+
   def activity_content(%{"content" => content}) do
     content
     |> String.replace(~r/[\n\r]/, "")
@@ -106,16 +164,99 @@ defmodule Pleroma.Web.Feed.FeedView do
 
   def activity_context(activity), do: escape(activity.data["context"])
 
+  def feed_self_url(conn), do: Phoenix.Controller.current_url(conn)
+
+  def activity_link(activity, data) do
+    cond do
+      activity.local -> data["id"] || data["url"]
+      true -> data["external_url"] || data["id"] || data["url"]
+    end
+  end
+
   def attachment_href(attachment) do
-    attachment["url"]
+    attachment
-    |> hd()
+    |> attachment_url_data()
     |> Map.get("href")
   end
 
   def attachment_type(attachment) do
-    attachment["url"]
+    attachment
-    |> hd()
+    |> attachment_url_data()
-    |> Map.get("mediaType")
+    |> Map.get("mediaType") || attachment["mediaType"]
+  end
+
+  def attachment_size(attachment) do
+    attachment
+    |> attachment_url_data()
+    |> Map.get("size", 0)
+  end
+
+  def attachment_size_positive(attachment) do
+    case attachment_size(attachment) do
+      size when is_integer(size) and size > 0 ->
+        size
+
+      size when is_binary(size) ->
+        case Integer.parse(size) do
+          {parsed, ""} when parsed > 0 -> parsed
+          _ -> nil
+        end
+
+      _ ->
+        nil
+    end
+  end
+
+  def attachment_medium(attachment) do
+    case attachment_type(attachment) do
+      "image/" <> _rest -> "image"
+      "video/" <> _rest -> "video"
+      "audio/" <> _rest -> "audio"
+      _ -> "document"
+    end
+  end
+
+  def attachment_previewable?(attachment) do
+    attachment_medium(attachment) in ["image", "video", "audio"]
+  end
+
+  def attachment_description(attachment) do
+    attachment["name"] || ""
+  end
+
+  def media_content_xml(attachment) do
+    href = escape(attachment_href(attachment) || "")
+    type = escape(attachment_type(attachment) || "application/octet-stream")
+    medium = escape(attachment_medium(attachment))
+
+    file_size_attr =
+      case attachment_size_positive(attachment) do
+        size when is_integer(size) -> ~s( fileSize="#{size}")
+        _ -> ""
+      end
+
+    description_xml =
+      case attachment_description(attachment) do
+        "" ->
+          ""
+
+        description ->
+          ~s(\n  <media:description type="plain">#{escape(description)}</media:description>)
+      end
+
+    """
+    <media:content url="#{href}" type="#{type}"#{file_size_attr} medium="#{medium}">
+      <media:rating scheme="urn:simple">nonadult</media:rating>#{description_xml}
+    </media:content>
+    """
+  end
+
+  defp attachment_url_data(attachment) do
+    case attachment["url"] do
+      [first | _] when is_map(first) -> first
+      %{} = map -> map
+      _ -> %{}
+    end
   end
 
   def get_href(id) do

lib/pleroma/web/templates/feed/feed/_activity.atom.eex

@@ -3,7 +3,7 @@
   <activity:verb>http://activitystrea.ms/schema/1.0/post</activity:verb>
   <id><%= @data["id"] %></id>
   <title><%= activity_title(@data, Keyword.get(@feed_config, :post_title, %{})) %></title>
-  <content type="html"><%= activity_description(@data) %></content>
+  <content type="html"><%= escape(atom_content_encoded(@data)) %></content>
   <published><%= to_rfc3339(@data["published"]) %></published>
   <updated><%= to_rfc3339(@data["published"]) %></updated>
   <ostatus:conversation ref="<%= activity_context(@activity) %>">
@@ -15,35 +15,23 @@
     <summary><%= escape(@data["summary"]) %></summary>
   <% end %>
 
-  <%= if @activity.local do %>
+  <link type="application/atom+xml" href='<%= activity_link(@activity, @data) %>' rel="self"/>
-    <link type="application/atom+xml" href='<%= @data["id"] %>' rel="self"/>
+  <link type="text/html" href='<%= activity_link(@activity, @data) %>' rel="alternate"/>
-    <link type="text/html" href='<%= @data["id"] %>' rel="alternate"/>
-  <% else %>
-    <link type="text/html" href='<%= @data["external_url"] %>' rel="alternate"/>
-  <% end %>
 
   <%= for tag <- Pleroma.Object.hashtags(@object) do %>
     <category term="<%= tag %>"></category>
   <% end %>
 
   <%= for attachment <- @data["attachment"] || [] do %>
-    <link rel="enclosure" href="<%= attachment_href(attachment) %>" type="<%= attachment_type(attachment) %>"/>
+    <%= unless attachment_previewable?(attachment) do %>
+      <link rel="enclosure" href="<%= attachment_href(attachment) %>" type="<%= attachment_type(attachment) %>" length="<%= attachment_size(attachment) %>"/>
+    <% end %>
   <% end %>
 
   <%= if @data["inReplyTo"] do %>
     <thr:in-reply-to ref='<%= @data["inReplyTo"] %>' href='<%= get_href(@data["inReplyTo"]) %>'/>
   <% end %>
 
-  <%= for id <- @activity.recipients do %>
-    <%= if id == Pleroma.Constants.as_public() do %>
-      <link rel="mentioned" ostatus:object-type="http://activitystrea.ms/schema/1.0/collection" href="http://activityschema.org/collection/public"/>
-    <% else %>
-      <%= unless Regex.match?(~r/^#{Pleroma.Web.Endpoint.url()}.+followers$/, id) do %>
-        <link rel="mentioned" ostatus:object-type="http://activitystrea.ms/schema/1.0/person" href="<%= id %>"/>
-      <% end %>
-    <% end %>
-  <% end %>
-
   <%= for {emoji, file} <- @data["emoji"] || %{} do %>
     <link name="<%= emoji %>" rel="emoji" href="<%= file %>"/>
   <% end %>

lib/pleroma/web/templates/feed/feed/_activity.rss.eex

@@ -1,45 +1,32 @@
 <item>
   <activity:object-type>http://activitystrea.ms/schema/1.0/note</activity:object-type>
   <activity:verb>http://activitystrea.ms/schema/1.0/post</activity:verb>
-  <guid><%= @data["id"] %></guid>
+  <guid isPermaLink="true"><%= @data["id"] %></guid>
   <title><%= activity_title(@data, Keyword.get(@feed_config, :post_title, %{})) %></title>
   <description><%= activity_description(@data) %></description>
+  <content:encoded><![CDATA[<%= rss_content_encoded(@data) %>]]></content:encoded>
   <pubDate><%= to_rfc2822(@data["published"]) %></pubDate>
   <ostatus:conversation ref="<%= activity_context(@activity) %>">
     <%= activity_context(@activity) %>
   </ostatus:conversation>
 
-  <%= if @activity.local do %>
+  <link><%= activity_link(@activity, @data) %></link>
-    <link><%= @data["id"] %></link>
-  <% else %>
-    <link><%= @data["external_url"] %></link>
-  <% end %>
-
-  <link rel="ostatus:conversation"><%= activity_context(@activity) %></link>
 
   <%= for tag <- Pleroma.Object.hashtags(@object) do %>
-    <category term="<%= tag %>"></category>
+    <%= if tag && tag != "" do %>
+      <category><%= tag %></category>
+    <% end %>
   <% end %>
 
   <%= for attachment <- @data["attachment"] || [] do %>
-    <enclosure url="<%= attachment_href(attachment) %>" type="<%= attachment_type(attachment) %>" />
+    <%= size = attachment_size_positive(attachment) %>
+    <%= unless attachment_previewable?(attachment) do %>
+      <enclosure url="<%= attachment_href(attachment) %>" type="<%= attachment_type(attachment) %>" length="<%= size || 0 %>" />
+    <% end %>
+    <%= media_content_xml(attachment) %>
   <% end %>
 
   <%= if @data["inReplyTo"] do %>
     <thr:in-reply-to ref='<%= @data["inReplyTo"] %>' href='<%= get_href(@data["inReplyTo"]) %>'/>
   <% end %>
-
-  <%= for id <- @activity.recipients do %>
-    <%= if id == Pleroma.Constants.as_public() do %>
-      <link rel="mentioned" ostatus:object-type="http://activitystrea.ms/schema/1.0/collection">http://activityschema.org/collection/public</link>
-    <% else %>
-      <%= unless Regex.match?(~r/^#{Pleroma.Web.Endpoint.url()}.+followers$/, id) do %>
-        <link rel="mentioned" ostatus:object-type="http://activitystrea.ms/schema/1.0/person"><%= id %></link>
-      <% end %>
-    <% end %>
-  <% end %>
-
-  <%= for {emoji, file} <- @data["emoji"] || %{} do %>
-    <link name="<%= emoji %>" rel="emoji"><%= file %></link>
-  <% end %>
 </item>

lib/pleroma/web/templates/feed/feed/_tag_activity.atom.eex

@@ -6,7 +6,7 @@
 
   <id><%= @data["id"] %></id>
   <title><%= activity_title(@data, Keyword.get(@feed_config, :post_title, %{})) %></title>
-  <content type="html"><%= activity_description(@data) %></content>
+  <content type="html"><%= escape(atom_content_encoded(@data)) %></content>
   <published><%= to_rfc3339(@data["published"]) %></published>
   <updated><%= to_rfc3339(@data["published"]) %></updated>
   <ostatus:conversation ref="<%= activity_context(@activity) %>">
@@ -15,34 +15,22 @@
   <link href="<%= activity_context(@activity) %>" rel="ostatus:conversation"/>
 
   <%= if @data["summary"] != "" do %>
-    <summary><%= @data["summary"] %></summary>
+    <summary><%= escape(@data["summary"]) %></summary>
   <% end %>
 
-  <%= if @activity.local do %>
+  <link type="application/atom+xml" href='<%= activity_link(@activity, @data) %>' rel="self"/>
-    <link type="application/atom+xml" href='<%= @data["id"] %>' rel="self"/>
+  <link type="text/html" href='<%= activity_link(@activity, @data) %>' rel="alternate"/>
-    <link type="text/html" href='<%= @data["id"] %>' rel="alternate"/>
-  <% else %>
-    <link type="text/html" href='<%= @data["external_url"] %>' rel="alternate"/>
-  <% end %>
-
-  <%= for id <- @activity.recipients do %>
-    <%= if id == Pleroma.Constants.as_public() do %>
-      <link rel="mentioned"
-            ostatus:object-type="http://activitystrea.ms/schema/1.0/collection"
-            href="http://activityschema.org/collection/public"/>
-    <% else %>
-      <%= unless Regex.match?(~r/^#{Pleroma.Web.Endpoint.url()}.+followers$/, id) do %>
-        <link rel="mentioned"
-              ostatus:object-type="http://activitystrea.ms/schema/1.0/person"
-              href="<%= id %>" />
-      <% end %>
-    <% end %>
-  <% end %>
 
   <%= for tag <- Pleroma.Object.hashtags(@object) do %>
     <category term="<%= tag %>"></category>
   <% end %>
 
+  <%= for attachment <- @data["attachment"] || [] do %>
+    <%= unless attachment_previewable?(attachment) do %>
+      <link rel="enclosure" href="<%= attachment_href(attachment) %>" type="<%= attachment_type(attachment) %>" length="<%= attachment_size(attachment) %>"/>
+    <% end %>
+  <% end %>
+
   <%= for {emoji, file} <- @data["emoji"] || %{} do %>
     <link name="<%= emoji %>" rel="emoji" href="<%= file %>"/>
   <% end %>

lib/pleroma/web/templates/feed/feed/_tag_activity.xml.eex

@@ -1,14 +1,22 @@
 <item>
-  <title><%= activity_title(@data, Keyword.get(@feed_config, :post_title, %{})) %></title>
+  <guid isPermalink="true"><%= activity_link(@activity, @data) %></guid>
-
+  <link><%= activity_link(@activity, @data) %></link>
-
-  <guid isPermalink="true"><%= activity_context(@activity) %></guid>
-  <link><%= activity_context(@activity) %></link>
   <pubDate><%= to_rfc2822(@data["published"]) %></pubDate>
-
+  <title><%= activity_title(@data, Keyword.get(@feed_config, :post_title, %{})) %></title>
   <description><%= activity_description(@data) %></description>
+  <content:encoded><![CDATA[<%= rss_content_encoded(@data) %>]]></content:encoded>
+
   <%= for attachment <- @data["attachment"] || [] do %>
-    <enclosure url="<%= attachment_href(attachment) %>" type="<%= attachment_type(attachment) %>"/>
+    <%= size = attachment_size_positive(attachment) %>
+    <%= unless attachment_previewable?(attachment) do %>
+      <enclosure url="<%= attachment_href(attachment) %>" type="<%= attachment_type(attachment) %>" length="<%= size || 0 %>"/>
+    <% end %>
+    <%= media_content_xml(attachment) %>
   <% end %>
 
+  <%= for tag <- Pleroma.Object.hashtags(@object) do %>
+    <%= if tag && tag != "" do %>
+      <category><%= tag %></category>
+    <% end %>
+  <% end %>
 </item>

lib/pleroma/web/templates/feed/feed/tag.atom.eex

@@ -9,10 +9,12 @@
 
   <id><%= Routes.tag_feed_url(@conn, :feed, @tag) <> ".atom" %></id>
   <title>#<%= @tag %></title>
-  <subtitle><%= Gettext.dpgettext("static_pages", "tag feed description", "These are public toots tagged with #%{tag}. You can interact with them if you have an account anywhere in the fediverse.", tag: @tag) %></subtitle>
+  <%= with subtitle when subtitle not in [nil, ""] <- Gettext.dpgettext("static_pages", "tag feed description", "These are public toots tagged with #%{tag}. You can interact with them if you have an account anywhere in the fediverse.", tag: @tag) do %>
+    <subtitle><%= subtitle %></subtitle>
+  <% end %>
   <logo><%= feed_logo() %></logo>
   <updated><%= most_recent_update(@activities) %></updated>
-  <link rel="self" href="<%= "#{Routes.tag_feed_url(@conn, :feed, @tag)}.atom"  %>" type="application/atom+xml"/>
+  <link rel="self" href="<%= feed_self_url(@conn) %>" type="application/atom+xml"/>
 
   <%= for activity <- @activities do %>
   <%= render Phoenix.Controller.view_module(@conn), "_tag_activity.atom", Map.merge(assigns, prepare_activity(activity, actor: true)) %>

lib/pleroma/web/templates/feed/feed/tag.rss.eex

@@ -1,6 +1,8 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <rss version="2.0"
      xmlns:webfeeds="http://webfeeds.org/rss/1.0"
+     xmlns:content="http://purl.org/rss/1.0/modules/content/"
+     xmlns:media="http://search.yahoo.com/mrss/"
      xmlns:thr="http://purl.org/syndication/thread/1.0">
   <channel>
 
@@ -8,7 +10,10 @@
     <description><%= Gettext.dpgettext("static_pages", "tag feed description", "These are public toots tagged with #%{tag}. You can interact with them if you have an account anywhere in the fediverse.", tag: @tag) %></description>
     <link><%= "#{Routes.tag_feed_url(@conn, :feed, @tag)}.rss" %></link>
     <webfeeds:logo><%= feed_logo() %></webfeeds:logo>
+    <webfeeds:icon><%= feed_logo() %></webfeeds:icon>
     <webfeeds:accentColor>2b90d9</webfeeds:accentColor>
+    <lastBuildDate><%= most_recent_update(@activities) || to_rfc2822(NaiveDateTime.utc_now()) %></lastBuildDate>
+    <generator><%= Pleroma.Application.named_version() %></generator>
     <%= for activity <- @activities do %>
     <%= render Phoenix.Controller.view_module(@conn), "_tag_activity.xml", Map.merge(assigns, prepare_activity(activity)) %>
     <% end %>

lib/pleroma/web/templates/feed/feed/user.atom.eex

@@ -8,10 +8,12 @@
 
   <id><%= Routes.user_feed_url(@conn, :feed, @user.nickname) <> ".atom" %></id>
   <title><%= @user.nickname <> "'s timeline" %></title>
-  <subtitle><%= escape(@user.bio) %></subtitle>
+  <%= if @user.bio && @user.bio != "" do %>
+    <subtitle><%= escape(@user.bio) %></subtitle>
+  <% end %>
   <updated><%= most_recent_update(@activities, @user, :atom) %></updated>
   <logo><%= logo(@user) %></logo>
-  <link rel="self" href="<%= "#{Routes.user_feed_url(@conn, :feed, @user.nickname)}.atom" %>" type="application/atom+xml"/>
+  <link rel="self" href="<%= feed_self_url(@conn) %>" type="application/atom+xml"/>
 
   <%= render Phoenix.Controller.view_module(@conn), "_author.atom", assigns %>
 

lib/pleroma/web/templates/feed/feed/user.rss.eex

@@ -1,26 +1,32 @@
 <?xml version="1.0" encoding="UTF-8" ?>
 <rss version="2.0"
      xmlns:atom="http://www.w3.org/2005/Atom"
+     xmlns:content="http://purl.org/rss/1.0/modules/content/"
+     xmlns:webfeeds="http://webfeeds.org/rss/1.0"
+     xmlns:media="http://search.yahoo.com/mrss/"
      xmlns:thr="http://purl.org/syndication/thread/1.0"
      xmlns:activity="http://activitystrea.ms/spec/1.0/"
      xmlns:ostatus="http://ostatus.org/schema/1.0"
      xmlns:poco="http://portablecontacts.net/spec/1.0">
   <channel>
     <title><%= @user.nickname <> "'s timeline" %></title>
-    <link><%= "#{Routes.user_feed_url(@conn, :feed, @user.nickname)}.rss" %></link>
+    <atom:link href="<%= feed_self_url(@conn) %>"
-    <atom:link href="<%= Routes.user_feed_url(@conn, :feed, @user.nickname) <> ".atom" %>"
                rel="self" type="application/rss+xml" />
+    <link><%= "#{Routes.user_feed_url(@conn, :feed, @user.nickname)}.rss" %></link>
     <description><%= escape(@user.bio) %></description>
     <image>
       <url><%= logo(@user) %></url>
       <title><%= @user.nickname <> "'s timeline" %></title>
       <link><%= "#{Routes.user_feed_url(@conn, :feed, @user.nickname)}.rss" %></link>
     </image>
+    <lastBuildDate><%= most_recent_update(@activities, @user, :rss) %></lastBuildDate>
+    <webfeeds:icon><%= logo(@user) %></webfeeds:icon>
+    <generator><%= Pleroma.Application.named_version() %></generator>
 
     <%= render Phoenix.Controller.view_module(@conn), "_author.rss", assigns %>
 
     <%= if last_activity(@activities) do %>
-      <link rel="next"><%= "#{Routes.user_feed_url(@conn, :feed, @user.nickname)}.rss?max_id=#{last_activity(@activities).id}" %></link>
+      <atom:link rel="next" href="<%= "#{Routes.user_feed_url(@conn, :feed, @user.nickname)}.rss?max_id=#{last_activity(@activities).id}" %>" />
     <% end %>
 
     <%= for activity <- @activities do %>

test/pleroma/web/feed/tag_controller_test.exs

@@ -141,7 +141,7 @@ defmodule Pleroma.Web.Feed.TagControllerTest do
              FeedView.to_rfc2822(activity1.data["published"])
            ]
 
-    assert xpath(xml, ~x"//channel/item/enclosure/@url"sl) == [
+    assert xpath(xml, ~x"//channel/item/media:content/@url"sl) == [
              "https://peertube.moe/static/webseed/df5f464b-be8d-46fb-ad81-2d4c2d1630e3-480.mp4"
            ]