Skip to content

3.2.11 Fix handling of typed references. Fix gc cycle handling. #369

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

Merged
TysonAndre merged 2 commits into from
Nov 6, 2022
Merged

3.2.11 Fix handling of typed references. Fix gc cycle handling. #369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aa9169c
3.2.11 Fix handling of typed references. Fix gc cycle handling.
TysonAndre Nov 6, 2022
c4344b8
Fix bugs
TysonAndre Nov 6, 2022
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
76 changes: 38 additions & 38 deletions .appveyor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,96 +33,96 @@ cache:
environment:
BIN_SDK_VER: 2.2.0
matrix:
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
ARCH: x86
- ARCH: x86
VC: vs16
PHP_VER: 8.2.0RC4
PHP_VER: 8.2.0RC5
TS: 1
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
ARCH: x64
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
- ARCH: x64
VC: vs16
PHP_VER: 8.2.0RC4
PHP_VER: 8.2.0RC5
TS: 0
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
ARCH: x86
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
- ARCH: x86
VC: vs16
PHP_VER: 8.1.11
TS: 1
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
ARCH: x64
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
- ARCH: x64
VC: vs16
PHP_VER: 8.1.11
TS: 0
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
ARCH: x86
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
- ARCH: x86
VC: vs16
PHP_VER: 8.0.24
TS: 1
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
ARCH: x64
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
- ARCH: x64
VC: vs16
PHP_VER: 8.0.24
TS: 0
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
ARCH: x64
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
- ARCH: x64
VC: vc14
PHP_VER: 7.0.33
TS: 0
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
ARCH: x64
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
- ARCH: x64
VC: vc14
PHP_VER: 7.0.33
TS: 1
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
ARCH: x86
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
- ARCH: x86
VC: vc14
PHP_VER: 7.0.33
TS: 0
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
ARCH: x86
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
- ARCH: x86
VC: vc14
PHP_VER: 7.0.33
TS: 1
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
ARCH: x64
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
- ARCH: x64
VC: vc14
PHP_VER: 7.1.33
TS: 0
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
ARCH: x86
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
- ARCH: x86
VC: vc14
PHP_VER: 7.1.33
TS: 1
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
ARCH: x64
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
- ARCH: x64
VC: vc15
PHP_VER: 7.2.34
TS: 1
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
ARCH: x86
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
- ARCH: x86
VC: vc15
PHP_VER: 7.2.34
TS: 0
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
ARCH: x86
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
- ARCH: x86
VC: vc15
PHP_VER: 7.3.32
TS: 0
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
ARCH: x64
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
- ARCH: x64
VC: vc15
PHP_VER: 7.3.32
TS: 1
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
ARCH: x64
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
- ARCH: x64
VC: vc15
PHP_VER: 7.4.30
TS: 1
- APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
ARCH: x86
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
- ARCH: x86
VC: vc15
PHP_VER: 7.4.30
TS: 0
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017

build_script:
ps: |
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ jobs:
DOCKER_ARCHITECTURE: i386
- PHP_VERSION: '8.1'
PHP_VERSION_FULL: 8.1.11
- PHP_VERSION: '8.2.0RC4'
PHP_VERSION_FULL: 8.2.0RC4
- PHP_VERSION: '8.2.0RC4'
PHP_VERSION_FULL: 8.2.0RC4
- PHP_VERSION: '8.2.0RC5'
PHP_VERSION_FULL: 8.2.0RC5
- PHP_VERSION: '8.2.0RC5'
PHP_VERSION_FULL: 8.2.0RC5
DOCKER_ARCHITECTURE: i386

