Fix validation findings

controller/rss.php

@@ -10,6 +10,7 @@
 
 namespace phpbbmodders\feedlinks\controller;
 
+use phpbb\exception\http_exception;
 use phpbb\feed\feed_interface;
 use phpbbmodders\feedlinks\atom_to_rss;
 use Symfony\Component\HttpFoundation\Response;
@@ -45,12 +46,22 @@ public function set_converter(atom_to_rss $converter)
 	 *
 	 * @param feed_interface $feed
 	 * @return Response
+	 * @throws http_exception When phpBB's feed can't be converted
 	 */
 	protected function send_feed_do(feed_interface $feed)
 	{
 		$response = parent::send_feed_do($feed);
 
-		$response->setContent($this->converter->convert($response->getContent()));
+		try
+		{
+			$response->setContent($this->converter->convert($response->getContent()));
+		}
+		catch (\RuntimeException $e)
+		{
+			$this->language->add_lang('feedlinks', 'phpbbmodders/feedlinks');
+			throw new http_exception(Response::HTTP_INTERNAL_SERVER_ERROR, 'FEEDLINKS_RSS_ERROR', [], $e);
+		}
+
 		$response->headers->set('Content-Type', 'application/rss+xml; charset=UTF-8');
 
 		return $response;

docs/events/forumlist_body_forum_desc_after.txt

@@ -40,8 +40,8 @@ not the forum has a description, moderators or subforums.
 USE CASE
 ========
 phpbbmodders/feedlinks shows a forum's Atom and RSS feed links below its
-description ("Subscribe to the feed: Atom or RSS"), as phpBB.com does by
-hand in its forum descriptions.
+description ("Subscribe to the feed, available in Atom or RSS format."),
+as phpBB.com does by hand in its forum descriptions.
 
 The existing events in the row do not fit:
 - forumlist_body_forum_image_before/prepend/append/after sit before the

event/listener.php

@@ -88,21 +88,22 @@ public static function getSubscribedEvents()
 	}
 
 	/**
-	 * Tell the page head whether to add RSS links for the board-wide feeds
+	 * Tell the page head whether to add RSS links for the board-wide feeds,
+	 * and load the language its RSS links use
 	 *
 	 * @return void
 	 */
 	public function add_rss_head_switch()
 	{
+		$this->load_language();
 		$this->template->assign_var('S_FEEDLINKS_RSS_HEAD', (bool) $this->config['feedlinks_rss_head']);
 	}
 
 	/**
 	 * Add a forum's feed links below its description on forum lists
 	 *
 	 * phpBB has no template event after the description of a forum row (see
-	 * docs/events/), so the rendered links are appended to FORUM_DESC. The
-	 * URLs are also given to the row for styles that place them elsewhere.
+	 * docs/events/), so the rendered links are appended to FORUM_DESC.
 	 *
 	 * @param data $event
 	 * @return void
@@ -117,8 +118,6 @@ public function add_forumlist_links(data $event)
 		}
 
 		$forum_row = $event['forum_row'];
-		$forum_row['U_FEEDLINKS_ATOM'] = $urls['atom'];
-		$forum_row['U_FEEDLINKS_RSS'] = $urls['rss'];
 		$forum_row['FORUM_DESC'] .= $this->render_links($urls, $event['row']['forum_name'], true, $forum_row['FORUM_DESC'] !== '');
 		$event['forum_row'] = $forum_row;
 	}
@@ -139,7 +138,6 @@ public function add_viewforum_links(data $event)
 		}
 
 		$this->template->assign_vars([
-			'U_FEEDLINKS_ATOM'		=> $urls['atom'],
 			'U_FEEDLINKS_RSS'		=> $urls['rss'],
 			'FEEDLINKS_FORUM_LINKS'	=> $this->render_links($urls, $event['forum_data']['forum_name'], false),
 		]);
@@ -193,7 +191,7 @@ public function acp_forum_initialise(data $event)
 		if ($event['action'] === 'add' && !$event['update'])
 		{
 			$forum_data = $event['forum_data'];
-			$forum_data['forum_feedlinks'] = 0;
+			$forum_data['forum_feedlinks'] = links::NONE;
 			$event['forum_data'] = $forum_data;
 		}
 	}
@@ -208,9 +206,9 @@ public function acp_forum_display(data $event)
 	{
 		$this->language->add_lang('acp_feedlinks', 'phpbbmodders/feedlinks');
 
-		$current = isset($event['forum_data']['forum_feedlinks']) ? (int) $event['forum_data']['forum_feedlinks'] : 0;
+		$current = isset($event['forum_data']['forum_feedlinks']) ? (int) $event['forum_data']['forum_feedlinks'] : links::NONE;
 		$options = [
-			0				=> 'FEEDLINKS_NONE',
+			links::NONE		=> 'FEEDLINKS_NONE',
 			links::ATOM		=> 'FEEDLINKS_ATOM',
 			links::RSS		=> 'FEEDLINKS_RSS',
 			links::BOTH		=> 'FEEDLINKS_BOTH',
@@ -266,11 +264,7 @@ public function acp_board_feed_settings(data $event)
 	 */
 	protected function render_links(array $urls, $forum_name, $in_list, $new_line = false)
 	{
-		if (!$this->language_loaded)
-		{
-			$this->language->add_lang('feedlinks', 'phpbbmodders/feedlinks');
-			$this->language_loaded = true;
-		}
+		$this->load_language();
 
 		return $this->twig->render(self::LINKS_TEMPLATE, [
 			'U_ATOM'		=> $urls['atom'],
@@ -280,4 +274,18 @@ protected function render_links(array $urls, $forum_name, $in_list, $new_line =
 			'S_NEW_LINE'	=> $new_line,
 		]);
 	}
+
+	/**
+	 * Load the front-end language file once
+	 *
+	 * @return void
+	 */
+	protected function load_language()
+	{
+		if (!$this->language_loaded)
+		{
+			$this->language->add_lang('feedlinks', 'phpbbmodders/feedlinks');
+			$this->language_loaded = true;
+		}
+	}
 }

