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

Jump to bottom

Handle up to 2GB payload on PHP32bits #65

Merged
merged 3 commits into from
Aug 13, 2022
Merged

Handle up to 2GB payload on PHP32bits #65

merged 3 commits into from
Aug 13, 2022

Conversation

Domochip
Copy link
Contributor

PR described in issue #64

Main idea is to avoid unpack('J',... on 32bits system.

$payload_length is in Big Endian allowing to keep only 4 last bytes to unpack it as an unsigned long instead of unsigned long long

@cboden
Copy link
Member

cboden commented Dec 16, 2021

Can you modify the CI workflow to test against a 32-bit core?

@Domochip
Copy link
Contributor Author

Honestly, I have very few knowledge about CI Test workflow and don't know how to do. :-(
(I had a look at PHPUnit but didn't find any x64/x86 settings for tests)

@cboden
Copy link
Member

cboden commented Dec 16, 2021

Allow me to summon the all-knowing @SimonFrings

Alternatively, are you able to successfully run the entire test suite on a 32-bit system and provide the results? I'm specifically looking for the Autobahn ones; they test the WebSocket messaging protocols.

@Domochip
Copy link
Contributor Author

Do you have any short instructions that describe how to run tests?
I just tested that modification make it work for my specific usage.
It is just a simple patch to the issue we encountering.

@BadWolf42
Copy link

BadWolf42 commented Dec 17, 2021 •
edited
Loading

Hi @cboden & @Domochip,
Please find bellow the results on a 32-bits system:

Current master branch code (commit 7c96451):

bad@32b-dev:~/tmp$ git clone https://github.com/ratchetphp/RFC6455.git RFC6455-ratchetphp
Cloning into 'RFC6455-ratchetphp'...
remote: Enumerating objects: 1658, done.
remote: Counting objects: 100% (128/128), done.
remote: Compressing objects: 100% (94/94), done.
remote: Total 1658 (delta 54), reused 75 (delta 28), pack-reused 1530
Receiving objects: 100% (1658/1658), 325.73 KiB | 3.07 MiB/s, done.
Resolving deltas: 100% (961/961), done.

bad@32b-dev:~/tmp$ cd RFC6455-ratchetphp/

bad@32b-dev:~/tmp/RFC6455-ratchetphp$ php ../composer.phar install
No composer.lock file present. Updating dependencies to latest instead of installing from lock file. See https://getcomposer.org/install for more information.
Loading composer repositories with package information
Updating dependencies
Lock file operations: 38 installs, 0 updates, 0 removals
  - Locking doctrine/instantiator (1.4.0)
  - Locking evenement/evenement (v3.0.1)
  - Locking guzzlehttp/psr7 (2.1.0)
  - Locking myclabs/deep-copy (1.10.2)
  - Locking phpdocumentor/reflection-common (2.2.0)
  - Locking phpdocumentor/reflection-docblock (5.3.0)
  - Locking phpdocumentor/type-resolver (1.5.1)
  - Locking phpspec/prophecy (v1.10.3)
  - Locking phpunit/php-code-coverage (4.0.8)
  - Locking phpunit/php-file-iterator (1.4.5)
  - Locking phpunit/php-text-template (1.2.1)
  - Locking phpunit/php-timer (1.0.9)
  - Locking phpunit/php-token-stream (2.0.2)
  - Locking phpunit/phpunit (5.7.27)
  - Locking phpunit/phpunit-mock-objects (3.4.4)
  - Locking psr/http-factory (1.0.1)
  - Locking psr/http-message (1.0.1)
  - Locking ralouphie/getallheaders (3.0.3)
  - Locking react/cache (v1.1.1)
  - Locking react/dns (v1.8.0)
  - Locking react/event-loop (v1.2.0)
  - Locking react/promise (v2.8.0)
  - Locking react/promise-timer (v1.8.0)
  - Locking react/socket (v1.10.0)
  - Locking react/stream (v1.2.0)
  - Locking sebastian/code-unit-reverse-lookup (1.0.2)
  - Locking sebastian/comparator (1.2.4)
  - Locking sebastian/diff (1.4.3)
  - Locking sebastian/environment (2.0.0)
  - Locking sebastian/exporter (2.0.0)
  - Locking sebastian/global-state (1.1.1)
  - Locking sebastian/object-enumerator (2.0.1)
  - Locking sebastian/recursion-context (2.0.0)
  - Locking sebastian/resource-operations (1.0.0)
  - Locking sebastian/version (2.0.1)
  - Locking symfony/polyfill-ctype (v1.23.0)
  - Locking symfony/yaml (v4.4.34)
  - Locking webmozart/assert (1.10.0)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 38 installs, 0 updates, 0 removals
  - Installing ralouphie/getallheaders (3.0.3): Extracting archive
  - Installing psr/http-message (1.0.1): Extracting archive
  - Installing psr/http-factory (1.0.1): Extracting archive
  - Installing guzzlehttp/psr7 (2.1.0): Extracting archive
  - Installing symfony/polyfill-ctype (v1.23.0): Extracting archive
  - Installing webmozart/assert (1.10.0): Extracting archive
  - Installing phpdocumentor/reflection-common (2.2.0): Extracting archive
  - Installing phpdocumentor/type-resolver (1.5.1): Extracting archive
  - Installing phpdocumentor/reflection-docblock (5.3.0): Extracting archive
  - Installing phpunit/php-token-stream (2.0.2): Extracting archive
  - Installing symfony/yaml (v4.4.34): Extracting archive
  - Installing sebastian/version (2.0.1): Extracting archive
  - Installing sebastian/resource-operations (1.0.0): Extracting archive
  - Installing sebastian/recursion-context (2.0.0): Extracting archive
  - Installing sebastian/object-enumerator (2.0.1): Extracting archive
  - Installing sebastian/global-state (1.1.1): Extracting archive
  - Installing sebastian/exporter (2.0.0): Extracting archive
  - Installing sebastian/environment (2.0.0): Extracting archive
  - Installing sebastian/diff (1.4.3): Extracting archive
  - Installing sebastian/comparator (1.2.4): Extracting archive
  - Installing phpunit/php-text-template (1.2.1): Extracting archive
  - Installing doctrine/instantiator (1.4.0): Extracting archive
  - Installing phpunit/phpunit-mock-objects (3.4.4): Extracting archive
  - Installing phpunit/php-timer (1.0.9): Extracting archive
  - Installing phpunit/php-file-iterator (1.4.5): Extracting archive
  - Installing sebastian/code-unit-reverse-lookup (1.0.2): Extracting archive
  - Installing phpunit/php-code-coverage (4.0.8): Extracting archive
  - Installing phpspec/prophecy (v1.10.3): Extracting archive
  - Installing myclabs/deep-copy (1.10.2): Extracting archive
  - Installing phpunit/phpunit (5.7.27): Extracting archive
  - Installing react/promise (v2.8.0): Extracting archive
  - Installing react/cache (v1.1.1): Extracting archive
  - Installing react/event-loop (v1.2.0): Extracting archive
  - Installing evenement/evenement (v3.0.1): Extracting archive
  - Installing react/stream (v1.2.0): Extracting archive
  - Installing react/promise-timer (v1.8.0): Extracting archive
  - Installing react/dns (v1.8.0): Extracting archive
  - Installing react/socket (v1.10.0): Extracting archive
8 package suggestions were added by new dependencies, use `composer suggest` to see details.
Package phpunit/php-token-stream is abandoned, you should avoid using it. No replacement was suggested.
Package phpunit/phpunit-mock-objects is abandoned, you should avoid using it. No replacement was suggested.
Generating autoload files
12 packages you are using are looking for funding.
Use the `composer fund` command to find out more!

bad@32b-dev:~/tmp/RFC6455-ratchetphp$ php ../composer.phar test
> ABTEST=client && sh tests/ab/run_ab_tests.sh
+ cd tests/ab
+ [  = client ]
+ [  = server ]
> ABTEST=server && sh tests/ab/run_ab_tests.sh
+ cd tests/ab
+ [  = client ]
+ [  = server ]
> phpunit --colors=always
PHPUnit 5.7.27 by Sebastian Bergmann and contributors.

SS.............................................................  63 / 210 ( 30%)
............................................................... 126 / 210 ( 60%)
...................................................EE.......F.. 189 / 210 ( 90%)
.........F...........                                           210 / 210 (100%)

Time: 5.65 seconds, Memory: 7.00MB

There were 2 errors:

1) Ratchet\RFC6455\Test\Unit\Messaging\MessageBufferTest::testInvalidFrameLength
unpack(): 64-bit format codes are not available for 32-bit versions of PHP

/home/bad/tmp/RFC6455-ratchetphp/src/Messaging/MessageBuffer.php:163
/home/bad/tmp/RFC6455-ratchetphp/tests/unit/Messaging/MessageBufferTest.php:109

2) Ratchet\RFC6455\Test\Unit\Messaging\MessageBufferTest::testFrameLengthTooBig
unpack(): 64-bit format codes are not available for 32-bit versions of PHP

