Refactor soap12:Body

simplesamlphp · Jan 23, 2024 · 5ac6565 · 5ac6565
1 parent b1bcd15
commit 5ac6565
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 60 deletions.
diff --git a/src/SOAP12/XML/env/Body.php b/src/SOAP12/XML/env/Body.php
@@ -35,55 +35,37 @@ final class Body extends AbstractSoapElement
     /** The namespace-attribute for the xs:anyAttribute element */
     public const XS_ANY_ATTR_NAMESPACE = NS::OTHER;
 
-    /**
-     * @var \SimpleSAML\SOAP12\XML\env\Fault|null
-     */
-    protected ?Fault $fault;
-
 
     /**
      * Initialize a soap:Body
      *
+     * @param \SimpleSAML\SOAP12\XML\Fault|null $fault
      * @param \SimpleSAML\XML\ElementInterface[] $children
      * @param list<\SimpleSAML\XML\Attribute> $namespacedAttributes
      */
-    public function __construct(array $children = [], array $namespacedAttributes = [])
-    {
-        /**
-         * 5.4: To be recognized as carrying SOAP error information, a SOAP message MUST contain a single SOAP Fault
-         *      element information item as the only child element information item of the SOAP Body .
-         */
-        $fault = array_values(array_filter($children, function ($elt) {
-            return $elt instanceof Fault;
-        }));
-        Assert::maxCount($fault, 1, ProtocolViolationException::class);
-
-        /**
-         * 5.4: When generating a fault, SOAP senders MUST NOT include additional element
-         *      information items in the SOAP Body .
-         */
-        $children = array_diff($children, $fault);
-        Assert::false(
-            !empty($fault) && !empty($children),
-            "When generating a fault, SOAP senders MUST NOT include additional elements in the SOAP Body.",
-            ProtocolViolationException::class,
-        );
+    public function __construct(
+        protected ?Fault $fault = null,
+        array $children = [],
+        array $namespacedAttributes = []
+    ) {
+        if ($fault !== null) {
+            /**
+             * 5.4: When generating a fault, SOAP senders MUST NOT include additional element
+             *      information items in the SOAP Body .
+             */
+            Assert::isEmpty(
+                $children,
+                "When generating a fault, SOAP senders MUST NOT include additional elements in the SOAP Body.",
+                ProtocolViolationException::class,
+            );
+        }
+        Assert::allNotInstanceOf($children, Fault::class, ProtocolViolationException::class);
 
-        $this->setFault(array_pop($fault));
         $this->setElements($children);
         $this->setAttributesNS($namespacedAttributes);
     }
 
 
-    /**
-     * @param \SimpleSAML\SOAP12\XML\env\Fault|null $fault
-     */
-    public function setFault(?Fault $fault): void
-    {
-        $this->fault = $fault;
-    }
-
-
     /**
      * @return \SimpleSAML\SOAP12\XML\env\Fault|null
      */
@@ -118,20 +100,27 @@ public static function fromXML(DOMElement $xml): static
         Assert::same($xml->localName, 'Body', InvalidDOMElementException::class);
         Assert::same($xml->namespaceURI, Body::NS, InvalidDOMElementException::class);
 
-        $children = [];
+        $children = $fault = [];
         foreach ($xml->childNodes as $child) {
             if (!($child instanceof DOMElement)) {
                 continue;
             } elseif ($child->namespaceURI === C::NS_SOAP_ENV_12) {
                 if ($child->localName === 'Fault') {
-                    Fault::fromXML($child);
+                    $fault = Fault::fromXML($child);
                     continue;
                 }
             }
             $children[] = new Chunk($child);
         }
 
+        /**
+         * 5.4: To be recognized as carrying SOAP error information, a SOAP message MUST contain a single SOAP Fault
+         *      element information item as the only child element information item of the SOAP Body .
+         */
+        Assert::maxCount($fault, 1, ProtocolViolationException::class);
+
         return new static(
+            array_pop($fault),
             $children,
             self::getAttributesNSFromXML($xml)
         );

diff --git a/tests/SOAP12/XML/env/BodyTest.php b/tests/SOAP12/XML/env/BodyTest.php
@@ -63,7 +63,7 @@ public function testMarshalling(): void
     {
         $domAttr = new Attribute('urn:test:something', 'test', 'attr1', 'testval1');
 
-        $body = new Body([new Chunk(self::$BodyContent)], [$domAttr]);
+        $body = new Body(null, [new Chunk(self::$BodyContent)], [$domAttr]);
         $this->assertFalse($body->isEmptyElement());
 
         $this->assertEquals(
@@ -77,7 +77,7 @@ public function testMarshalling(): void
      */
     public function testMarshallingWithNoContent(): void
     {
-        $body = new Body([], []);
+        $body = new Body(null, [], []);
         $this->assertEquals(
             '<env:Body xmlns:env="http://www.w3.org/2003/05/soap-envelope/"/>',
             strval($body)
@@ -86,31 +86,14 @@ public function testMarshallingWithNoContent(): void
     }
 
 
-    /**
-     */
-    public function testMarshallingWithMultipleFaults(): void
-    {
-        $this->expectException(ProtocolViolationException::class);
-        new Body(
-            [
-                new Fault(new Code(new Value('env:Sender')), new Reason([new Text('en', 'Something is wrong')])),
-                new Fault(new Code(new Value('env:Sender')), new Reason([new Text('en', 'It is broken')])),
-            ],
-            [],
-        );
-    }
-
-
     /**
      */
     public function testMarshallingWithFaultAndContent(): void
     {
         $this->expectException(ProtocolViolationException::class);
         new Body(
-            [
-                new Fault(new Code(new Value('env:Sender')), new Reason([new Text('en', 'Something is wrong')])),
-                new Chunk(self::$BodyContent),
-            ],
+            new Fault(new Code(new Value('env:Sender')), new Reason([new Text('en', 'Something is wrong')])),
+            [new Chunk(self::$BodyContent)],
             [],
         );
     }

diff --git a/tests/SOAP12/XML/env/EnvelopeTest.php b/tests/SOAP12/XML/env/EnvelopeTest.php
@@ -65,7 +65,7 @@ public function testMarshalling(): void
     {
         $domAttr = new Attribute('urn:test:something', 'test', 'attr1', 'testval1');
 
-        $body = new Body([new Chunk(self::$bodyContent)], [$domAttr]);
+        $body = new Body(null, [new Chunk(self::$bodyContent)], [$domAttr]);
         $header = new Header([new Chunk(self::$headerContent)], [$domAttr]);
 
         $envelope = new Envelope($body, $header, [$domAttr]);