Zend Framework

Zend_Dom_Query::_getNodeList() mishandles query arrays

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.7.8
  • Fix Version/s: 1.10.0
  • Component/s: Zend_Dom_Query
  • Labels:
    None

Description

Zend_Dom_Query::query() calls Zend_Dom_Query_Css2XPath::transform(), which can return an array. It passes this as $xpathQuery to Zend_Dom_Query::queryXPath(), which passes it to Zend_Dom_Query::_getNodeList(). According to the docblock for this method, $xpathQuery may be a string or an array. However, this method explicitly casts $xpathQuery to a string, which results in that variable being equal to the string "Array" if the original value for $xpathQuery is an array.

In my opinion, it would be best for the sake of consistency to restrict $xpathQuery to be a string for both queryXPath() and _getNodeList() and to modify Zend_Dom_Query_Css2XPath::transform() so that it only ever returns a string.

Issue Links

Activity

Hide
Satoru Yoshida added a comment -

Is it still alive after ZF-6280 fix?

Show
Satoru Yoshida added a comment - Is it still alive after ZF-6280 fix?
Hide
Matthew Turland added a comment -

Yes, this issue is still applicable. ZF-6280 only corrected API docblocks. This deals with corresponding source code. The reason that the two issues are related is because the fix from ZF-6280 (updating the API docblocks to reflect that the parameter/return types could be string or array) affects how this issue should be handled.

If both strings and arrays are to be allowed, line 207 of Zend/Dom/Query.php in trunk (in _getNodeList() where $xpathQuery is typecasted to a string) needs to be modified to handle actually allowing arrays rather than assigning the string 'Array' to $xpathQuery.

As a side note, it seems odd that _getNodeList() is even separated from queryXpath(), as the latter is the only thing that calls the former.

Show
Matthew Turland added a comment - Yes, this issue is still applicable. ZF-6280 only corrected API docblocks. This deals with corresponding source code. The reason that the two issues are related is because the fix from ZF-6280 (updating the API docblocks to reflect that the parameter/return types could be string or array) affects how this issue should be handled. If both strings and arrays are to be allowed, line 207 of Zend/Dom/Query.php in trunk (in _getNodeList() where $xpathQuery is typecasted to a string) needs to be modified to handle actually allowing arrays rather than assigning the string 'Array' to $xpathQuery. As a side note, it seems odd that _getNodeList() is even separated from queryXpath(), as the latter is the only thing that calls the former.
Hide
Victor Gryshko added a comment -

I've resolved this issue on my end (ZF version 1.9.3) by using the following line in Css2Xpath.php (line 52)

return implode(' | ', $expressions);

instead of

return $expressions;

Show
Victor Gryshko added a comment - I've resolved this issue on my end (ZF version 1.9.3) by using the following line in Css2Xpath.php (line 52) return implode(' | ', $expressions); instead of return $expressions;
Hide
Matthew Weier O'Phinney added a comment -

Fixed in trunk and 1.10 release branch

Show
Matthew Weier O'Phinney added a comment - Fixed in trunk and 1.10 release branch

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: