msgpack / msgpack-php

msgpack.org[PHP]
BSD 3-Clause "New" or "Revised" License
773 stars 119 forks source link

msgpack (un)pack error #149

Closed a4blue closed 3 years ago

a4blue commented 4 years ago

As attachment i have an object which produces following Warning: (msgpack_unserialize_map_item) Invalid references value: 29696 This Behaviour was tested and reproduced under following conditions: php_only was set to true, php7.4 with msgpack-master php7.4 with msgpack 2.0.3 php7.4 with msgpack 2.1.0 php 7.2 with msgpack 2.0.2

simple script to reproduce:

<?php
$data=unserialize(file_get_contents('php_serialized.txt));
// echo is empty and msgpack issues a warning 
// there should be no warning and echo should print 1
echo $data == msgpack_unpack(msgpack_pack($data)));

Any help on how to debug this would be appreciated php_serialized.txt

m6w6 commented 4 years ago

Hi! Thank you for your report. We'll look into it.

andypost commented 2 years ago

Just get the test failure when packaged 2.2.0RC1 for Alpinelinux, btw restart of test helped and it passed

The failure https://gitlab.alpinelinux.org/alpine/aports/-/jobs/474442#L510

PHP         : /usr/bin/php7 
PHP_SAPI    : cli
PHP_VERSION : 7.4.23
ZEND_VERSION: 3.4.0
PHP_OS      : Linux - Linux runner-xdqdyyub-project-1-concurrent-0 5.10.7-0-lts #1-Alpine SMP Thu, 14 Jan 2021 07:31:13 UTC i686
INI actual  : /builds/alpine/aports/community/php7-pecl-msgpack/src/msgpack-2.2.0RC1/tmp-php.ini
More .INIs  :   
CWD         : /builds/alpine/aports/community/php7-pecl-msgpack/src/msgpack-2.2.0RC1
Extra dirs  : 
VALGRIND    : Not used
=====================================================================
TIME START 2021-08-30 19:47:10
....
TEST 135/135 [tests/issue149.phpt]
========DIFF========
002+ bool(false)
002- bool(true)
========DONE========
FAIL Issue #149 (msgpack (un)pack error with serialize()) [tests/issue149.phpt] 
m6w6 commented 2 years ago

@andypost Do you mean it once failed, and then passed?

andypost commented 2 years ago

Yes, it randomly fails (only on x86 arch) both 7.4 and 8.0

m6w6 commented 2 years ago

Oh :facepalm: x86! Any idea where I could easily test against that?

andypost commented 2 years ago

Here's a way to reproduce

$ make -f AMakefile x86
...docker download image and run it mounting source code

...inside container
$ make -f AMakefile provision -B
...
$ make clean && make

... repeat next until diff is broken
$ make -f AMakefile test

@m6w6 you could use sudo apk add gdb or whatever tool you need from https://pkgs.alpinelinux.org/packages

Here's a AMakefile and test-dubug.patch and few *.diff of data vs check objects, see attached

test-debug.tar.gz

andypost commented 2 years ago

Also you can replace php8 with php7 package

m6w6 commented 2 years ago

Awesome, I'll give it a go, thanks!

andypost commented 2 years ago

FYI just caught the same test fail on armhf which is 32-bit as well

m6w6 commented 2 years ago

Looks like a hash collision, apparently existing since forever on 32bit. https://github.com/msgpack/msgpack-php/blob/master/msgpack_pack.c#L51

andypost commented 2 years ago

Sounds like better to disable for 32-bit arches for a while