Problem/Motivation

On a Minimal install of Drupal (7.x or later) and PHP (5.4 or later), I get the following on-screen error:

  • Warning: Illegal string offset 'field' in DatabaseCondition->__clone() (line 1901 of /var/www/includes/database/query.inc).

After inserting a debug_backtrace() line, I discovered that the $this->conditions array had the following contents:

[conditions:protected] => Array
  (
    [#conjunction] => AND
    [0] => Array
      (
        [field] => module
        [value] => system
        [operator] => =
      )
    [1] => Array
      (
        [field] => menu_name
        [value] => management
        [operator] => =
      )
  )

Apparently, the initial '#conjunction' element is causing the error, because it lacks the 'field' subelement presumed to exist by the Condition::__clone() method:

function __clone() {
  $this->changed = TRUE;
  foreach ($this->conditions as $key => $condition) {
    if ($condition['field'] instanceOf ConditionInterface) {
      $this->conditions[$key]['field'] = clone($condition['field']);
    }
  }
}

Proposed resolution

According to Damien Tournoud in #9, the correct fix is to explicitly skip the '#condition' element.

function __clone() {
  $this->changed = TRUE;
  foreach ($this->conditions as $key => $condition) {
    if ($key !== '#conjunction' && $condition['field'] instanceOf ConditionInterface) {
      $this->conditions[$key]['field'] = clone($condition['field']);
    }
  }
}

The original function only worked because PHP (5.3 and earlier) went to extreme lengths to treat a string as an array of character, such as in the following code example.

  $condition = 'AND';
  echo $condition['field'];

In PHP 5.3 and earlier,

  1. The $condition variable 'AND' is implicitly cast to array('A', 'N', 'D')
  2. The index ['field'] is implicitly cast to integer [0]
  3. The echo statement then prints the letter 'A' as the first character (element) of the $condition string (array).

The same code in PHP 5.4 produces the runtime error reported by this issue.

Remaining tasks

None; the fix will be available in the next point release.

If you have any further questions, please visit the support forums or open a separate issue.

User interface changes

None.

API changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pillarsdotnet’s picture

Status: Active » Needs review
FileSize
1.03 KB
1.05 KB

Here are the patches for 8.x and 7.x.

Status: Needs review » Needs work

The last submitted patch, database-query-clone.diff, failed testing.

pillarsdotnet’s picture

Status: Needs work » Needs review
FileSize
1.04 KB
1.06 KB

Alternative approach simply checks for the 'field' element before accessing it.

pillarsdotnet’s picture

Issue summary: View changes

fixed typo.

pillarsdotnet’s picture

Damien Tournoud’s picture

Title: Filter the condition array with element_children() in __clone() function of database/query.inc » Bogus traversal of the ondition array in __clone() function of database/query.inc
Status: Needs review » Postponed (maintainer needs more info)

That seems to make sense, but instructions on how to reproduce would be nice. I never seen this error before.

pillarsdotnet’s picture

Title: Bogus traversal of the ondition array in __clone() function of database/query.inc » Bogus traversal of the condition array in __clone() function of database/query.inc
@Damien Tournoud
instructions on how to reproduce would be nice.

The fact that I'm running nginx and php-fpm rather than apache might be relevant. I have no idea how to write a test case. If you like, I can write a shell script to download/build/install the latest versions of nginx, php, mariadb, and drupal. I have not tested with other environments; I'm just annoyed that every single page load contains 20-50 user-visible error notices.

pillarsdotnet’s picture

Status: Postponed (maintainer needs more info) » Needs review
FileSize
18.46 KB

Attached are my phpinfo, settings.php, and nginx.conf files.

pillarsdotnet’s picture

I could also provide a database dump if you like.

Damien Tournoud’s picture

Status: Needs review » Needs work

Ok, that's a PHP 5.4 specific error. It does make sense, in PHP 5.3 we would get the first character of the conjunction string.

The fix is not correct: we need to check if $key is different then #conjunction, as it is done elsewhere (for example in ::compile() where the #conjunction is removed before iterating).

pillarsdotnet’s picture

Title: Bogus traversal of the condition array in __clone() function of database/query.inc » Skip #conjunction key in __clone() method of core/includes/database/query.inc
Status: Needs work » Needs review
FileSize
3.35 KB
3.37 KB

Re-rolled as requested.

pillarsdotnet’s picture

Grr... (WHY OH WHY DO WE HAVE TRAILING SPACES ALL OVER CORE????)

Re-rolled without the gratuitous space changes.

pillarsdotnet’s picture

pillarsdotnet’s picture

Tests blocked by #61456-86
Yay! HEAD is un-b0rken!

Status: Needs review » Needs work
Issue tags: -Needs backport to D7

The last submitted patch, database-query-clone-1414412-11.patch, failed testing.

pillarsdotnet’s picture

Status: Needs work » Needs review
pillarsdotnet’s picture

Status: Needs review » Needs work
Issue tags: +Needs backport to D7

The last submitted patch, database-query-clone-1414412-11.patch, failed testing.

pillarsdotnet’s picture

Okay, so #4 is technically wrong but passes tests.
But #11, while technically correct, fails tests.

Seems like I've seen this situation before...

pillarsdotnet’s picture

Status: Needs work » Needs review
FileSize
1.08 KB

Must convert keys to string before comparing, because ( (int) 0 == '#conjunction' ) evaluates to TRUE.

msonnabaum’s picture

Status: Needs review » Needs work

I also ran into this on php 5.4.

Patch in #19 works, but why not just use !== instead of casting to a string?

andrewia’s picture

You fixed my error! I'm an absolute n00b, but after some guesswork, I manually patched the file in my installation. Now, Drupal 7 is error message-free! Many thanks!

rmnanney’s picture

I can reproduce this on my Gentoo box as well, on Apache 2.2.21 and PHP Version 5.4.0-rc6-pl0-gentoo. I have
D7.12 installed and have manually merged in the changes from #19, works great.

Regards,
Ryan

MrHaroldA’s picture

Status: Needs work » Needs review
FileSize
695 bytes

I've encountered the same error using PHP 5.4-rc8 and Drupal 7.12. Manually patching includes/database/query.inc with the line in #19 solved the error.

I've rerolled the patch in #19 against today's Drupal 8 git checkout.

PedroMiguel’s picture

Tested manual patch on #19 and #23 on Drupal 7.12 with PHP 5.4 ("final") and got this errors:

Warning: Illegal string offset 'field' in DatabaseCondition->__clone() (line 1902 of /home/xxxxx/public_html/includes/database/query.inc).

Notice: Undefined variable: key in DatabaseCondition->compile() (line 1817 of /home/xxxxx/public_html/includes/database/query.inc).

PedroMiguel’s picture

Re-tested patch on #23 (now after sleep) and all works ok, sorry guys.

aspilicious’s picture

I also ran into this on php 5.4.
Patch in #19 works, but why not just use !== instead of casting to a string?

I got the same question as #20

droplet’s picture

FileSize
688 bytes

see #19 reason, seems we can use strict comparison.

pillarsdotnet’s picture

+1 for #27 (would RTBC if I could).

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

I can

webchick’s picture

Assigned: Unassigned » Crell

Assigning to Crell for a sanity-check.

webchick’s picture

Issue summary: View changes

Revised approach.

pillarsdotnet’s picture

Issue summary: View changes

Updated to reflect current status.

pillarsdotnet’s picture

Issue summary: View changes

Add pointer to Condition::__clone() docs.

pillarsdotnet’s picture

Issue summary: View changes

Explicitly show original and changed code.

pillarsdotnet’s picture

Issue summary: View changes

Explain why this fix is technically only necessary in PHP 5.4 and later.

pillarsdotnet’s picture

Updated issue summary to make it crystal clear why this is happening only in PHP 5.4.

Damien Tournoud’s picture

That's sane.

Crell’s picture

I recall when this discussion came up on the internals list. This is the correct fix.

Thanks, pillarsdotnet for the excellent summary!

webchick’s picture

Version: 8.x-dev » 7.x-dev
Assigned: Crell » Unassigned
Status: Reviewed & tested by the community » Patch (to be ported)

Great, thanks! (And thanks to Damien too, but since it was your original suggestion I was just looking for an extra +1 from another DB system maintainer to be safe. :))

