If I try to store non-UTF-8 data in ctools_object_cache, MySQL cuts off the data at the first occurrence of any non-UTF-8 8-bit character (or byte, in other words). Following this is failure of unserialize() and thus, loss of the cached data.

I propose to change ctools_object_cache.data into type "blob", since IMHO the actual contents of ctools_object_cache.data should be opaque to MySQL anyway.

(Patch follows in next comment for having the issue number in the patch file name.)

CommentFileSizeAuthor
#1 ctools-865142.patch917 bytesraphaelhuefner

Comments

raphaelhuefner’s picture

StatusFileSize
new917 bytes

This patch is against a 2010-07-26T15-21-00Z CVS checkout of ctools 6.x-1.x-dev as detailed in http://drupal.org/node/343333/cvs-instructions/DRUPAL-6--1

merlinofchaos’s picture

Status: Active » Needs review

If we change it to a blob, I believe that our fetch function will need to utilize db_decode_blob() on that field for use in pgsql databases?

bojanz’s picture

Per #690746: Text column type doesn't reliably hold serialized variables, blob columns are now mandated for serialized data in Drupal 7.

This means that CTools for Drupal 7 should definitely switch to blob (leaving the D6 discussion and decision to you)

blasto333’s picture

Switching to blob (which I think already happened) causes problems with views:

http://drupal.org/node/1323744

Peter Bex’s picture

@blasto333: This change has not been made (blob types for postgres were temporarily introduced by git commit 49b3555c0187559b483074385d109d9d620fb428 in 2009, but this was rolled back by 0da7776f6817d4e104488fcd6ae19dfc810faab5 in the same year as far as I can tell).

Maybe it should use type blob in postgres and text in mysql (or possibly the blob column type for mysql could be fixed in core so that the proper type is used in both DBs).

I'm getting errors whenever I try to create views or panels related to Organic Groups; there are some NUL bytes that are inserted when I use them. This means I'm stuck on this site. Going back to MySQL is not an option because of integration with another application that's using Postgres pretty heavily.

PS: Shouldn't the version for this bug be changed to 7? That's what I'm using and there it breaks.

Peter Bex’s picture

roderik’s picture

Well. This is strange. It seems like this was supposed to be fixed in D6 in 2013, but the fix only ended up in D7.

I found this issue at the end of an attempt to dig up & write up what had happened to the state of BLOB fields / PostgreSQL in D6 contrib... and now that I have it, I'll post most of the writeup here. (Though I fully understand that noone's going to change anything anymore.)

Views

views_object_cache.data used to be a BLOB field; it was later changed to LONGTEXT "to increase its size".

I'm not sure what happened here exactly. #295246-17: An error occurred at /drupal/admin/build/views/ajax/preview/ explicitly says "the solution is to change views_object_cache.data to a LONGBLOB type" but then the patch that is uploaded 15 minutes later changes it to TEXT.

If views_object_cache.data never contains any non-UTF-8 data that can cause the same issue as this issue... then the change to TEXT is IMHO an improvement.

If it can contain non-UTF-8 data, this was not only a mistake, but also was inadvertently the start of a long series of incomplete changes to CTools. (For completeness: also, the change to TEXT was incomplete because the 'get' side removed the db_decode_blob() part, but the 'set' side still had the %b in. Commit #cf3aace fixed that a few weeks later.)

In D7, views_object_cache is removed and its data is moved into ctools_object_cache.

CTools

ctools_object_cache.data was also changed from BLOB to TEXT at some point, but

  • not completely - it still contains the same %b that Views initially also had, causing bugs on PostgreSQL.
  • as this issue shows, that was a mistake (and turned back, but not in D6).

What happened:

  • 6.x-1.0-alpha1 had it as a BLOB field.
  • 6.x-1.0-alpha2 (#23755e3) mentioned it was making the same change as was done to views_object_cache.data: changing to 'big text'. It did change the size to big and removed the db_decode_blob() in ctools_object_cache_get(), but didn't change the field type or the %b in ctools_object_cache_set().
  • 6.x-1.0-alpha3 (#f427f87) changed the field type to TEXT.
  • A PostgreSQL bug was reported in #495240: ctools_object_cache_get can't unserialize text field in postgres and 'unfixed' ctools_object_cache_get() (put back the db_decode_blob() for the text field). This was added to 6.x-1.0 (#49b3555).
  • The above was the wrong fix for a TEXT field; ctools_object_cache_set() should have been fixed instead, to get rid of the error. A correct fix (which undoes the previous) is posted in #495240-15: ctools_object_cache_get can't unserialize text field in postgres.
  • Besides this issue, 'duplicate' #1377634: Can't add new display in views was opened for D7 and a fix was committed February 2013: https://git.drupalcode.org/project/ctools/-/commit/4a961f2146d428f5c20ed.... But:
    • This was only committed to the 7.x-1.x branch; D6 never got it.
    • ...despite the fix including a ctools_update_6008(). Unless I'm missing something, this would not actually have updated earlier D7 installs to BLOB because the update hook would never have been executed.

Given that the ctools_object_cache_get() and ctools_object_cache_set() are now written as if they're handling a BLOB field, the patch in this issue is correct - except

  • It needs to be a LONGBLOB field instead of a BLOB field
  • I guess ctools_schema_2() needs to be modified in the same way as the D7 commit does it
  • and the table probably needs to be truncated (/ data deleted) in the update hook before changing its definition

Also: #495240-15: ctools_object_cache_get can't unserialize text field in postgres contains changes to export.inc that would fix two places in export.inc, that could present bugs (on PostgreSQL) when reading BLOB fields. That also seems pertinent to fix at the same time.

but I guess noone's interested anymore.