Merge pull request #53 from dignat/master

Filter invalid characters from basket item names, handle multiple discounts and no discounts.
thephpleague · Feb 7, 2016 · ca992b2 · ca992b2
2 parents 4208d23 + cf4795f
commit ca992b2
Show file tree

Hide file tree

Showing 2 changed files with 119 additions and 7 deletions.
diff --git a/src/Message/AbstractRequest.php b/src/Message/AbstractRequest.php
@@ -144,6 +144,46 @@ protected function createResponse($data)
         return $this->response = new Response($this, $data);
     }
 
+    /**
+     * Filters out any characters that SagePay does not support from the item name.
+     *
+     * Believe it or not, SagePay actually have separate rules for allowed characters
+     * for item names and discount names, hence the need for two separate methods.
+     *
+     * @param string $name
+     *
+     * @return string
+     */
+    protected function filterItemName($name)
+    {
+        $standardChars = "0-9a-zA-Z";
+        $allowedSpecialChars = " +'/\\&:,.-{}";
+        $pattern = '`[^'.$standardChars.preg_quote($allowedSpecialChars, '/').']`';
+        $name = trim(substr(preg_replace($pattern, '', $name), 0, 100));
+
+        return $name;
+    }
+
+    /**
+     * Filters out any characters that SagePay does not support from the discount name.
+     *
+     * Believe it or not, SagePay actually have separate rules for allowed characters
+     * for item names and discount names, hence the need for two separate methods.
+     *
+     * @param string $name
+     *
+     * @return string
+     */
+    protected function filterDiscountName($name)
+    {
+        $standardChars = "0-9a-zA-Z";
+        $allowedSpecialChars = " +'/\\:,.-{};_@()^\"~[]$=!#?|";
+        $pattern = '`[^'.$standardChars.preg_quote($allowedSpecialChars, '/').']`';
+        $name = trim(substr(preg_replace($pattern, '', $name), 0, 100));
+
+        return $name;
+    }
+
     /**
      * Get an XML representation of the current cart items
      *
@@ -160,27 +200,33 @@ protected function getItemData()
         }
 
         $xml = new \SimpleXMLElement('<basket/>');
+        $cartHasDiscounts = false;
 
         foreach ($items as $basketItem) {
             if ($basketItem->getPrice() < 0) {
-                $discounts = $xml->addChild('discounts');
-                $discount = $discounts->addChild('discount');
-                $discount->addChild('fixed', $basketItem->getPrice() * -1);
-                $discount->addChild('description', $basketItem->getName());
+                $cartHasDiscounts = true;
             } else {
                 $total = ($basketItem->getQuantity() * $basketItem->getPrice());
                 $item = $xml->addChild('item');
-                $item->addChild('description', $basketItem->getName());
+                $item->description = $this->filterItemName($basketItem->getName());
                 $item->addChild('quantity', $basketItem->getQuantity());
                 $item->addChild('unitNetAmount', $basketItem->getPrice());
                 $item->addChild('unitTaxAmount', '0.00');
                 $item->addChild('unitGrossAmount', $basketItem->getPrice());
                 $item->addChild('totalGrossAmount', $total);
             }
         }
-
+        if ($cartHasDiscounts) {
+            $discounts = $xml->addChild('discounts');
+            foreach ($items as $discountItem) {
+                if ($discountItem->getPrice() < 0) {
+                    $discount = $discounts->addChild('discount');
+                    $discount->addChild('fixed', ($discountItem->getPrice() * $discountItem->getQuantity()) * -1);
+                    $discount->description = $this->filterDiscountName($discountItem->getName());
+                }
+            }
+        }
         $xmlString = $xml->asXML();
-
         if ($xmlString) {
             $result = $xmlString;
         }

diff --git a/tests/Message/DirectAuthorizeRequestTest.php b/tests/Message/DirectAuthorizeRequestTest.php
@@ -6,6 +6,11 @@
 
 class DirectAuthorizeRequestTest extends TestCase
 {
+    /**
+     * @var \Omnipay\Common\Message\AbstractRequest $request
+     */
+    protected $request;
+
     public function setUp()
     {
         parent::setUp();
@@ -60,6 +65,7 @@ public function testNoBasket()
 
         // Then with a basket containing no items.
         $items = new \Omnipay\Common\ItemBag(array());
+        $this->request->setItems($items);
         $data = $this->request->getData();
         $this->assertArrayNotHasKey('BasketXML', $data);
     }
@@ -182,4 +188,64 @@ public function testGetDataNullBillingAddress2()
         // converted to an empty string. I'm not clear how that would be
         // tested.
     }
+
+    public function testBasketWithNoDiscount()
+    {
+        $items = new \Omnipay\Common\ItemBag(array(
+            new \Omnipay\Common\Item(array(
+                'name' => 'Name',
+                'description' => 'Description',
+                'quantity' => 1,
+                'price' => 1.23,
+            ))
+        ));
+
+        $this->request->setItems($items);
+        $data = $this->request->getData();
+        // The element does exist, and must contain the basket XML, with optional XML header and
+        // trailing newlines.
+        $this->assertArrayHasKey('BasketXML', $data);
+        $this->assertNotContains('<discount>', $data['BasketXML']);
+    }
+
+    public function testMixedBasketWithSpecialChars()
+    {
+        $items = new \Omnipay\Common\ItemBag(array(
+            new \Omnipay\Common\Item(array(
+                'name' => "Denisé's Odd & Wierd £name? #12345678901234567890123456789012345678901234567890123456789012345678901234567890",
+                'description' => 'Description',
+                'quantity' => 2,
+                'price' => 4.23,
+            )),
+            array(
+                'name' => "Denisé's \"Odd\" & Wierd £discount? #",
+                'description' => 'My Offer',
+                'quantity' => 2,
+                'price' => -0.10,
+            ),
+            array(
+                // 101 character name
+                'name' => '12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901',
+                'description' => 'My 2nd Offer',
+                'quantity' => 1,
+                'price' => -1.60,
+            )
+        ));
+
+        // Names/descriptions should be max 100 characters in length, once invalid characters have been removed.
+        $expected = '<basket><item>'
+            . '<description>Denis\'s Odd &amp; Wierd name 123456789012345678901234567890123456789012345678901234567890123456789012345</description><quantity>2</quantity>'
+            . '<unitNetAmount>4.23</unitNetAmount><unitTaxAmount>0.00</unitTaxAmount>'
+            . '<unitGrossAmount>4.23</unitGrossAmount><totalGrossAmount>8.46</totalGrossAmount>'
+            . '</item><discounts>'
+            . '<discount><fixed>0.2</fixed><description>Denis\'s "Odd"  Wierd discount? #</description></discount>'
+            . '<discount><fixed>1.6</fixed><description>1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890</description></discount>'
+            . '</discounts></basket>';
+
+        $this->request->setItems($items);
+        $data = $this->request->getData();
+
+        $this->assertArrayHasKey('BasketXML', $data);
+        $this->assertContains($expected, $data['BasketXML'], 'Basket XML does not match the expected output');
+    }
 }