Improve OPML conformance

NEWS.md

@@ -43,6 +43,9 @@
 - Fixed missing styling in article contents ([#1221](https://github.com/fossar/selfoss/pull/1221))
 - Golem, Lightreading and Heise spouts now use Graby for extracting article contents instead of our own defunct extraction rules. ([#1245](https://github.com/fossar/selfoss/pull/1245))
 - The tag colour picker now pre-selects the current colour instead of a placeholder colour. ([#1269](https://github.com/fossar/selfoss/pull/1269))
+- OPML import now correctly handles valid files. ([#1366](https://github.com/fossar/selfoss/pull/1366))
+- OPML import will prefer `title` attribute over text for feed names. ([#1366](https://github.com/fossar/selfoss/pull/1366))
+- OPML import is now able to read files when the browser sends an incorrect MIME type. ([#1366](https://github.com/fossar/selfoss/pull/1366))
 
 ### API changes
 - `tags` attribute is now consistently array of strings, numbers are numbers and booleans are booleans. **This might break third-party clients that have not updated yet.** ([#948](https://github.com/fossar/selfoss/pull/948))

assets/js/opml.js

@@ -2,6 +2,14 @@ const form = document.querySelector('form');
 const msgContainer = document.querySelector('.message-container');
 const submit = document.querySelector('input[type=submit]');
 
+function escapeHtml(text) {
+    return text.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;');
+}
+
+function listMessages(messages) {
+    return (messages ?? []).map(escapeHtml).join('<br>');
+}
+
 form.addEventListener('submit', e => {
     e.preventDefault();
 
@@ -19,17 +27,17 @@ form.addEventListener('submit', e => {
     fetch(request).then(response => {
         return response.json().then(({messages}) => {
             if (response.status === 200) {
-                msgContainer.innerHTML = `<p class="msg success">${messages.join('<br>')} You might want to <a href="update">update now</a> or <a href="./">view your feeds</a>.</p>`;
+                msgContainer.innerHTML = `<p class="msg success">${listMessages(messages)} You might want to <a href="update">update now</a> or <a href="./">view your feeds</a>.</p>`;
             } else if (response.status === 202) {
-                msgContainer.innerHTML = `<p class="msg error">The following feeds could not be imported:<br>${messages.join('<br>')}</p>`;
+                msgContainer.innerHTML = `<p class="msg error">The following feeds could not be imported:<br>${listMessages(messages)}</p>`;
             } else if (response.status === 400) {
-                msgContainer.innerHTML = `<p class="msg error">There was a problem importing your OPML file:<br>${messages.join('<br>')}</p>`;
+                msgContainer.innerHTML = `<p class="msg error">There was a problem importing your OPML file:<br>${listMessages(messages)}</p>`;
             } else {
-                msgContainer.innerHTML = `<p class="msg error">Unexpected happened. <details><pre>${JSON.stringify(messages)}</pre></details></p>`;
+                msgContainer.innerHTML = `<p class="msg error">Unexpected happened. <details><pre>${escapeHtml(JSON.stringify(messages))}</pre></details></p>`;
             }
         });
     }).catch(err => {
-        msgContainer.innerHTML = `<p class="msg error">Unexpected happened. <details><pre>${JSON.stringify(err)}</pre></details></p>`;
+        msgContainer.innerHTML = `<p class="msg error">Unexpected happened. <details><pre>${escapeHtml(JSON.stringify(err))}</pre></details></p>`;
     }).finally(() => {
         submit.disabled = false;
         submit.value = originalButtonCaption;

src/controllers/Opml/Import.php

@@ -63,17 +63,34 @@ public function add() {
             if ($opml['error'] === UPLOAD_ERR_NO_FILE) {
                 throw new \Exception('No file uploaded!');
             }
-            if (!in_array($opml['type'], ['application/xml', 'text/xml', 'text/x-opml+xml', 'text/x-opml'], true)) {
-                throw new \Exception('Unsupported file type: ' . $opml['type']);
-            }
 
             $this->logger->debug('start OPML import ');
 
             if (!function_exists('simplexml_load_file')) {
                 throw new \Exception('Missing SimpleXML PHP extension. Please install/enable it as described on https://www.php.net/manual/en/simplexml.installation.php');
             }
 
-            $subs = simplexml_load_file($opml['tmp_name']);
+            $subs = false;
+            try {
+                $previousUseErrors = libxml_use_internal_errors(true);
+
+                $subs = simplexml_load_file($opml['tmp_name']);
+
+                if ($subs === false) {
+                    // When parsing fails, check MIME type supplied by browser since it is possible user supplied file of a wrong type.
+                    if (!in_array($opml['type'], ['application/xml', 'text/xml', 'text/x-opml+xml', 'text/x-opml'], true)) {
+                        throw new \Exception('Unsupported file type: ' . $opml['type']);
+                    }
+
+                    // If type is correct, check the error reported by parser.
+                    $error = libxml_get_last_error();
+                    $errorDetail = $error !== false ? ': ' . $error->message : '';
+
+                    throw new \Exception('Unable to parse OPML file' . $errorDetail);
+                }
+            } finally {
+                libxml_use_internal_errors($previousUseErrors);
+            }
             $errors = $this->processGroup($subs->body);
 
             // cleanup tags
@@ -120,8 +137,13 @@ private function processGroup(SimpleXMLElement $xml, array $tags = []) {
 
         $xml->registerXPathNamespace('selfoss', 'https://selfoss.aditu.de/');
 
-        // tags are the words of the outline parent
+        // In Google Reader (and now Feedly), folders/tags/labels were just the text of the outline parent.
+        // Now, it is not valid for an <outline> element with the default “text” type to use the “title” attribute
+        // but both Google Reader and Feedly duplicate the “text” attribute as “title” so it seems to be common.
+        // Feedly seems to prefer “title” for both category names and feed names.
+        // We will do the same in case someone mistakenly exports the “title” and forgets about “text”.
         $title = (string) $xml->attributes()->title;
+        $title = $title ?: (string) $xml->attributes()->text;
         if ($title !== '' && $title !== '/') {
             $tags[] = $title;
             // for new tags, try to import tag color, otherwise use random color
@@ -169,9 +191,11 @@ private function addSubscription(SimpleXMLElement $xml, array $tags) {
         $nsattrs = $xml->attributes('selfoss', true);
 
         // description
-        $title = (string) $attrs->text;
+        // Google Reader (and now Feedly) duplicate the feed title in “title” and “text” attributes.
+        // Prefer “title” as it is optional and it might contain more detailed label.
+        $title = (string) $attrs->title;
         if ($title === '') {
-            $title = (string) $attrs->title;
+            $title = (string) $attrs->text;
         }
 
         // RSS URL