Committed and pushed to 8.x. This needs a small re-roll for D7, since lib/Drupal/Core/Database/Query/Condition.php does not exist there.

webchick’s picture

Issue tags: +Novice

And since that means literally changing one line of code, tagging as novice.

droplet’s picture

Status: Patch (to be ported) » Needs review
FileSize
625 bytes
droplet’s picture

Issue summary: View changes

Adjust initial versions to the minimum necessary to produce the reported error.

JamesOakley’s picture

Status: Needs review » Reviewed & tested by the community

Applied patch #36 to my installation that was triggering this error in the dblog. After applying the patch, and clearing the caches (to be safe), no more such errors appearing. Marking RTBC - please put it back to CNR if it needs more users to verify the patch before committing.

JamesOakley’s picture

Issue summary: View changes

Update status.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Looks good, thanks James!

Committed and pushed to 7.x.

ROMEO’s picture

Hy guys! I have the same problem,i have drupal 7 installed on my server and i got same erorr on screen Warning: Illegal string offset 'field' in DatabaseCondition->__clone() (line 1901 of /htdocs/site/includes/database/query.inc). how can i apply this pach? i must edit the file query.inc ? Thx, and sorry for my bad english :( !!!

Crell’s picture

ROMEO: See http://drupal.org/patch, or download the latest dev snapshot of Drupal 7, or just wait for the next stable release of it. (That usually happens the last Wednesday of the month.)

ROMEO’s picture

The only problem is that i have drupal installed on 1 sever wich i dont have direct acces...so i cant run those commands to modify the query.inc file. If only the query.inc file should be modified can someone upload the corect modified query,inc file pls, so i can upload in server.THX

Crell’s picture

ROMEO: That's now a general support request. Please see http://drupal.org/support

In general you should be able to patch the file locally yourself and then upload the modified file.

ROMEO’s picture

Thx i will try!

pillarsdotnet’s picture

You can also browse to the latest version of any file in the repository by starting at http://drupalcode.org/project/drupal.git/tree/refs/heads/7.x

(Right-click on the "raw" link and then click on "save as" to download a file.)

pillarsdotnet’s picture

Issue summary: View changes

update.

ROMEO’s picture

Thx everyone for support i replace the file and now all works fine :) !