/home/bad/tmp/RFC6455-ratchetphp/src/Messaging/MessageBuffer.php:163
/home/bad/tmp/RFC6455-ratchetphp/tests/unit/Messaging/MessageBufferTest.php:153

--

There were 2 failures:

1) Ratchet\RFC6455\Test\Unit\Messaging\MessageBufferTest::testMemoryLimits with data set "2 GB with small "g"" ('2g', 2147483648.0)
Failed asserting that 0 is identical to 2147483648.0.

/home/bad/tmp/RFC6455-ratchetphp/tests/unit/Messaging/MessageBufferTest.php:266

2) Ratchet\RFC6455\Test\Unit\Messaging\MessageBufferTest::testIniSizes with data set "2 GB with small "g"" ('2g', 2147483648.0)
Failed asserting that 0 matches expected 536870912.0.

/home/bad/tmp/RFC6455-ratchetphp/tests/unit/Messaging/MessageBufferTest.php:343

ERRORS!
Tests: 210, Assertions: 1277, Errors: 2, Failures: 2, Skipped: 2.
Script phpunit --colors=always handling the phpunit event returned with error code 2
Script @phpunit was called via test

bad@32b-dev:~/tmp/RFC6455-ratchetphp$

Domochip master branch code (commit https://github.com/Domochip/RFC6455/commit/c018f30d0c39c5012725686a7259c85a1864b57b):

bad@32b-dev:~/tmp$ git clone https://github.com/Domochip/RFC6455.git RFC6455-Domochip
Cloning into 'RFC6455-Domochip'...
remote: Enumerating objects: 1663, done.
remote: Counting objects: 100% (133/133), done.
remote: Compressing objects: 100% (97/97), done.
remote: Total 1663 (delta 57), reused 79 (delta 30), pack-reused 1530
Receiving objects: 100% (1663/1663), 326.29 KiB | 3.93 MiB/s, done.
Resolving deltas: 100% (964/964), done.

bad@32b-dev:~/tmp$ cd RFC6455-Domochip

bad@32b-dev:~/tmp/RFC6455-Domochip$ php ../composer.phar install
No composer.lock file present. Updating dependencies to latest instead of installing from lock file. See https://getcomposer.org/install for more information.
Loading composer repositories with package information
Updating dependencies
Lock file operations: 38 installs, 0 updates, 0 removals
  - Locking doctrine/instantiator (1.4.0)
  - Locking evenement/evenement (v3.0.1)
  - Locking guzzlehttp/psr7 (2.1.0)
  - Locking myclabs/deep-copy (1.10.2)
  - Locking phpdocumentor/reflection-common (2.2.0)
  - Locking phpdocumentor/reflection-docblock (5.3.0)
  - Locking phpdocumentor/type-resolver (1.5.1)
  - Locking phpspec/prophecy (v1.10.3)
  - Locking phpunit/php-code-coverage (4.0.8)
  - Locking phpunit/php-file-iterator (1.4.5)
  - Locking phpunit/php-text-template (1.2.1)
  - Locking phpunit/php-timer (1.0.9)
  - Locking phpunit/php-token-stream (2.0.2)
  - Locking phpunit/phpunit (5.7.27)
  - Locking phpunit/phpunit-mock-objects (3.4.4)
  - Locking psr/http-factory (1.0.1)
  - Locking psr/http-message (1.0.1)
  - Locking ralouphie/getallheaders (3.0.3)
  - Locking react/cache (v1.1.1)
  - Locking react/dns (v1.8.0)
  - Locking react/event-loop (v1.2.0)
  - Locking react/promise (v2.8.0)
  - Locking react/promise-timer (v1.8.0)
  - Locking react/socket (v1.10.0)
  - Locking react/stream (v1.2.0)
  - Locking sebastian/code-unit-reverse-lookup (1.0.2)
  - Locking sebastian/comparator (1.2.4)
  - Locking sebastian/diff (1.4.3)
  - Locking sebastian/environment (2.0.0)
  - Locking sebastian/exporter (2.0.0)
  - Locking sebastian/global-state (1.1.1)
  - Locking sebastian/object-enumerator (2.0.1)
  - Locking sebastian/recursion-context (2.0.0)
  - Locking sebastian/resource-operations (1.0.0)
  - Locking sebastian/version (2.0.1)
  - Locking symfony/polyfill-ctype (v1.23.0)
  - Locking symfony/yaml (v4.4.34)
  - Locking webmozart/assert (1.10.0)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 38 installs, 0 updates, 0 removals
  - Installing ralouphie/getallheaders (3.0.3): Extracting archive
  - Installing psr/http-message (1.0.1): Extracting archive
  - Installing psr/http-factory (1.0.1): Extracting archive
  - Installing guzzlehttp/psr7 (2.1.0): Extracting archive
  - Installing symfony/polyfill-ctype (v1.23.0): Extracting archive
  - Installing webmozart/assert (1.10.0): Extracting archive
  - Installing phpdocumentor/reflection-common (2.2.0): Extracting archive
  - Installing phpdocumentor/type-resolver (1.5.1): Extracting archive
  - Installing phpdocumentor/reflection-docblock (5.3.0): Extracting archive
  - Installing phpunit/php-token-stream (2.0.2): Extracting archive
  - Installing symfony/yaml (v4.4.34): Extracting archive
  - Installing sebastian/version (2.0.1): Extracting archive
  - Installing sebastian/resource-operations (1.0.0): Extracting archive
  - Installing sebastian/recursion-context (2.0.0): Extracting archive
  - Installing sebastian/object-enumerator (2.0.1): Extracting archive
  - Installing sebastian/global-state (1.1.1): Extracting archive
  - Installing sebastian/exporter (2.0.0): Extracting archive
  - Installing sebastian/environment (2.0.0): Extracting archive
  - Installing sebastian/diff (1.4.3): Extracting archive
  - Installing sebastian/comparator (1.2.4): Extracting archive
  - Installing phpunit/php-text-template (1.2.1): Extracting archive
  - Installing doctrine/instantiator (1.4.0): Extracting archive
  - Installing phpunit/phpunit-mock-objects (3.4.4): Extracting archive
  - Installing phpunit/php-timer (1.0.9): Extracting archive
  - Installing phpunit/php-file-iterator (1.4.5): Extracting archive
  - Installing sebastian/code-unit-reverse-lookup (1.0.2): Extracting archive
  - Installing phpunit/php-code-coverage (4.0.8): Extracting archive
  - Installing phpspec/prophecy (v1.10.3): Extracting archive
  - Installing myclabs/deep-copy (1.10.2): Extracting archive
  - Installing phpunit/phpunit (5.7.27): Extracting archive
  - Installing react/promise (v2.8.0): Extracting archive
  - Installing react/cache (v1.1.1): Extracting archive
  - Installing react/event-loop (v1.2.0): Extracting archive
  - Installing evenement/evenement (v3.0.1): Extracting archive
  - Installing react/stream (v1.2.0): Extracting archive
  - Installing react/promise-timer (v1.8.0): Extracting archive
  - Installing react/dns (v1.8.0): Extracting archive
  - Installing react/socket (v1.10.0): Extracting archive
8 package suggestions were added by new dependencies, use `composer suggest` to see details.
Package phpunit/php-token-stream is abandoned, you should avoid using it. No replacement was suggested.
Package phpunit/phpunit-mock-objects is abandoned, you should avoid using it. No replacement was suggested.
Generating autoload files
12 packages you are using are looking for funding.
Use the `composer fund` command to find out more!

bad@32b-dev:~/tmp/RFC6455-Domochip$ php ../composer.phar test
> ABTEST=client && sh tests/ab/run_ab_tests.sh
+ cd tests/ab
+ [  = client ]
+ [  = server ]
> ABTEST=server && sh tests/ab/run_ab_tests.sh
+ cd tests/ab
+ [  = client ]
+ [  = server ]
> phpunit --colors=always
PHPUnit 5.7.27 by Sebastian Bergmann and contributors.

SS.............................................................  63 / 210 ( 30%)
............................................................... 126 / 210 ( 60%)
....................................................F.......F.. 189 / 210 ( 90%)
.........F...........                                           210 / 210 (100%)

Time: 5.6 seconds, Memory: 7.00MB

There were 3 failures:

1) Ratchet\RFC6455\Test\Unit\Messaging\MessageBufferTest::testFrameLengthTooBig
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 1009
+    0 => 1002
 )

