Skip to content
This repository has been archived by the owner on Sep 29, 2023. It is now read-only.

bugfix: UTF8 encoding in result response from sandbox, corrupts json response #655

Merged
merged 4 commits into from
Sep 26, 2016

Conversation

bhoehl
Copy link
Contributor

@bhoehl bhoehl commented Sep 22, 2016

@jaypatel512

  • PHP 5.6:
  • PayPal-PHP-SDK 1.7.4:
  • Debug ID(s):

file /lib/PayPal/Core\PayPalHttpConnection.php line 150 reads
// Get Request and Response Headers
$requestHeaders = curl_getinfo($ch, CURLINFO_HEADER_OUT);
//Using alternative solution to CURLINFO_HEADER_SIZE as it throws invalid number when called using PROXY.
$responseHeaderSize = strlen($result) - curl_getinfo($ch, CURLINFO_SIZE_DOWNLOAD);
$responseHeaders = substr($result, 0, $responseHeaderSize);
$result = substr($result, $responseHeaderSize);

The response getting from curl_exec is a string, which is binary. In my case strlen is treating this binary string as UTF8 and is returning a different strlen than the string is when written to disk, using characters like 'äöü ... '. strlen seems to be replaced with mb_strlen, which then uses the default encoding ( UTF8 ).

A better approach is to use mb_strlen in combination with 8bit encoding.
Any idea, if utf_decode has to be used, too ?

see: php.ini using mbstring.func_overload

Bernd Hoehl added 2 commits September 22, 2016 13:18

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@bhoehl bhoehl changed the title UTF8 encoding in result response from sandbox, corrupts json response bugfix: UTF8 encoding in result response from sandbox, corrupts json response Sep 22, 2016
$responseHeaderSize = strlen($result) - curl_getinfo($ch, CURLINFO_SIZE_DOWNLOAD);
$responseHeaders = substr($result, 0, $responseHeaderSize);
$result = substr($result, $responseHeaderSize);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra newline here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@braebot
removed

@jaypatel512
Copy link
Contributor

Hey @bhoehl !

You are absolutely right. I will go ahead test these changes out soon. Thanks again for your PR.

@jaypatel512 jaypatel512 merged commit 8757cdb into paypal:master Sep 26, 2016
@bhoehl
Copy link
Contributor Author

bhoehl commented Sep 29, 2016

@jaypatel512
when will there be a release containing this fix ?
this hinders us using composer, as we can not change our php.ini settings
and patching this file is no option for us

Thanks

Bernd

@jaypatel512
Copy link
Contributor

Good news !

We have made a new release with your PR fixes. You can download v1.8.0 with your changes.

@bhoehl
Copy link
Contributor Author

bhoehl commented Sep 29, 2016

Thank you!

Bernd

On Thursday, 29 September 2016, Jay notifications@github.com wrote:

Good news !

We have made a new release with your PR fixes. You can download v1.8.0
https://github.com/paypal/PayPal-PHP-SDK/releases/tag/1.8.0 with your
changes.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#655 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFf5w4Y2QQrcIwT-ioz2jOTODoNJudKCks5qu-2qgaJpZM4KDyH8
.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants