Permalink
Browse files

[Fix] Dont pass invalid keys to ArrayData::remove

Summary: Variant::remove(CVarRef) could end up passing a "null" key to ArrayData::remove if given an invalid key (array or object). In the DEBUG build, this asserted, and in the release build it removed the element with key 0.

The new test case produced incorrect results in the release build, and asserted in the debug build.

Task ID: #768801

Blame Rev:

Reviewers: kma qigao

CC:

Test Plan: fast_tests slow_tests (new test case added).

Revert Plan:

Tags:

Platform Impact (PUBLIC):

Differential Revision: 347370
  • Loading branch information...
1 parent 2d023da commit 57902c00bb44dd97111114dc7285753366aa0caa @markw65 markw65 committed with scottmac Oct 19, 2011
Showing with 10 additions and 1 deletion.
  1. +3 −1 src/runtime/base/type_variant.cpp
  2. +7 −0 src/test/test_code_run.cpp
@@ -3478,7 +3478,9 @@ void Variant::removeImpl(CVarRef key, bool isString /* false */) {
if (isString) {
escalated = arr->remove(key, (arr->getCount() > 1));
} else {
- escalated = arr->remove(key.toKey(), (arr->getCount() > 1));
+ const VarNR &k = key.toKey();
+ if (k.isNull()) return;
+ escalated = arr->remove(k, (arr->getCount() > 1));
}
if (escalated) {
set(escalated);
@@ -7242,6 +7242,13 @@ bool TestCodeRun::TestUnset() {
"goo();\n"
"unset($this);\n"
"var_dump($this);\n");
+
+ MVCR("<?php "
+ "function rmv($a, $b) { unset($a[$b]); return $a; }"
+ "$a = array('foo');"
+ "$b = array();"
+ "var_dump(rmv($a, $b));");
+
return true;
}

0 comments on commit 57902c0

Please sign in to comment.