Sylvain_G’s picture

Subscribing

archit.lohokare’s picture

Status: Fixed » Needs review
Issue tags: -Novice, -Needs backport to D7
archit.lohokare’s picture

JamesOakley’s picture

Status: Needs review » Fixed
illusionista_86’s picture

Status: Fixed » Needs review

#27: 1414412-27.patch queued for re-testing.

JamesOakley’s picture

Status: Needs review » Fixed
kraigory’s picture

Patch #36 also worked for me- thanks!

marptr’s picture

svp disregard this comment, in light of Patch #36 which is working.

pp’s picture

Status: Fixed » Needs review
FileSize
1.04 KB

The problem exists when install Drupal, and use php 5.4.0 built in webserver.

I take a following steps:

git clone --branch 7.x http://git.drupal.org/project/drupal.git
cd drupal

Use PHP bulit in webserver:

/usr/local/bin/php -S <code>localhost:9999

go browser: localhost:9999/install.php
use minimal profile and SQLite

I got a following errors:

Warning: Illegal string offset 'field' in UpdateQuery_sqlite->removeFieldsInCondition() (line 75 of /Users/pp/Work/temp/drupal/includes/database/sqlite/query.inc).

Warning: Illegal string offset 'field' in UpdateQuery_sqlite->removeFieldsInCondition() (line 79 of /Users/pp/Work/temp/drupal/includes/database/sqlite/query.inc).

I made a patch which resolve this problem.