/home/bad/tmp/RFC6455-Domochip/tests/unit/Messaging/MessageBufferTest.php:158

2) Ratchet\RFC6455\Test\Unit\Messaging\MessageBufferTest::testMemoryLimits with data set "2 GB with small "g"" ('2g', 2147483648.0)
Failed asserting that 0 is identical to 2147483648.0.

/home/bad/tmp/RFC6455-Domochip/tests/unit/Messaging/MessageBufferTest.php:266

3) Ratchet\RFC6455\Test\Unit\Messaging\MessageBufferTest::testIniSizes with data set "2 GB with small "g"" ('2g', 2147483648.0)
Failed asserting that 0 matches expected 536870912.0.

/home/bad/tmp/RFC6455-Domochip/tests/unit/Messaging/MessageBufferTest.php:343

FAILURES!
Tests: 210, Assertions: 1287, Failures: 3, Skipped: 2.
Script phpunit --colors=always handling the phpunit event returned with error code 1
Script @phpunit was called via test

bad@32b-dev:~/tmp/RFC6455-Domochip$

Bad

@Domochip
Copy link
Contributor Author

Domochip commented Dec 18, 2021 •
edited
Loading

Thanks a lot @BadWolf42
@cboden Is it OK for you? Or do you prefer that length over 2GB raises an exception on 32bits systems?

