Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
A small bug exists in drupal_write_record() failing to convert an array passed in as $object back to array format should drupal_get_schema() fail for any reason. Although drupal_get_schema() is not meant to return FALSE for any reason drupal_write_record() is still meant to live up to expectation and leave $object in it's original state. Patch attached.
Comment | File | Size | Author |
---|---|---|---|
#8 | common-drupal-write-record-200824-8.patch | 1.45 KB | cburschka |
#6 | drupal_write_record_0.patch | 1.29 KB | Anonymous (not verified) |
#3 | drupal_write_record.patch | 1.3 KB | Anonymous (not verified) |
drupal_write_record_consistency.200712141624.patch | 482 bytes | sammys | |
Comments
Comment #1
triclops CreditAttribution: triclops commented+1
Comment #2
sammys CreditAttribution: sammys commentedBumping status...
Comment #3
Anonymous (not verified) CreditAttribution: Anonymous commentedupdated patch to account for code committed here.
Comment #4
Pancho+1. This looks very good.
Comment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedBump.
Patch still applies cleanly to head. Is there a reason this is not going in other than time?
Comment #6
Anonymous (not verified) CreditAttribution: Anonymous commentedUpdated patch to keep up with HEAD.
Comment #7
chx CreditAttribution: chx commentedReason: we need people to test this.
Comment #8
cburschkaPatch broken by a trivial conflict. Here's a re-roll.
As an aside, does this concept code actually work?
Because that's what this "avoid confusing the caller" code seems to do. Does (object) and (array) casting work in-place in spite of the assignment?
Comment #9
sammys CreditAttribution: sammys commentedThanks for the reroll. :) PHP does work that way. Here is what happens when I execute your code above with PHP on the command line.
Comment #10
sammys CreditAttribution: sammys commentedbumping to 6.1
Comment #11
yched CreditAttribution: yched commentedThis should be bumped to D7, actually.
Should be fixed there first, then backported
Comment #12
vladimir.dolgopolov CreditAttribution: vladimir.dolgopolov commentedHere is the test for Simpletest module.
I cannot reproduce the situation here: http://drupal.org/node/207170 since drupal_write_record() generate a valid query for core shemas.
Comment #13
Gábor Hojtsy@Vladimir, how is a random name results in valid queries? Or do I misunderstand something here?
Comment #14
vladimir.dolgopolov CreditAttribution: vladimir.dolgopolov commentedThe point is:
So if drupal_get_schema() fails (no schema found because of a random name) then drupal_write_record() won't convert "array" to "object" in any case.
Comment #15
bjaspan CreditAttribution: bjaspan commentedI re-discovered this bug, applied the patch from this issue to D6, reviewed it, and it looks fine to me.
Vladimir's test case is currently sitting in this issue, not in a file anywhere. We do not yet have a plan for where to put test cases for individual bug fixes/issues. Once we figure that out, we can put his test case there.
Comment #16
Gábor HojtsyCommitted to Drupal 6, still to be committed to 7!
Comment #17
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
I'm update the component to 'tests' so we can still write a test for this (if necessary).
Comment #18
bjaspan CreditAttribution: bjaspan commentedI discussed this with Gabor yesterday. Vladimir already wrote a test case, posted in this issue. I wasn't sure where to put it in the filesystem. It's a test for a particular bug fix, in this case in the includes/ directory. Can we have includes/includes.test? Or should we create a directory of per-issue test cases like ./tests/200824.test? Or something else? we;re going to end up having a lot of "here's the test for the bug reported in issue #xyz" and we need a consistent strategy for handling them.
Comment #19
sammys CreditAttribution: sammys commentedThanks for getting this committed everyone!
Comment #20
dropcube CreditAttribution: dropcube commentedTest for drupal_write_record() here: #353918: drupal_write_record writes empty string instead of empty serialized array