language/en/feedlinks.php

@@ -29,4 +29,5 @@
 	'FEEDLINKS_RSS'			=> 'RSS',
 	'FEEDLINKS_ATOM_TITLE'	=> 'Atom feed - %s',
 	'FEEDLINKS_RSS_TITLE'	=> 'RSS feed - %s',
+	'FEEDLINKS_RSS_ERROR'	=> 'The RSS feed could not be created.',
 ]);

links.php

@@ -19,6 +19,9 @@
  */
 class links
 {
+	/** Forum setting: show no links */
+	const NONE = 0;
+
 	/** Forum setting bit: show the Atom link */
 	const ATOM = 1;
 
@@ -59,11 +62,11 @@ public function __construct(auth $auth, config $config, helper $helper)
 	 *
 	 * @param array $forum Forum row with forum_id, forum_type, forum_options,
 	 *                     forum_password and forum_feedlinks
-	 * @return int Bits of self::ATOM and self::RSS; 0 for no links
+	 * @return int Bits of self::ATOM and self::RSS; self::NONE for no links
 	 */
 	public function get_formats(array $forum)
 	{
-		$formats = isset($forum['forum_feedlinks']) ? ((int) $forum['forum_feedlinks'] & self::BOTH) : 0;
+		$formats = isset($forum['forum_feedlinks']) ? ((int) $forum['forum_feedlinks'] & self::BOTH) : self::NONE;
 
 		if (!$formats
 			|| !$this->config['feed_enable']
@@ -73,7 +76,7 @@ public function get_formats(array $forum)
 			|| !empty($forum['forum_password'])
 			|| !$this->auth->acl_get('f_read', (int) $forum['forum_id']))
 		{
-			return 0;
+			return self::NONE;
 		}
 
 		return $formats;

styles/all/template/event/overall_header_feeds.html

@@ -1,9 +1,9 @@
 {% if S_FEEDLINKS_RSS_HEAD %}
-	{% if S_ENABLE_FEEDS_OVERALL %}<link rel="alternate" type="application/rss+xml" title="{{ lang('FEED') }} (RSS) - {{ SITENAME }}" href="{{ path('phpbbmodders_feedlinks_rss_overall') }}">{% endif %}
-	{% if S_ENABLE_FEEDS_NEWS %}<link rel="alternate" type="application/rss+xml" title="{{ lang('FEED') }} (RSS) - {{ lang('FEED_NEWS') }}" href="{{ path('phpbbmodders_feedlinks_rss_news') }}">{% endif %}
-	{% if S_ENABLE_FEEDS_FORUMS %}<link rel="alternate" type="application/rss+xml" title="{{ lang('FEED') }} (RSS) - {{ lang('ALL_FORUMS') }}" href="{{ path('phpbbmodders_feedlinks_rss_forums') }}">{% endif %}
-	{% if S_ENABLE_FEEDS_TOPICS %}<link rel="alternate" type="application/rss+xml" title="{{ lang('FEED') }} (RSS) - {{ lang('FEED_TOPICS_NEW') }}" href="{{ path('phpbbmodders_feedlinks_rss_topics') }}">{% endif %}
-	{% if S_ENABLE_FEEDS_TOPICS_ACTIVE %}<link rel="alternate" type="application/rss+xml" title="{{ lang('FEED') }} (RSS) - {{ lang('FEED_TOPICS_ACTIVE') }}" href="{{ path('phpbbmodders_feedlinks_rss_topics_active') }}">{% endif %}
+	{% if S_ENABLE_FEEDS_OVERALL %}<link rel="alternate" type="application/rss+xml" title="{{ lang('FEED')|e }} ({{ lang('FEEDLINKS_RSS')|e }}) - {{ SITENAME }}" href="{{ path('phpbbmodders_feedlinks_rss_overall') }}">{% endif %}
+	{% if S_ENABLE_FEEDS_NEWS %}<link rel="alternate" type="application/rss+xml" title="{{ lang('FEED')|e }} ({{ lang('FEEDLINKS_RSS')|e }}) - {{ lang('FEED_NEWS')|e }}" href="{{ path('phpbbmodders_feedlinks_rss_news') }}">{% endif %}
+	{% if S_ENABLE_FEEDS_FORUMS %}<link rel="alternate" type="application/rss+xml" title="{{ lang('FEED')|e }} ({{ lang('FEEDLINKS_RSS')|e }}) - {{ lang('ALL_FORUMS')|e }}" href="{{ path('phpbbmodders_feedlinks_rss_forums') }}">{% endif %}
+	{% if S_ENABLE_FEEDS_TOPICS %}<link rel="alternate" type="application/rss+xml" title="{{ lang('FEED')|e }} ({{ lang('FEEDLINKS_RSS')|e }}) - {{ lang('FEED_TOPICS_NEW')|e }}" href="{{ path('phpbbmodders_feedlinks_rss_topics') }}">{% endif %}
+	{% if S_ENABLE_FEEDS_TOPICS_ACTIVE %}<link rel="alternate" type="application/rss+xml" title="{{ lang('FEED')|e }} ({{ lang('FEEDLINKS_RSS')|e }}) - {{ lang('FEED_TOPICS_ACTIVE')|e }}" href="{{ path('phpbbmodders_feedlinks_rss_topics_active') }}">{% endif %}
 {% endif %}
-{% if U_FEEDLINKS_RSS %}<link rel="alternate" type="application/rss+xml" title="{{ lang('FEED') }} (RSS) - {{ lang('FORUM') }} - {{ FORUM_NAME }}" href="{{ U_FEEDLINKS_RSS }}">{% endif %}
-{% if U_FEEDLINKS_RSS_TOPIC_HEAD %}<link rel="alternate" type="application/rss+xml" title="{{ lang('FEED') }} (RSS) - {{ lang('TOPIC') }} - {{ TOPIC_TITLE }}" href="{{ U_FEEDLINKS_RSS_TOPIC_HEAD }}">{% endif %}
+{% if U_FEEDLINKS_RSS %}<link rel="alternate" type="application/rss+xml" title="{{ lang('FEED')|e }} ({{ lang('FEEDLINKS_RSS')|e }}) - {{ lang('FORUM')|e }} - {{ FORUM_NAME }}" href="{{ U_FEEDLINKS_RSS }}">{% endif %}
+{% if U_FEEDLINKS_RSS_TOPIC_HEAD %}<link rel="alternate" type="application/rss+xml" title="{{ lang('FEED')|e }} ({{ lang('FEEDLINKS_RSS')|e }}) - {{ lang('TOPIC')|e }} - {{ TOPIC_TITLE }}" href="{{ U_FEEDLINKS_RSS_TOPIC_HEAD }}">{% endif %}

styles/all/template/feedlinks_links.html

@@ -3,8 +3,8 @@
    links follow the description inline, after a line break (S_NEW_LINE) when there is one, as the moderators
    and subforums lines do. #}
 {% set feedlinks_icon %}<svg class="feedlinks-icon" viewBox="0 0 16 16" width="14" height="14" aria-hidden="true" focusable="false"><rect width="16" height="16" rx="3" fill="#f26522"/><circle cx="4.5" cy="11.5" r="1.6" fill="#fff"/><path d="M3 7.1a5.9 5.9 0 0 1 5.9 5.9h-1.9A4 4 0 0 0 3 9zM3 3.4a9.6 9.6 0 0 1 9.6 9.6h-1.9A7.7 7.7 0 0 0 3 5.3z" fill="#fff"/></svg>{% endset %}
-{% set feedlinks_atom %}{% if U_ATOM %}<a class="feedlinks-link" href="{{ U_ATOM }}" type="application/atom+xml" title="{{ lang('FEEDLINKS_ATOM_TITLE', FORUM_NAME) }}">{{ feedlinks_icon }}{{ lang('FEEDLINKS_ATOM') }}<span class="sr-only"> – {{ FORUM_NAME }}</span></a>{% endif %}{% endset %}
-{% set feedlinks_rss %}{% if U_RSS %}<a class="feedlinks-link" href="{{ U_RSS }}" type="application/rss+xml" title="{{ lang('FEEDLINKS_RSS_TITLE', FORUM_NAME) }}">{{ feedlinks_icon }}{{ lang('FEEDLINKS_RSS') }}<span class="sr-only"> – {{ FORUM_NAME }}</span></a>{% endif %}{% endset %}
+{% set feedlinks_atom %}{% if U_ATOM %}<a class="feedlinks-link" href="{{ U_ATOM }}" type="application/atom+xml" title="{{ lang('FEEDLINKS_ATOM_TITLE')|e|format(FORUM_NAME) }}">{{ feedlinks_icon }}{{ lang('FEEDLINKS_ATOM') }}<span class="sr-only"> – {{ FORUM_NAME }}</span></a>{% endif %}{% endset %}
+{% set feedlinks_rss %}{% if U_RSS %}<a class="feedlinks-link" href="{{ U_RSS }}" type="application/rss+xml" title="{{ lang('FEEDLINKS_RSS_TITLE')|e|format(FORUM_NAME) }}">{{ feedlinks_icon }}{{ lang('FEEDLINKS_RSS') }}<span class="sr-only"> – {{ FORUM_NAME }}</span></a>{% endif %}{% endset %}
 {% if S_NEW_LINE %}<br />{% endif %}<span class="feedlinks{% if S_IN_LIST %} feedlinks-list{% else %} feedlinks-page{% endif %}">
 	{%- if U_ATOM and U_RSS -%}
 		{{ lang('FEEDLINKS_AVAILABLE_BOTH', feedlinks_atom, feedlinks_rss) }}

tests/event/listener_test.php

@@ -121,11 +121,7 @@ public function test_forumlist_links()
 		]);
 		$this->get_listener()->add_forumlist_links($event);
 
-		$this->assertSame([
-			'FORUM_DESC'		=> 'Description<links>',
-			'U_FEEDLINKS_ATOM'	=> 'atom-url',
-			'U_FEEDLINKS_RSS'	=> '',
-		], $event['forum_row']);
+		$this->assertSame(['FORUM_DESC' => 'Description<links>'], $event['forum_row']);
 	}
 
 	public function test_forumlist_without_links()