Skip to content

Commit 42e2551

Browse files
bubasumabubasuma
committed
MC-39895: PayPal PayflowPro redirect Parameter list format error
- Fix checkout with Credit Card (Payflow Pro) fails if billing address has special characters (&, =)
1 parent ad29452 commit 42e2551

File tree

2 files changed

+217
-28
lines changed

2 files changed

+217
-28
lines changed

‎app/code/Magento/Paypal/Model/Payflow/Service/Gateway.php

Lines changed: 63 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,8 @@ public function postRequest(DataObject $request, ConfigInterface $config)
8585
);
8686
$client->setConfig($clientConfig);
8787
$client->setMethod(\Zend_Http_Client::POST);
88-
$client->setParameterPost($request->getData());
88+
$requestData = $this->prepareRequestData($request->getData());
89+
$client->setParameterPost($requestData);
8990
$client->setHeaders(
9091
[
9192
'X-VPS-VIT-CLIENT-CERTIFICATION-ID' => '33baf5893fc2123d8b191d2d011b7fdc',
@@ -97,9 +98,7 @@ public function postRequest(DataObject $request, ConfigInterface $config)
9798

9899
try {
99100
$response = $client->request();
100-
101-
$responseArray = [];
102-
parse_str(strstr($response->getBody(), 'RESULT'), $responseArray);
101+
$responseArray = $this->parseNVP(strstr($response->getBody(), 'RESULT'));
103102

104103
$result->setData(array_change_key_case($responseArray, CASE_LOWER));
105104
$result->setData('result_code', $result->getData('result'));
@@ -115,7 +114,7 @@ public function postRequest(DataObject $request, ConfigInterface $config)
115114
} finally {
116115
$this->logger->debug(
117116
[
118-
'request' => $request->getData(),
117+
'request' => $requestData,
119118
'result' => $result->getData()
120119
],
121120
(array)$config->getValue('getDebugReplacePrivateDataKeys'),
@@ -125,4 +124,63 @@ public function postRequest(DataObject $request, ConfigInterface $config)
125124

126125
return $result;
127126
}
127+
128+
/**
129+
* Add length tag to parameters name which contains special characters: =, &
130+
*
131+
* The length tag specifies the exact number of characters and spaces (number of bytes) that appear in the value
132+
* eg ['COMPANYNAME[14]' => 'Ruff & Johnson')]
133+
*
134+
* @param array $data
135+
* @return array
136+
*/
137+
private function prepareRequestData(array $data): array
138+
{
139+
$requestData = [];
140+
foreach ($data as $k => $v) {
141+
if (strpos($v, '&') !== false || strpos($v, '=') !== false) {
142+
$requestData[$k . '[' . strlen($v) . ']'] = $v;
143+
} else {
144+
$requestData[$k] = $v;
145+
}
146+
}
147+
return $requestData;
148+
}
149+
150+
/**
151+
* Parse NVP string into array
152+
*
153+
* Use length tag (if present) to parse the key value.
154+
*
155+
* The length tag specifies the exact number of characters and spaces (number of bytes) that appear in the value
156+
* e.g COMPANYNAME[14]=Ruff & Johnson
157+
* e.g COMMENT1[7]=Level=5
158+
*
159+
* @param string $nvp
160+
* @return array
161+
*/
162+
private function parseNVP(string $nvp): array
163+
{
164+
$result = [];
165+
while (strlen($nvp) > 0) {
166+
$keyPos = strpos($nvp, '=');
167+
if ($keyPos !== false) {
168+
$key = substr($nvp, 0, $keyPos);
169+
if (preg_match('/\[(\d+)]$/', $key, $keyParts)) {
170+
$valueLength = (int) $keyParts[1];
171+
$key = substr($key, 0, strpos($key, '['));
172+
$result[$key] = substr($nvp, $keyPos + 1, $valueLength);
173+
$valuePos = $keyPos + 1 + $valueLength;
174+
} else {
175+
$valuePos = strpos($nvp, '&') ? strpos($nvp, '&') : strlen($nvp);
176+
$value = substr($nvp, $keyPos + 1, $valuePos - $keyPos - 1);
177+
$result[$key] = $value;
178+
}
179+
$nvp = substr($nvp, $valuePos + 1);
180+
} else {
181+
$nvp = '';
182+
}
183+
}
184+
return $result;
185+
}
128186
}

‎app/code/Magento/Paypal/Test/Unit/Model/Payflow/Service/GatewayTest.php

Lines changed: 154 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,27 +17,43 @@
1717
use PHPUnit\Framework\MockObject\MockObject;
1818
use PHPUnit\Framework\TestCase;
1919
use Psr\Log\LoggerInterface;
20+
use ReflectionMethod;
21+
use Zend_Http_Client_Exception;
22+
use Zend_Http_Response;
2023

2124
/**
2225
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
2326
*/
2427
class GatewayTest extends TestCase
2528
{
26-
/** @var Gateway|MockObject */
27-
protected $object;
28-
29-
/** @var ZendClientFactory|MockObject */
30-
protected $httpClientFactoryMock;
31-
32-
/** @var Random|MockObject */
33-
protected $mathRandomMock;
34-
35-
/** @var Logger|MockObject */
36-
protected $loggerMock;
37-
38-
/** @var ZendClient|MockObject */
39-
protected $zendClientMock;
40-
29+
/**
30+
* @var Gateway|MockObject
31+
*/
32+
private $object;
33+
34+
/**
35+
* @var ZendClientFactory|MockObject
36+
*/
37+
private $httpClientFactoryMock;
38+
39+
/**
40+
* @var Random|MockObject
41+
*/
42+
private $mathRandomMock;
43+
44+
/**
45+
* @var Logger|MockObject
46+
*/
47+
private $loggerMock;
48+
49+
/**
50+
* @var ZendClient|MockObject
51+
*/
52+
private $zendClientMock;
53+
54+
/**
55+
* @inheritdoc
56+
*/
4157
protected function setUp(): void
4258
{
4359
$this->httpClientFactoryMock = $this->getMockBuilder(ZendClientFactory::class)
@@ -66,24 +82,28 @@ protected function setUp(): void
6682
);
6783
}
6884

69-
public function testPostRequestOk()
85+
/**
86+
* @param string $nvpResponse
87+
* @param array $expectedResult
88+
* @dataProvider postRequestOkDataProvider
89+
*/
90+
public function testPostRequestOk(string $nvpResponse, array $expectedResult): void
7091
{
7192
$configMap = [
7293
['getDebugReplacePrivateDataKeys', null, ['masked']],
7394
['debug', null, true]
7495
];
75-
$expectedResponse = 'RESULT=0&RESPMSG=Approved&SECURETOKEN=8ZIaw2&SECURETOKENID=2481d53';
7696

7797
/** @var ConfigInterface|MockObject $configInterfaceMock */
7898
$configInterfaceMock = $this->getMockBuilder(ConfigInterface::class)
7999
->getMockForAbstractClass();
80-
$zendResponseMock = $this->getMockBuilder(\Zend_Http_Response::class)
100+
$zendResponseMock = $this->getMockBuilder(Zend_Http_Response::class)
81101
->setMethods(['getBody'])
82102
->disableOriginalConstructor()
83103
->getMock();
84104
$zendResponseMock->expects(static::once())
85105
->method('getBody')
86-
->willReturn($expectedResponse);
106+
->willReturn($nvpResponse);
87107
$this->zendClientMock->expects(static::once())
88108
->method('request')
89109
->willReturn($zendResponseMock);
@@ -98,8 +118,119 @@ public function testPostRequestOk()
98118

99119
$result = $this->object->postRequest($object, $configInterfaceMock);
100120

101-
static::assertInstanceOf(DataObject::class, $result);
102-
static::assertArrayHasKey('result_code', $result->getData());
121+
static::assertEquals($expectedResult, $result->toArray());
122+
}
123+
124+
/**
125+
* @return array[]
126+
*/
127+
public function postRequestOkDataProvider(): array
128+
{
129+
return [
130+
[
131+
'RESULT=0&RESPMSG=Approved&SECURETOKEN=9tl4MmP46NUadl9pwCKFgfQjA'
132+
. '&SECURETOKENID=vVWBMSNb9j0SLlYw4AbqBnKmuogtzNNC',
133+
[
134+
'result' => '0',
135+
'securetoken' => '9tl4MmP46NUadl9pwCKFgfQjA',
136+
'securetokenid' => 'vVWBMSNb9j0SLlYw4AbqBnKmuogtzNNC',
137+
'respmsg' => 'Approved',
138+
'result_code' => '0',
139+
]
140+
],
141+
[
142+
'RESULT=0&PNREF=A30A3A958244&RESPMSG=Approved&AUTHCODE=028PNI&AVSADDR=N&AVSZIP=N&HOSTCODE=A'
143+
. '&PROCAVS=N&VISACARDLEVEL=12&TRANSTIME=2020-12-16 14:43:57&FIRSTNAME[4]=Joé'
144+
. '&LASTNAME=O\'Reilly&COMPANYNAME[14]=Ruff & Johnson&COMMENT1[7]=Level=5'
145+
. '&AMT=30.00&ACCT=1111&EXPDATE=1224&CARDTYPE=0&IAVS=N',
146+
[
147+
'result' => '0',
148+
'pnref' => 'A30A3A958244',
149+
'respmsg' => 'Approved',
150+
'authcode' => '028PNI',
151+
'avsaddr' => 'N',
152+
'avszip' => 'N',
153+
'hostcode' => 'A',
154+
'procavs' => 'N',
155+
'visacardlevel' => '12',
156+
'transtime' => '2020-12-16 14:43:57',
157+
'firstname' => 'Joé',
158+
'lastname' => 'O\'Reilly',
159+
'companyname' => 'Ruff & Johnson',
160+
'comment1' => 'Level=5',
161+
'amt' => '30.00',
162+
'acct' => '1111',
163+
'expdate' => '1224',
164+
'cardtype' => '0',
165+
'iavs' => 'N',
166+
'result_code' => '0',
167+
]
168+
],
169+
];
170+
}
171+
172+
/**
173+
* @param array $requestData
174+
* @param string $requestBody
175+
* @dataProvider requestBodyDataProvider
176+
*/
177+
public function testRequestBody(array $requestData, string $requestBody): void
178+
{
179+
$configMap = [
180+
['getDebugReplacePrivateDataKeys', null, ['masked']],
181+
['debug', null, true]
182+
];
183+
184+
/** @var ConfigInterface|MockObject $configInterfaceMock */
185+
$configInterfaceMock = $this->getMockBuilder(ConfigInterface::class)
186+
->getMockForAbstractClass();
187+
$zendResponseMock = $this->getMockBuilder(Zend_Http_Response::class)
188+
->setMethods(['getBody'])
189+
->disableOriginalConstructor()
190+
->getMock();
191+
$zendResponseMock->expects(static::once())
192+
->method('getBody')
193+
->willReturn('RESULT=0&RESPMSG=Approved');
194+
$this->zendClientMock->expects(static::once())
195+
->method('request')
196+
->willReturn($zendResponseMock);
197+
198+
$configInterfaceMock->expects(static::any())
199+
->method('getValue')
200+
->willReturnMap($configMap);
201+
$this->loggerMock->expects(static::once())
202+
->method('debug');
203+
204+
$request = new DataObject($requestData);
205+
$this->object->postRequest($request, $configInterfaceMock);
206+
$method = new ReflectionMethod($this->zendClientMock, '_prepareBody');
207+
$method->setAccessible(true);
208+
$this->assertEquals($requestBody, $method->invoke($this->zendClientMock));
209+
}
210+
211+
/**
212+
* @return array[]
213+
*/
214+
public function requestBodyDataProvider(): array
215+
{
216+
return [
217+
[
218+
[
219+
'companyname' => 'Ruff & Johnson',
220+
'comment1' => 'Level=5',
221+
'shiptofirstname' => 'Joé',
222+
'shiptolastname' => 'O\'Reilly',
223+
'shiptostreet' => '4659 Rainbow Road',
224+
'shiptocity' => 'Los Angeles',
225+
'shiptostate' => 'CA',
226+
'shiptozip' => '90017',
227+
'shiptocountry' => 'US',
228+
],
229+
'companyname[14]=Ruff & Johnson&comment1[7]=Level=5&shiptofirstname=Joé&shiptolastname=O\'Reilly'
230+
. '&shiptostreet=4659 Rainbow Road&shiptocity=Los Angeles&shiptostate=CA&shiptozip=90017'
231+
. '&shiptocountry=US'
232+
]
233+
];
103234
}
104235

105236
public function testPostRequestFail()
@@ -108,15 +239,15 @@ public function testPostRequestFail()
108239
/** @var ConfigInterface|MockObject $configInterfaceMock */
109240
$configInterfaceMock = $this->getMockBuilder(ConfigInterface::class)
110241
->getMockForAbstractClass();
111-
$zendResponseMock = $this->getMockBuilder(\Zend_Http_Response::class)
242+
$zendResponseMock = $this->getMockBuilder(Zend_Http_Response::class)
112243
->setMethods(['getBody'])
113244
->disableOriginalConstructor()
114245
->getMock();
115246
$zendResponseMock->expects(static::never())
116247
->method('getBody');
117248
$this->zendClientMock->expects(static::once())
118249
->method('request')
119-
->willThrowException(new \Zend_Http_Client_Exception());
250+
->willThrowException(new Zend_Http_Client_Exception());
120251

121252
$object = new DataObject();
122253
$this->object->postRequest($object, $configInterfaceMock);

0 commit comments

Comments
 (0)