Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

View: correctly validate input in PartialLoop #4546

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 8 additions & 16 deletions library/Zend/View/Helper/PartialLoop.php
Expand Up @@ -44,22 +44,14 @@ public function __invoke($name = null, $values = null)
return $this;
}

if (!is_array($values)
&& (!$values instanceof Traversable)
&& (is_object($values) && !method_exists($values, 'toArray'))
) {
throw new Exception\InvalidArgumentException('PartialLoop helper requires iterable data');
}

if (is_object($values)
&& (!$values instanceof Traversable)
&& method_exists($values, 'toArray')
) {
$values = $values->toArray();
}

if ($values instanceof Iterator) {
$values = ArrayUtils::iteratorToArray($values);
if (!is_array($values)) {
if ($values instanceof Traversable) {
$values = ArrayUtils::iteratorToArray($values);
} elseif (is_object($values) && method_exists($values, 'toArray')) {
$values = $values->toArray();
} else {
throw new Exception\InvalidArgumentException('PartialLoop helper requires iterable data');
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe wrap with if (!is_array($values)) {

if (!is_array($values)) {
    if ($values instanceof Traversable) {
        $values = ArrayUtils::iteratorToArray($values);
    } else if (is_object($values) && method_exists($values, 'toArray')) {
        $values = $values->toArray();
    } else {
        throw new Exception\InvalidArgumentException('PartialLoop helper requires iterable data');
    }
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. Updated.


// reset the counter if it's called again
Expand Down
16 changes: 16 additions & 0 deletions tests/ZendTest/View/Helper/PartialLoopTest.php
Expand Up @@ -156,6 +156,22 @@ public function testPartialLoopThrowsExceptionWithBadIterator()
}
}

/**
* @return void
*/
public function testPassingNullDataThrowsExcpetion()
{
$view = new View();
$view->resolver()->addPath($this->basePath . '/application/views/scripts');
$this->helper->setView($view);

try {
$result = $this->helper->__invoke('partialLoop.phtml', null);
$this->fail('PartialLoop should throw exception when passed null data.');
} catch (\Zend\View\Exception\InvalidArgumentException $e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use $this->setExpectedException() here instead of a try/catch. I'll do this when merging.

}
}

public function testPassingNoArgsReturnsHelperInstance()
{
$test = $this->helper->__invoke();
Expand Down