diff --git a/php/ext/google/protobuf/array.c b/php/ext/google/protobuf/array.c index 72c7809674c6..09daffa16129 100644 --- a/php/ext/google/protobuf/array.c +++ b/php/ext/google/protobuf/array.c @@ -406,13 +406,13 @@ PHP_METHOD(RepeatedField, offsetUnset) { return; } - if (size == 0 || index != size - 1) { + if (size == 0 || index < 0 || index >= size) { php_error_docref(NULL, E_USER_ERROR, "Cannot remove element at %ld.\n", index); return; } - upb_Array_Resize(intern->array, size - 1, Arena_Get(&intern->arena)); + upb_Array_Delete(intern->array, index, 1); } /** diff --git a/php/src/Google/Protobuf/Internal/RepeatedField.php b/php/src/Google/Protobuf/Internal/RepeatedField.php index ea7971f134fe..f6ecd1c84a95 100644 --- a/php/src/Google/Protobuf/Internal/RepeatedField.php +++ b/php/src/Google/Protobuf/Internal/RepeatedField.php @@ -219,13 +219,13 @@ public function offsetSet($offset, $value) public function offsetUnset($offset) { $count = count($this->container); - if (!is_numeric($offset) || $count === 0 || $offset !== $count - 1) { + if (!is_numeric($offset) || $count === 0 || $offset < 0 || $offset >= $count) { trigger_error( "Cannot remove element at the given index", E_USER_ERROR); return; } - array_pop($this->container); + array_splice($this->container, $offset, 1); } /** diff --git a/php/tests/ArrayTest.php b/php/tests/ArrayTest.php index 9e8fcb8beada..2cade79026ea 100644 --- a/php/tests/ArrayTest.php +++ b/php/tests/ArrayTest.php @@ -655,4 +655,31 @@ public function testClone() $arr2 = clone $arr; $this->assertSame($arr[0], $arr2[0]); } + + ######################################################### + # Test offsetUnset + ######################################################### + + public function testOffsetUnset() + { + $arr = new RepeatedField(GPBType::INT32); + $arr[] = 0; + $arr[] = 1; + $arr[] = 2; + + $this->assertSame(3, count($arr)); + $this->assertSame(0, $arr[0]); + $this->assertSame(1, $arr[1]); + $this->assertSame(2, $arr[2]); + + $arr->offsetUnset(1); + $this->assertSame(0, $arr[0]); + $this->assertSame(2, $arr[1]); + + $arr->offsetUnset(0); + $this->assertSame(2, $arr[0]); + + $arr->offsetUnset(0); + $this->assertCount(0, $arr); + } }