pillarsdotnet’s picture

Status: Needs review » Closed (fixed)

@#54 by pp:

Please re-roll your patch against Drupal 8.x and post it in a separate issue. The originally reported problem in this issue has been fixed.

pp’s picture

@#55 by pillarsdotnet

You are absolutely right, here it is the re-rolled patch and separate issue:
http://drupal.org/node/1542186#comment-5901876

pk.long’s picture

Title: Skip #conjunction key in __clone() method of core/includes/database/query.inc » Illegal string offset 'fid' in media_element_validate() (line 1000 of C:\xampp\htdocs\drupal\sites\all\modules\media\media.m
Component: database system » file system
Status: Closed (fixed) » Needs review
droplet’s picture

Title: Illegal string offset 'fid' in media_element_validate() (line 1000 of C:\xampp\htdocs\drupal\sites\all\modules\media\media.m » Skip #conjunction key in __clone() method of core/includes/database/query.inc
Status: Needs review » Closed (fixed)

@pk.long,

Please submit your new issue to media module issue list.

JamesOakley’s picture

Component: file system » database system

Reverting component

coolsandythombare’s picture

Patch #36 Works for me.
Thanks a lot.

amontero’s picture

Getting the same warning as the OP message on a Drupal 7.8 when moving (MySQL, PHP 5.4).
Applied #36 and the warning went away.

steverweber’s picture

PHP 5.4.9-4

Warning: Illegal string offset 'field' in DatabaseCondition->__clone() (line 1895 of /home/s8weber/svc/drupal/srv/http/drupal/includes/database/query.inc).

can a patch be pushed to 7.x-dev?

!! disregard was on an old branch !!

steverweber’s picture

Status: Closed (fixed) » Needs work

reopened

steverweber’s picture

Status: Needs work » Closed (fixed)

opps disregard last 2 comments, was on an old branch.

ZiggyQubert’s picture

Status: Closed (fixed) » Needs work

Looks like this needs to be ported to other database types, spicificly this still fails if using a SQLite database, I fixed it localy by changing this function in \includes\database\sqlite\query.inc

protected function removeFieldsInCondition(&$fields, QueryConditionInterface $condition) {
foreach ($condition->conditions() as $key=>$child_condition) {
if ($key !== '#conjunction')
{
if ($child_condition['field'] instanceof QueryConditionInterface) {
$this->removeFieldsInCondition($fields, $child_condition['field']);
}
else {
unset($fields[$child_condition['field']]);
}
}
}
}

Anonymous’s picture

Hi and thks for this patch, too work for me !

Heine’s picture

Priority: Normal » Critical

This completely blocks install on SQLite3 with profiles standard and minimal.

droplet’s picture

apotek’s picture

Issue tags: +Needs backport to D6

Adding tag "needs backport to D6". Problem exists there too.

apotek’s picture

Issue summary: View changes

Problem is fixed.

Status: Closed (fixed) » Needs review

kakemonster queued 54: 1414412-54.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 54: 1414412-54.patch, failed testing.

  • webchick committed f9c539a on 8.3.x
    Issue #1414412 by pillarsdotnet, MrHaroldA, droplet: Fixed Skip #...

  • webchick committed f9c539a on 8.3.x
    Issue #1414412 by pillarsdotnet, MrHaroldA, droplet: Fixed Skip #...
Sutharsan’s picture

Issue tags: -Novice

Removing Novice. Better not expose this issue history to a novice ;)

  • webchick committed f9c539a on 8.4.x
    Issue #1414412 by pillarsdotnet, MrHaroldA, droplet: Fixed Skip #...

  • webchick committed f9c539a on 8.4.x
    Issue #1414412 by pillarsdotnet, MrHaroldA, droplet: Fixed Skip #...

  • webchick committed f9c539a on 9.1.x
    Issue #1414412 by pillarsdotnet, MrHaroldA, droplet: Fixed Skip #...
daffie’s picture

Status: Needs work » Fixed
daffie’s picture

Status: Fixed » Closed (fixed)