PKP Bugzilla – Bug 6918
Native import plugin reference bug may garble multiple supplementary file import
Last modified: 2011-10-07 11:30:02 PDT
The Native Import plugin contains an interesting bug for which I have a simple fix. The fix is one character, but the explanation is necessarily much longer. The symptom: When importing an article with more than one supplemental file, the second-to-last file will be duplicated in place of the last file. The patch: diff --git a/plugins/importexport/native/NativeImportDom.inc.php b/plugins/importexport/native/NativeImportDom.inc.php index a3c7504..652c5be 100644 --- a/plugins/importexport/native/NativeImportDom.inc.php +++ b/plugins/importexport/native/NativeImportDom.inc.php @@ -870,7 +870,7 @@ class NativeImportDom { $hasErrors = false; $galleyCount = 0; for ($index=0; $index < count($articleNode->children); $index++) { - $node =& $articleNode->children[$index]; + $node = $articleNode->children[$index]; if ($node->getName() == 'htmlgalley') $isHtml = true; elseif ($node->getName() == 'galley') $isHtml = false; The explanation: It boils down to references vs. objects in PHP 5+. Because all the XML nodes being accessed are already objects, it isn't necessary, and in fact doesn't make sense, to take a reference to them as in the original code above. That said, doing so is usually harmless; in this case it's a problem. What happens is that, after executing the loop above, $node ends up being a reference to the last entry of the $articleNode->children array. That entry contains an object identifier, identifying the last child. The problem shows up when the following code iterates through the supplemental_file elements, assigning each in turn to $node. Because $node is a reference to the last array entry, the last array entry gets changed to each supplemental_file element in turn, which is obviously not desirable. Thus on the second-to-last iteration the last element is changed to the second-to-last element; on the last iteration it is then reassigned to itself; the last element is lost forever. The patch makes $node a simple copy of the last array element instead of a reference to it. This is still efficient because we have to remember that in PHP, the array element doesn't contain the entire XML node, only an object identifier for that node. See http://www.php.net/manual/en/language.oop5.references.php for more on that.
Good catch, those are tricky to debug! Also patched OCS for the same issue. Thanks for contributing.
Fixed reference glitch https://github.com/pkp/ocs/commit/8b4e3540de6e8d6f65bfabe01845fb048c57d40a
Fixed reference glitch https://github.com/pkp/ocs/commit/63d294fb38bf750849a2d1c956b642cd6b6b5e45
Fixed reference glitch https://github.com/pkp/ojs/commit/bd0acc5f6802750527ccca4e660c4c27fb49433d