Fail nicely when unparsable xml is being passed (closes #1327)

modules/admin/lib/Controller/Federation.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace SimpleSAML\Module\admin\Controller;
 
+use Exception;
 use SimpleSAML\Auth;
 use SimpleSAML\Configuration;
 use SimpleSAML\HTTP\RunnableResponse;
@@ -354,39 +355,48 @@ class Federation
             $xmldata = trim($xmldata);
         }
 
+        $error = null;
         if (!empty($xmldata)) {
             Utils\XML::checkSAMLMessage($xmldata, 'saml-meta');
-            $entities = SAMLParser::parseDescriptorsString($xmldata);
-
-            // get all metadata for the entities
-            foreach ($entities as &$entity) {
-                $entity = [
-                    'saml20-sp-remote'  => $entity->getMetadata20SP(),
-                    'saml20-idp-remote' => $entity->getMetadata20IdP(),
-                ];
+
+            $entities = null;
+            try {
+                $entities = SAMLParser::parseDescriptorsString($xmldata);
+            } catch (Exception $e) {
+                $error = $e->getMessage();
             }
 
-            // transpose from $entities[entityid][type] to $output[type][entityid]
-            $output = Utils\Arrays::transpose($entities);
+            if ($entities !== null) {
+                // get all metadata for the entities
+                foreach ($entities as &$entity) {
+                    $entity = [
+                        'saml20-sp-remote'  => $entity->getMetadata20SP(),
+                        'saml20-idp-remote' => $entity->getMetadata20IdP(),
+                    ];
+                }
 
-            // merge all metadata of each type to a single string which should be added to the corresponding file
-            foreach ($output as $type => &$entities) {
-                $text = '';
-                foreach ($entities as $entityId => $entityMetadata) {
-                    if ($entityMetadata === null) {
-                        continue;
+                // transpose from $entities[entityid][type] to $output[type][entityid]
+                $output = Utils\Arrays::transpose($entities);
+
+                // merge all metadata of each type to a single string which should be added to the corresponding file
+                foreach ($output as $type => &$entities) {
+                    $text = '';
+                    foreach ($entities as $entityId => $entityMetadata) {
+                        if ($entityMetadata === null) {
+                            continue;
+                        }
+
+                        /**
+                         * remove the entityDescriptor element because it is unused,
+                         * and only makes the output harder to read
+                         */
+                        unset($entityMetadata['entityDescriptor']);
+
+                        $text .= '$metadata[' . var_export($entityId, true) . '] = '
+                            . VarExporter::export($entityMetadata) . ";\n";
                     }
-
-                    /**
-                     * remove the entityDescriptor element because it is unused,
-                     * and only makes the output harder to read
-                     */
-                    unset($entityMetadata['entityDescriptor']);
-
-                    $text .= '$metadata[' . var_export($entityId, true) . '] = '
-                        . VarExporter::export($entityMetadata) . ";\n";
+                    $entities = $text;
                 }
-                $entities = $text;
             }
         } else {
             $xmldata = '';
@@ -398,6 +408,7 @@ class Federation
             'logouturl' => Utils\Auth::getAdminLogoutURL(),
             'xmldata' => $xmldata,
             'output' => $output,
+            'error' => $error,
         ];
 
         $this->menu->addOption('logout', $t->data['logouturl'], Translate::noop('Log out'));

modules/admin/templates/metadata_converter.twig

@@ -47,6 +47,14 @@
             <br><br>
             {%- set i=i+1 %}
         {%- endfor -%}
+    {% elseif error is not null %}
+    <br>
+    <h2 id="error">{{ 'An error occured'|trans }}</h2>
+    <div class="code-box">
+        <div class="code-box-content">
+            <pre id="error" class="fa-warning">{{ error }}</pre>
+        </div>
+    </div>
     {% endif -%}
 {% endblock content -%}
 {% block postload %}