If there is no data in the arguments column of the migrate_status table, unserialize($row->arguments) will return FALSE, and FALSE (instead of the expected array) will be passed to getInstance() as the third parameter.

Proposing the following change:

<?php
- $migration = MigrationBase::getInstance($row->machine_name,
-         $row->class_name, unserialize($row->arguments));
+ $arguments = unserialize($row->arguments);
+ if (!$arguments || !is_array($arguments)) {
+   $arguments = array();
+ }
+ $migration = MigrationBase::getInstance($row->machine_name,
+          $row->class_name, $arguments);

?>

If you agree, I'm happy to roll the patch.

CommentFileSizeAuthor
#1 arg3_as_array-1595056-1.patch1.32 KBapotek

Comments

apotek’s picture

apotek commented
Status: Active » Needs review
StatusFileSize
new arg3_as_array-1595056-1.patch1.32 KB

Here 'tis.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented

I was going to suggest just an update function to make sure arguments was always properly populated, but it looks like there's the potential for beginProcess() to lazy-create rows without arguments, so this looks good, I'll test it in a bit...

Thanks.

apotek’s picture

apotek commented

Having second thoughts already. Wondering if it would be better to cast the $arguments to an array rather than reset to an empty array. Hmmm.

This is bulky, but perhaps the safest:

<?php
        $arguments = unserialize($row->arguments);
        if (!$arguments) {
          $arguments = array();
        }
        elseif (!is_array($arguments)) {
          $arguments = (array) $arguments;
        }
?>

What happens if we simply do this?:

$arguments = (array) unserialize($row->arguments);

which could create an array like so:

array(1) {
  [0]=>
  bool(false)
}

Is that so bad? Might that have unintended consequences if one of the arguments is empty or boolean?

Let me know if you would prefer me to leave as is, or roll a new patch with either one of the options above.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs review » Fixed

I've committed the original patch, it looks fine. I don't see any way the arguments could be anything other than empty or a serialized array.

apotek’s picture

apotek commented

Great! Thanks mikeryan

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

clarified the introductory sentence.