Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for very slow chunk processing for larger content in the http res… #354

Merged
merged 3 commits into from
Jul 22, 2024
Merged

Fix for very slow chunk processing for larger content in the http res… #354

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b3586c1
Fix for very slow chunk processing for larger content in the http res…
aleksandar-mitrev Jun 13, 2023
cfaf5bb
- revoke function skope
aleksandar-mitrev Jun 13, 2023
78f59e6
restore mbstring overloading for full backwards compatibility
thomaslauria Jul 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions library/Zend/Http/Response.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,26 +591,30 @@ public static function extractBody($response_str)
public static function decodeChunkedBody($body)
{
$decBody = '';
$offset = 0;

// If mbstring overloads substr and strlen functions, we have to
// override it's internal encoding
if (function_exists('mb_internal_encoding') &&
((int) ini_get('mbstring.func_overload')) & 2) {
Copy link

Choose a reason for hiding this comment

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

mbstring.func_overload was deprecated in PHP 7.2 and removed in PHP 8.0... But this lib aims to be compatible from PHP 7.1 onward, so... IMHO, this logic needs to be put back.

But I see the bigger optimization about using the offset. Very nice.

Copy link

@thomaslauria thomaslauria Jun 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

IMHO, this logic needs to be put back

So do we have to change here something, or shall it remain as it is?

Copy link

@boenrobot boenrobot Jun 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

IMHO, this logic needs to be put back

So do we have to change here something, or shall it remain as it is?

It's up to @Shardj , @glensc and I don't know whoever else has permissions to merge.

The above is just my opinion as a fellow user of this library. I am not using mb function overloads in my projects, so I'm not personally affected either way. But if I had merge permissions, I would not merge this without also restoring the function overload logic immediately after.

Copy link

Choose a reason for hiding this comment

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

restored!


$mbIntEnc = mb_internal_encoding();
mb_internal_encoding('ASCII');
}

while (trim($body)) {
if (! preg_match("/^([\da-fA-F]+)[^\r\n]*\r\n/sm", $body, $m)) {
require_once 'Zend/Http/Exception.php';
throw new Zend_Http_Exception("Error parsing body - doesn't seem to be a chunked message");
while (true) {
if (! preg_match("/^([\da-fA-F]+)[^\r\n]*\r\n/sm", $body, $m, 0, $offset)) {
if (trim(substr($body, $offset))) {
require_once 'Zend/Http/Exception.php';
throw new Zend_Http_Exception("Error parsing body - doesn't seem to be a chunked message");
}
// Message was consumed completely
break;
}

$length = hexdec(trim($m[1]));
$cut = strlen($m[0]);
$decBody .= substr($body, $cut, $length);
$body = substr($body, $cut + $length + 2);
$length = hexdec(trim($m[1]));
$cut = strlen($m[0]);
$decBody .= substr($body, $offset + $cut, $length);
$offset += $cut + $length + 2;
}

if (isset($mbIntEnc)) {
Expand Down
Loading