Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix infinite recursion in wddx
Summary:
It wasn't checking for infinite recursion due to references or self-referential
objects. As it turns out closures always return themselves when converted to an
array. Raising a warning and returning is how PHP-src deals with this problem,
nothing special is done for closures.

Reviewed By: alexmalyshev

Differential Revision: D3465655

fbshipit-source-id: a42bc34d30cf4825faf33596139c0c05f8e4f5f1
  • Loading branch information
Orvid authored and Hhvm Bot committed Aug 1, 2016
1 parent 999e2f1 commit 1888810
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 9 deletions.
37 changes: 29 additions & 8 deletions hphp/runtime/ext/wddx/ext_wddx.cpp
Expand Up @@ -66,22 +66,43 @@ bool WddxPacket::serialize_value(const Variant& varVariant) {
bool WddxPacket::recursiveAddVar(const String& varName,
const Variant& varVariant,
bool hasVarTag) {
SeenContainers seen;
return recursiveAddVarImpl(varName, varVariant, hasVarTag, seen);
}

bool WddxPacket::recursiveAddVarImpl(const String& varName,
const Variant& varVariant,
bool hasVarTag,
SeenContainers& seen) {
bool isArray = varVariant.isArray();
bool isObject = varVariant.isObject();

if (isArray || isObject) {
Array varAsArray;
Object varAsObject;
ArrayOrObject ptr;
if (isArray) {
varAsArray = varVariant.toArray();
ptr = varAsArray.get();
}
if (isObject) {
varAsObject = varVariant.toObject();
varAsArray = varAsObject.toArray();
ptr = varAsObject.get();
}
assert(!ptr.isNull());
if (!seen.emplace(ptr).second) {
raise_warning("recursion detected");
return false;
}
SCOPE_EXIT { seen.erase(ptr); };

if (hasVarTag) {
m_packetString.append("<var name='");
m_packetString.append(varName.data());
m_packetString.append("'>");
}

Array varAsArray;
Object varAsObject = varVariant.toObject();
if (isArray) varAsArray = varVariant.toArray();
if (isObject) varAsArray = varAsObject.toArray();

int length = varAsArray.length();
if (length > 0) {
ArrayIter it = ArrayIter(varAsArray);
Expand All @@ -99,9 +120,9 @@ bool WddxPacket::recursiveAddVar(const String& varName,
m_packetString.append("'>");
}
for (ArrayIter it(varAsArray); it; ++it) {
Variant key = it.first();
Variant value = it.second();
recursiveAddVar(key.toString(), value, isObject);
auto key = it.first();
auto const& value = it.secondRef();
recursiveAddVarImpl(key.toString(), value, isObject, seen);
}
if (isObject) {
m_packetString.append("</struct>");
Expand Down
13 changes: 12 additions & 1 deletion hphp/runtime/ext/wddx/ext_wddx.h
Expand Up @@ -38,7 +38,7 @@ struct WddxPacket : ResourceData {
String packet_end();
bool serialize_value(const Variant& varVariant);
bool recursiveAddVar(const String& varName, const Variant& varVariant,
bool hasVarTag );
bool hasVarTag);

private:
String getWddxEncoded(const String& varType,
Expand All @@ -52,6 +52,17 @@ struct WddxPacket : ResourceData {
const String& varName,
bool hasVarTag);

using ArrayOrObject = Either<ArrayData*,ObjectData*>;
struct EitherHash {
size_t operator()(const ArrayOrObject data) const {
return data.toOpaque();
}
};
using SeenContainers = req::hash_set<ArrayOrObject, EitherHash>;

bool recursiveAddVarImpl(const String& varName, const Variant& varVariant,
bool hasVarTag, SeenContainers& seen);

StringBuffer m_packetString;
bool m_packetClosed;
bool m_manualPacketCreation;
Expand Down
7 changes: 7 additions & 0 deletions hphp/test/slow/ext_wddx/recursion.php
@@ -0,0 +1,7 @@
<?php
$s1 = wddx_serialize_value(function () {});
var_dump($s1);

$a = []; $a[] =& $a;
$s2 = wddx_serialize_value($a);
var_dump($s2);
6 changes: 6 additions & 0 deletions hphp/test/slow/ext_wddx/recursion.php.expectf
@@ -0,0 +1,6 @@

Warning: recursion detected in %s/recursion.php on line 2
string(%d) "<wddxPacket version='1.0'><header/><data><struct><var name='php_class_name'><string>Closure$;%s$%s$</string></var></struct></data></wddxPacket>"

Warning: recursion detected in %s/recursion.php on line 6
string(87) "<wddxPacket version='1.0'><header/><data><array length='1'></array></data></wddxPacket>"

0 comments on commit 1888810

Please sign in to comment.