I was getting a lot of php warnings about an illegal offset in pathauto_cleanstring(). After a little bit of testing I found out that in some cases $string can be an array.

So I modified the top of pathauto_cleanstring() in pathauto.inc to check if the string is an array.

  // Empty strings do not need any proccessing.
  if ($string === '' || $string === NULL || is_array($string)) {
    return '';
  }

Is there any problem with this? I don't see how an array would ever be an acceptable value for $string.

Thanks.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

GDI’s picture

I`ve had the same php warnings during the new blog entry creation

warning: Illegal offset type in isset or empty in
sites/all/modules/pathauto/pathauto.inc on line 91.
warning: strip_tags() expects parameter 1 to be string, array given in
sites/all/modules/pathauto/pathauto.inc on line 96.
warning: Illegal offset type in
sites/all/modules/pathauto/pathauto.inc on line 176.

Your solution helps me.
These warnings had appeared when my host provider had performed upgrade to php 5.4.

Dave Reid’s picture

Status: Active » Postponed (maintainer needs more info)

Something being passed that is not a string to pathauto_cleanstring() means that you have some kind of larger problem with data and you'd need to try and debug where it is coming from. Typically it means you have two different modules trying to use two different values for the same token name, and the module_invoke_all() does an array merge which converts the token's value to an array rather than a string.

Ken Ficara’s picture

Version: 6.x-1.6 » 7.x-1.1
Status: Postponed (maintainer needs more info) » Needs review
FileSize
465 bytes

Just experienced the exact same problem. Once again, there is indeed a larger problem, but let's still fix this code so it works even if someone passes it bad data. Patch attached.

Status: Needs review » Needs work

The last submitted patch, pathauto-1604766.patch, failed testing.

Dave Reid’s picture

I would rather have us investigate what the problem is so we can fix it rather than ignore the problem through supporting bad input.

Ken Ficara’s picture

Status: Needs work » Needs review
FileSize
497 bytes

I rerolled the patch properly against 7.x-1.x-dev and also improved the check. Dave, I agree with you on the need to fix whatever upstream module is passing the bad data, but why not harden pathauto against such eventualities? It seems to be happening on multiple sites, so I suspect it is another contributed module rather than someone's custom code. Perhaps I'm overly cautious but I always lean towards belt-and-suspenders error checking. Thanks for looking into this, in any case.

Status: Needs review » Needs work

The last submitted patch, pathauto-cleanstring-1604766-6.patch, failed testing.

kaidjohnson’s picture

Version: 7.x-1.1 » 7.x-1.x-dev

After some quick investigation, this issue appears to be coming from (at least from what I can see on our site), the workflow module. Tracing the development of the variable $string back a few functions, I get this:

Array
(
[[node:title]] => Node Title
[[node:nodehierarchy:parent:title]] => About Us
[workflow-name] => Array
(
[0] => Content
[1] => Content
)

[workflow-current-state-name] => Array
(
[0] => (creation)
[1] => Publish
)

[workflow-old-state-name] => Array
(
[0] => (creation)
[1] => Publish
)

[workflow-current-state-date-iso] => Array
(
[0] => 20121101055108
[1] => 20121101055108
)

[workflow-current-state-date-tstamp] => Array
(
[0] => 1351806668
[1] => 1351806668
)

[workflow-current-state-date-formatted] => Array
(
[0] => Nov 01, 2012 05:51:08
[1] => Nov 01, 2012 05:51:08
)

[workflow-current-state-updating-user-name] => Array
(
[0] => admin
[1] => admin
)

[workflow-current-state-updating-user-uid] => Array
(
[0] => 1
[1] => 1
)

[workflow-current-state-updating-user-mail] => Array
(
[0] =>
[1] =>
)

[workflow-current-state-log-entry] => Array
(
[0] =>
[1] =>
)

)

Each of those keys gets sent to the parser as a sring value, but as you can see, workflow sends them out as an array with an improperly formatted key. It appears that workflow has not properly followed guidelines for the tokens api, documented here: http://drupal.org/node/1418622 The patch (#17) against Workflow 7.x-1.0 at the bottom looks as though it would solve this issue (though I haven't [yet] tested it on our setup). I strongly advise against the proposed pathauto patches above and recommend further investigating the real cause of this issue (be it Workflow or another module failing to conform to the standards set up by the Tokens API).

iaha’s picture

I am definitely in favor of fixing the real problem(s) and leaving the pathauto_cleanstring() function alone. The pathauto_cleanstring() function clearly expects a string, so any error generated when a non-string value being passed is necessary and useful.

If the patch in #6 was applied, the indication of a problem would disappear, but the problem itself would not. That would make unexpected behaviors harder to troubleshoot and further enable the practices that are creating these issues in the first place.

Dave Reid’s picture

Status: Needs work » Fixed

Let's fix this in Workflow module then.

Status: Fixed » Closed (fixed)

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