@Domochip
Copy link
Contributor Author

@cboden Do BadWolf42 test results are good for you?
Do you prefer we raise an exception or a close frame CLOSE_TOO_BIG if payload is over 2Go?

@BadWolf42
Copy link

BadWolf42 commented Dec 31, 2021 •
edited
Loading

Hi @cboden,
Could we please have an update regarding this issue ?
Now that the root cause is identified, more users are registering as impacted.

Can you guide us to PR an acceptable fix? Or do you need more time to think about it?
In short, do we patch our code to handle this issue or can we expect a resolution during January?

Thanks and happy festive season,
Bad

@BadWolf42
Copy link

Hi guys,
Any news for us ?
Thanks,
Bad

@Domochip
Copy link
Contributor Author

Hi @cboden,
Do you need time to evaluate issue and proposed fix?
Do you think we need to include a temporary fix in our code during this evaluation time?
thx

@SQKo SQKo mentioned this pull request Jan 23, 2022
Open
@SQKo
Copy link

SQKo commented May 26, 2022

Hello, any update on this? The code in the commit fixed our issue with DiscordPHP

cboden
cboden approved these changes Aug 13, 2022
View reviewed changes
Copy link
Member

@cboden cboden left a comment

Choose a reason for hiding this comment

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

Hi everyone. Sorry for dragging my feet on this. The code looks good to me and things continue to work on my systems. I'm going to merge it.

If you guys know of a way I can virtualize a 32 bit system and provide me with some code I can use to reproduce getting a too big response, I'd appreciate it.

@cboden cboden merged commit 382a3ed into ratchetphp:master Aug 13, 2022
@BadWolf42
Copy link

Hello,

I don't fully understand the way the test suite is working, but as per shivammathur/setup-php#302 (comment) and https://github.com/shivammathur/setup-php#multi-arch-setup, shivammathur/setup-php should be able to handle 32 bit containerized test suites.

For the tests done in December, I used a Raspberry Pi 3B+.

Bad

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clue clue clue left review comments

@SQKo SQKo SQKo left review comments

@cboden cboden cboden approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Domochip @cboden @BadWolf42 @SQKo @clue