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,
- The
$condition
variable'AND'
is implicitly cast toarray('A', 'N', 'D')
- The index
['field']
is implicitly cast to integer[0]
- 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.
Comment | File | Size | Author |
---|---|---|---|
#54 | 1414412-54.patch | 1.04 KB | pp |
#36 | 1414412-27.patch | 625 bytes | droplet |
#27 | 1414412-27.patch | 688 bytes | droplet |
#23 | database-query-clone-1414412.patch | 695 bytes | MrHaroldA |
#19 | database-query-clone-1414412-19.patch | 1.08 KB | pillarsdotnet |
Comments
Comment #1
pillarsdotnet CreditAttribution: pillarsdotnet commentedHere are the patches for 8.x and 7.x.
Comment #3
pillarsdotnet CreditAttribution: pillarsdotnet commentedAlternative approach simply checks for the
'field'
element before accessing it.Comment #3.0
pillarsdotnet CreditAttribution: pillarsdotnet commentedfixed typo.
Comment #4
pillarsdotnet CreditAttribution: pillarsdotnet commentedFixed typo.
Comment #5
Damien Tournoud CreditAttribution: Damien Tournoud commentedThat seems to make sense, but instructions on how to reproduce would be nice. I never seen this error before.
Comment #6
pillarsdotnet CreditAttribution: pillarsdotnet commentedThe 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.
Comment #7
pillarsdotnet CreditAttribution: pillarsdotnet commentedAttached are my phpinfo, settings.php, and nginx.conf files.
Comment #8
pillarsdotnet CreditAttribution: pillarsdotnet commentedI could also provide a database dump if you like.
Comment #9
Damien Tournoud CreditAttribution: Damien Tournoud commentedOk, 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).Comment #10
pillarsdotnet CreditAttribution: pillarsdotnet commentedRe-rolled as requested.
Comment #11
pillarsdotnet CreditAttribution: pillarsdotnet commentedGrr... (WHY OH WHY DO WE HAVE TRAILING SPACES ALL OVER CORE????)
Re-rolled without the gratuitous space changes.
Comment #12
pillarsdotnet CreditAttribution: pillarsdotnet commented(sigh)
Comment #13
pillarsdotnet CreditAttribution: pillarsdotnet commentedTests blocked by #61456-86Yay! HEAD is un-b0rken!
Comment #15
pillarsdotnet CreditAttribution: pillarsdotnet commented#4: database-query-clone-1414412-4.patch queued for re-testing.
Comment #16
pillarsdotnet CreditAttribution: pillarsdotnet commented#11: database-query-clone-1414412-11.patch queued for re-testing.
Comment #18
pillarsdotnet CreditAttribution: pillarsdotnet commentedOkay, so #4 is technically wrong but passes tests.
But #11, while technically correct, fails tests.
Seems like I've seen this situation before...
Comment #19
pillarsdotnet CreditAttribution: pillarsdotnet commentedMust convert keys to string before comparing, because
( (int) 0 == '#conjunction' )
evaluates toTRUE
.Comment #20
msonnabaum CreditAttribution: msonnabaum commentedI also ran into this on php 5.4.
Patch in #19 works, but why not just use !== instead of casting to a string?
Comment #21
andrewia CreditAttribution: andrewia commentedYou 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!
Comment #22
rmnanney CreditAttribution: rmnanney commentedI 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
Comment #23
MrHaroldA CreditAttribution: MrHaroldA commentedI'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.
Comment #24
PedroMiguel CreditAttribution: PedroMiguel commentedTested 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).
Comment #25
PedroMiguel CreditAttribution: PedroMiguel commentedRe-tested patch on #23 (now after sleep) and all works ok, sorry guys.
Comment #26
aspilicious CreditAttribution: aspilicious commentedI got the same question as #20
Comment #27
droplet CreditAttribution: droplet commentedsee #19 reason, seems we can use strict comparison.
Comment #28
pillarsdotnet CreditAttribution: pillarsdotnet commented+1 for #27 (would RTBC if I could).
Comment #29
aspilicious CreditAttribution: aspilicious commentedI can
Comment #30
webchickAssigning to Crell for a sanity-check.
Comment #30.0
webchickRevised approach.
Comment #30.1
pillarsdotnet CreditAttribution: pillarsdotnet commentedUpdated to reflect current status.
Comment #30.2
pillarsdotnet CreditAttribution: pillarsdotnet commentedAdd pointer to Condition::__clone() docs.
Comment #30.3
pillarsdotnet CreditAttribution: pillarsdotnet commentedExplicitly show original and changed code.
Comment #30.4
pillarsdotnet CreditAttribution: pillarsdotnet commentedExplain why this fix is technically only necessary in PHP 5.4 and later.
Comment #31
pillarsdotnet CreditAttribution: pillarsdotnet commentedUpdated issue summary to make it crystal clear why this is happening only in PHP 5.4.
Comment #32
Damien Tournoud CreditAttribution: Damien Tournoud commentedThat's sane.
Comment #33
Crell CreditAttribution: Crell commentedI recall when this discussion came up on the internals list. This is the correct fix.
Thanks, pillarsdotnet for the excellent summary!
Comment #34
webchickGreat, 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.
Comment #35
webchickAnd since that means literally changing one line of code, tagging as novice.
Comment #36
droplet CreditAttribution: droplet commentedComment #36.0
droplet CreditAttribution: droplet commentedAdjust initial versions to the minimum necessary to produce the reported error.
Comment #37
JamesOakleyApplied 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.
Comment #37.0
JamesOakleyUpdate status.
Comment #38
webchickLooks good, thanks James!
Committed and pushed to 7.x.
Comment #39
ROMEO CreditAttribution: ROMEO commentedHy 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 :( !!!
Comment #40
Crell CreditAttribution: Crell commentedROMEO: 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.)
Comment #41
ROMEO CreditAttribution: ROMEO commentedThe 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
Comment #42
Crell CreditAttribution: Crell commentedROMEO: 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.
Comment #43
ROMEO CreditAttribution: ROMEO commentedThx i will try!
Comment #44
pillarsdotnet CreditAttribution: pillarsdotnet commentedYou 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.)
Comment #44.0
pillarsdotnet CreditAttribution: pillarsdotnet commentedupdate.
Comment #45
ROMEO CreditAttribution: ROMEO commentedThx everyone for support i replace the file and now all works fine :) !
Comment #46
Sylvain_G CreditAttribution: Sylvain_G commentedSubscribing
Comment #47
archit.lohokare CreditAttribution: archit.lohokare commented#4: database-query-clone-1414412-4.patch queued for re-testing.
Comment #48
archit.lohokare CreditAttribution: archit.lohokare commented#4: database-query-clone-1414412-4-d7.patch queued for re-testing.
Comment #49
JamesOakleyComment #50
illusionista_86 CreditAttribution: illusionista_86 commented#27: 1414412-27.patch queued for re-testing.
Comment #51
JamesOakleyComment #52
kraigory CreditAttribution: kraigory commentedPatch #36 also worked for me- thanks!
Comment #53
marptr CreditAttribution: marptr commentedsvp disregard this comment, in light of Patch #36 which is working.
Comment #54
pp CreditAttribution: pp commentedThe problem exists when install Drupal, and use php 5.4.0 built in webserver.
I take a following steps:
Use PHP bulit in webserver:
go browser: localhost:9999/install.php
use minimal profile and SQLite
I got a following errors:
I made a patch which resolve this problem.
Comment #55
pillarsdotnet CreditAttribution: pillarsdotnet commented@#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.
Comment #56
pp CreditAttribution: pp commented@#55 by pillarsdotnet
You are absolutely right, here it is the re-rolled patch and separate issue:
http://drupal.org/node/1542186#comment-5901876
Comment #57
pk.long CreditAttribution: pk.long commentedComment #58
droplet CreditAttribution: droplet commented@pk.long,
Please submit your new issue to media module issue list.
Comment #59
JamesOakleyReverting component
Comment #60
coolsandythombare CreditAttribution: coolsandythombare commentedPatch #36 Works for me.
Thanks a lot.
Comment #61
amonteroGetting 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.
Comment #62
steverweber CreditAttribution: steverweber commentedPHP 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 !!
Comment #63
steverweber CreditAttribution: steverweber commentedreopened
Comment #64
steverweber CreditAttribution: steverweber commentedopps disregard last 2 comments, was on an old branch.
Comment #65
ZiggyQubert CreditAttribution: ZiggyQubert commentedLooks 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']]);
}
}
}
}
Comment #66
Anonymous (not verified) CreditAttribution: Anonymous commentedHi and thks for this patch, too work for me !
Comment #67
Heine CreditAttribution: Heine commentedThis completely blocks install on SQLite3 with profiles standard and minimal.
Comment #68
droplet CreditAttribution: droplet commentedSQLite: #1266572: Workaround in UpdateQuery_sqlite for affected rows count causes certain updates to be suppressed
Comment #69
apotek CreditAttribution: apotek commentedAdding tag "needs backport to D6". Problem exists there too.
Comment #69.0
apotek CreditAttribution: apotek commentedProblem is fixed.
Comment #74
Sutharsan CreditAttribution: Sutharsan commentedRemoving Novice. Better not expose this issue history to a novice ;)
Comment #78
daffie CreditAttribution: daffie commentedComment #79
daffie CreditAttribution: daffie commented