# Steps represent a sequence of tasks that will be executed as part of the job
Expand Down
34 changes: 30 additions & 4 deletions package.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ memcached or similar memory based storages for serialized data.</description>
<date>2022-11-06</date>
<time>16:00:00</time>
<version>
<release>3.2.10</release>
<release>3.2.11</release>
<api>1.4.0</api>
</version>
<stability>
Expand All @@ -43,8 +43,8 @@ memcached or similar memory based storages for serialized data.</description>
</stability>
<license uri="https://github.com/igbinary/igbinary/blob/master/COPYING">BSD-3-Clause</license>
<notes>
* Add a macro that callers can use to check if igbinary will accept the header for data being unserialized.
* Fix bug preventing the unserialization of data containing representations of strings larger than 4GB.
* Fix a bug that could prevent objects/arrays with reference cycles from being properly garbage collected.
* Fix bugs in unserializing PHP references to values found in php 7.4 typed properties (#363)
</notes>
<contents>
<dir name="/">
Expand Down Expand Up @@ -72,7 +72,7 @@ memcached or similar memory based storages for serialized data.</description>
<file name="README.md" role="doc" />
<file name="TECH_NOTES.md" role="doc" />
<file name="NEWS" role="doc" />
<!-- Generated by: ls tests|grep phpt|LC_ALL=C sort|sed 's,.*, <file name="\0" role="test" />,' -->
<!-- Generated by: ls tests|grep -E '(phpt|inc)$' |LC_ALL=C sort|sed 's,.*, <file name="\0" role="test" />,' -->
<dir name="tests">
<file name="__serialize_001.phpt" role="test" />
<file name="__serialize_002.phpt" role="test" />
Expand Down Expand Up @@ -124,6 +124,7 @@ memcached or similar memory based storages for serialized data.</description>
<file name="igbinary_020.phpt" role="test" />
<file name="igbinary_021.phpt" role="test" />
<file name="igbinary_022.phpt" role="test" />
<file name="igbinary_022_php82.phpt" role="test" />
<file name="igbinary_023.phpt" role="test" />
<file name="igbinary_024.phpt" role="test" />
<file name="igbinary_025.phpt" role="test" />
Expand Down Expand Up @@ -209,12 +210,20 @@ memcached or similar memory based storages for serialized data.</description>
<file name="igbinary_095.phpt" role="test" />
<file name="igbinary_096.phpt" role="test" />
<file name="igbinary_097.phpt" role="test" />
<file name="igbinary_098.phpt" role="test" />
<file name="igbinary_099.phpt" role="test" />
<file name="igbinary_bug54662.phpt" role="test" />
<file name="igbinary_bug72134.phpt" role="test" />
<file name="igbinary_enums_1.phpt" role="test" />
<file name="igbinary_enums_2.phpt" role="test" />
<file name="igbinary_unserialize_v1_compatible.phpt" role="test" />
<file name="php82_suppress_dynamic_properties_warning.inc" role="test" />
<file name="sleep_mangled_name_clash.phpt" role="test" />
<file name="typed_property_ref_assignment_failure.phpt" role="test" />
<file name="typed_property_ref_overwrite.phpt" role="test" />
<file name="typed_property_ref_overwrite2.phpt" role="test" />
<file name="typed_property_refs.phpt" role="test" />
<file name="typed_property_refs_php74.phpt" role="test" />
</dir>
</dir>
</contents>
Expand All @@ -231,6 +240,23 @@ memcached or similar memory based storages for serialized data.</description>
<providesextension>igbinary</providesextension>
<extsrcrelease />
<changelog>
<release>
<date>2022-11-06</date>
<time>16:00:00</time>
<version>
<release>3.2.10</release>
<api>1.4.0</api>
</version>
<stability>
<release>stable</release>
<api>stable</api>
</stability>
<license uri="https://github.com/igbinary/igbinary/blob/master/COPYING">BSD-3-Clause</license>
<notes>
* Add a macro that callers can use to check if igbinary will accept the header for data being unserialized.
* Fix bug preventing the unserialization of data containing representations of strings larger than 4GB.
</notes>
</release>
<release>
<date>2022-10-17</date>
<time>16:00:00</time>
Expand Down
63 changes: 62 additions & 1 deletion src/php7/igbinary.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,9 @@ struct igbinary_unserialize_data {
uint32_t deferred_count; /**< count of objects in array for calls to __unserialize/__wakeup. NOTE: Current php releases including 8.1 limit the total number of objects to a 32-bit integer. */
zend_bool deferred_finished; /**< whether the deferred calls were performed */
struct deferred_dtor_tracker deferred_dtor_tracker; /**< refcounted objects and arrays to call dtor on after unserializing. See i_zval_ptr_dtor */
#if PHP_VERSION_ID >= 70400
HashTable *ref_props; /**< objects&data for calls to __unserialize/__wakeup */
#endif
};

#define IGB_REF_VAL_2(igsd, n) ((igsd)->references[(n)])
Expand Down Expand Up @@ -781,6 +784,14 @@ IGBINARY_API int igbinary_unserialize(const uint8_t *buf, size_t buf_len, zval *
ret = 1;
goto cleanup;
}
if (Z_REFCOUNTED_P(z)) {
#if PHP_VERSION_ID >= 70200
zend_refcounted *ref = Z_COUNTED_P(z);
gc_check_possible_root(ref);
#else
gc_check_possible_root(z);
#endif
}

if (UNEXPECTED(igsd.buffer_ptr < igsd.buffer_end)) {
// https://github.com/igbinary/igbinary/issues/64
Expand Down Expand Up @@ -2066,6 +2077,9 @@ inline static int igbinary_unserialize_data_init(struct igbinary_unserialize_dat
igsd->deferred_dtor_tracker.zvals = NULL;
igsd->deferred_dtor_tracker.count = 0;
igsd->deferred_dtor_tracker.capacity = 0;
#if PHP_VERSION_ID >= 70400
igsd->ref_props = NULL;
#endif

return 0;
}
Expand Down Expand Up @@ -2110,6 +2124,12 @@ inline static void igbinary_unserialize_data_deinit(struct igbinary_unserialize_
efree(calls);
}
free_deferred_dtors(&igsd->deferred_dtor_tracker);
#if PHP_VERSION_ID >= 70400
if (igsd->ref_props) {
zend_hash_destroy(igsd->ref_props);
FREE_HASHTABLE(igsd->ref_props);
}
#endif

return;
}
Expand Down Expand Up @@ -2815,6 +2835,18 @@ inline static int igbinary_unserialize_object_properties(struct igbinary_unseria
prototype_value = Z_INDIRECT_P(prototype_value);
#if PHP_VERSION_ID >= 70400
info = zend_get_typed_property_info_for_slot(Z_OBJ_P(z_deref), prototype_value);
if (info) {
if (Z_ISREF_P(prototype_value)) {
/* If the value is overwritten, remove old type source from ref. */
ZEND_REF_DEL_TYPE_SOURCE(Z_REF_P(prototype_value), info);
}

if (igsd->ref_props) {
/* Remove old entry from ref_props table, if it exists. */
zend_hash_index_del(
igsd->ref_props, (zend_uintptr_t) prototype_value);
}
}
#endif
}
/* This is written to avoid the overhead of a second zend_hash_update call. See https://github.com/php/php-src/pull/5095 */
Expand Down Expand Up @@ -2865,6 +2897,13 @@ inline static int igbinary_unserialize_object_properties(struct igbinary_unseria
ZEND_ASSERT(Z_TYPE_P(vp) != IS_INDIRECT);

if (UNEXPECTED(igbinary_unserialize_zval(igsd, vp, WANT_CLEAR))) {
#if PHP_VERSION_ID >= 70400
if (info && Z_ISREF_P(vp)) {
/* Add type source even if we failed to unserialize.
* The data is still stored in the property. */
ZEND_REF_ADD_TYPE_SOURCE(Z_REF_P(vp), info);
}
#endif
/* Unserializing a property into this zval has failed. */
/* zval_ptr_dtor(z); */
/* zval_ptr_dtor(vp); */
Expand All @@ -2879,6 +2918,15 @@ inline static int igbinary_unserialize_object_properties(struct igbinary_unseria
}
if (Z_ISREF_P(vp)) {
ZEND_REF_ADD_TYPE_SOURCE(Z_REF_P(vp), info);
} else {
/* Remember to which property this slot belongs, so we can add a
* type source if it is turned into a reference lateron. */
if (!igsd->ref_props) {
igsd->ref_props = emalloc(sizeof(HashTable));
zend_hash_init(igsd->ref_props, 8, NULL, NULL, 0);
}
zend_hash_index_update_ptr(
igsd->ref_props, (zend_uintptr_t) vp, info);
}
}
#endif
Expand Down Expand Up @@ -3406,7 +3454,20 @@ static int igbinary_unserialize_zval(struct igbinary_unserialize_data *igsd, zva
/* Permanently convert the zval in IGB_REF_VAL() into a IS_REFERENCE if it wasn't already one. */
/* TODO: Support multiple reference groups to the same object */
/* Similar to https://github.com/php/php-src/blob/master/ext/standard/var_unserializer.re , for "R:" */
ZVAL_MAKE_REF(z);
if (!Z_ISREF_P(z)) {
#if PHP_VERSION_ID >= 70400
zend_property_info *info = NULL;
if (igsd->ref_props) {
info = zend_hash_index_find_ptr(igsd->ref_props, (zend_uintptr_t) z);
}
#endif
ZVAL_NEW_REF(z, z);
#if PHP_VERSION_ID >= 70400
if (info) {
ZEND_REF_ADD_TYPE_SOURCE(Z_REF_P(z), info);
}
#endif
}
switch (type) {
case IS_STRING:
case IS_LONG:
Expand Down
2 changes: 1 addition & 1 deletion src/php7/igbinary.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ struct zval;
/** Binary protocol version of igbinary. */
#define IGBINARY_FORMAT_VERSION 0x00000002

#define PHP_IGBINARY_VERSION "3.2.10"
#define PHP_IGBINARY_VERSION "3.2.11"

/* Macros */

Expand Down
21 changes: 21 additions & 0 deletions tests/sleep_mangled_name_clash.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
--TEST--
__sleep() returns properties clashing only after mangling
--FILE--
<?php
class Test {
private $priv;
public function __sleep() {
return ["\0Test\0priv", "priv"];
}
}
// igbinary currently does not emit a notice or check for duplicates
$s = igbinary_serialize(new Test);
echo urlencode($s), "\n";
var_dump(igbinary_unserialize($s));
?>
--EXPECT--
%00%00%00%02%17%04Test%14%02%11%0A%00Test%00priv%00%0E%01%00
object(Test)#1 (1) {
["priv":"Test":private]=>
NULL
}
Loading