I've just issued a bug report and patch for a problem with the Search Query class. It's not a huge bug but it prevented me from doing something I needed to do: add some restrictions to a Search Query.
There are two ways of modifying queries in Drupal 7, you can either use a generic "hook_query_alter()" or use "hook_query_TAG_alter()". Tags are a way of telling other modules what a query is about. For example you can have a tag "node_access" which allows modules to add additional restrictions on the query, in this case the Node module connects the query to node_grants.
The Search Query adds a "search_node" or "search_user" tag, unfortunately it adds the tag after the hooks have been called - which is as useful as a chocolate teapot. So I added the patch here: http://drupal.org/node/1146564 which simply moves the place where the tags are added.
Another thing to watch out for: the generic hook_query_alter() is not called on queries that have no tags. This is an efficiency thing as it was discovered early on that this hook was called a lot for no good reason during bootstrap.
Wednesday, 4 May 2011
Tuesday, 3 May 2011
Field encryption
In the next couple of weeks, along with my field_extract module (which allows developers to extract values from fields more easily), I'll also be uploading the initial version of the field_encrypt module (it works, but big sites would be problematic).
An "encrypted field" module already exists for Drupal 7 which allows you to add a special text field, that is encrypted, to an entity, but that's not what I needed. What I needed was a way of encrypting any field even if it already exists (install the module and choose which fields you'd like to encrypt).
There are a three handy hooks which make this possible: 'hook_field_storage_pre_insert', 'hook_field_storage_pre_update', and 'hook_field_storage_pre_load'; they allow other modules to intercept those actions and save or load a field - replacing the field module actions. So, knowing what fields I want to encrypt, I can intercept all load and save actions with those fields and replace them with my own save/load.
This module is only a first attempt and stores all encrypted data in one table. It would be more efficient to create separate tables for separate fields the way Drupal does. But that's for the future. It also doesn't handle deletion but I don't see that as a major issue for small sites. (There are some other areas where it would, in this initial version, have problems with big sites.)
Having said all that there is still a huge hole in security: In Drupal 7 loaded fields are stored in the "cache_field" table - completely unencrypted. I'll have to solve that too for the current client.
Sorry it's taking me so long after promising these modules but I've been rather busy...
EDIT: As I hate leaving too many loose ends the field_encryption module now checks for the presence of the mcrypt library before letting itself be installed; prevents itself being uninstalled if it is currently encrypting a field (to prevent data loss); and now overrides the standard "cache_field" functionality and encrypts cached field data as well. So it's about as secure as it can get.
An "encrypted field" module already exists for Drupal 7 which allows you to add a special text field, that is encrypted, to an entity, but that's not what I needed. What I needed was a way of encrypting any field even if it already exists (install the module and choose which fields you'd like to encrypt).
There are a three handy hooks which make this possible: 'hook_field_storage_pre_insert', 'hook_field_storage_pre_update', and 'hook_field_storage_pre_load'; they allow other modules to intercept those actions and save or load a field - replacing the field module actions. So, knowing what fields I want to encrypt, I can intercept all load and save actions with those fields and replace them with my own save/load.
This module is only a first attempt and stores all encrypted data in one table. It would be more efficient to create separate tables for separate fields the way Drupal does. But that's for the future. It also doesn't handle deletion but I don't see that as a major issue for small sites. (There are some other areas where it would, in this initial version, have problems with big sites.)
Having said all that there is still a huge hole in security: In Drupal 7 loaded fields are stored in the "cache_field" table - completely unencrypted. I'll have to solve that too for the current client.
Sorry it's taking me so long after promising these modules but I've been rather busy...
EDIT: As I hate leaving too many loose ends the field_encryption module now checks for the presence of the mcrypt library before letting itself be installed; prevents itself being uninstalled if it is currently encrypting a field (to prevent data loss); and now overrides the standard "cache_field" functionality and encrypts cached field data as well. So it's about as secure as it can get.
Thursday, 28 April 2011
Chrome image bug and file headers
There is a bug in Chrome which means that, under some circumstances, an image can be displayed on the page for a moment and then disappear.
http://www.google.com/support/forum/p/Chrome/thread?tid=1d825178248ab136&hl=en
The fix is to remove the Content-Length header. Unfortunately that is virtually impossible to do in Drupal without hacking core.
Going into this problem revealed an interesting omission: the file_download() function collects headers from interested modules, but if two modules offer the same header the earlier one gets overwritten. Heavier modules can overwrite the headers of lighter modules.
So making my module heavy would mean that I could overwrite Content-Length with NULL.
But this is not a good solution. What's missing is that, after collecting the headers, file_download() does not use drupal_alter() to allow modules to change the headers.
So the correct solution is a patch which inserts drupal_alter('file_download_headers', $headers, $uri) into the file_download() function at the correct location. And you can find the patch here:
http://drupal.org/node/1137534
All I had to do then was create a function that implemented hook_file_download_headers() which took a look at the $uri and, if it's a graphic, remove the Content-Length header, which is perfectly safe because it's not a critical header.
http://www.google.com/support/forum/p/Chrome/thread?tid=1d825178248ab136&hl=en
The fix is to remove the Content-Length header. Unfortunately that is virtually impossible to do in Drupal without hacking core.
Going into this problem revealed an interesting omission: the file_download() function collects headers from interested modules, but if two modules offer the same header the earlier one gets overwritten. Heavier modules can overwrite the headers of lighter modules.
So making my module heavy would mean that I could overwrite Content-Length with NULL.
But this is not a good solution. What's missing is that, after collecting the headers, file_download() does not use drupal_alter() to allow modules to change the headers.
So the correct solution is a patch which inserts drupal_alter('file_download_headers', $headers, $uri) into the file_download() function at the correct location. And you can find the patch here:
http://drupal.org/node/1137534
All I had to do then was create a function that implemented hook_file_download_headers() which took a look at the $uri and, if it's a graphic, remove the Content-Length header, which is perfectly safe because it's not a critical header.
Tuesday, 19 April 2011
Conditions for using where
Just to help you avoid spending as many hours as I have trying to force a nasty little query with a subquery to do what I wanted...
If you want to have a WHERE clause that contains something like "fss.entity_id = n.nid", you can't use the condition function. In other words, you can't do this:
$query->condition('fss.entity_id', 'n.nid');
You have to use this:
$query->where('fss.entity_id = n.nid');
Because the first version gets translated into fss.entity_id = 'n.nid' in the SQL, which will never be true.
In debugging this it slowly became obvious that something fundamental was wrong - because when I output the $query string and then replaced the placeholders with the right arguments, and shoved it straight into the MySQL server - it worked.
Eventually it dawned on me to use the Devel query output to see what was actually being called and found the error - though by that time I was looking for it, having exhausted every other option.
Here's a quick tip for outputting the entire contents of an object which contains public and private properties:
dpm(print_r($object, TRUE));
I could have done without that today.
If you want to have a WHERE clause that contains something like "fss.entity_id = n.nid", you can't use the condition function. In other words, you can't do this:
$query->condition('fss.entity_id', 'n.nid');
You have to use this:
$query->where('fss.entity_id = n.nid');
Because the first version gets translated into fss.entity_id = 'n.nid' in the SQL, which will never be true.
In debugging this it slowly became obvious that something fundamental was wrong - because when I output the $query string and then replaced the placeholders with the right arguments, and shoved it straight into the MySQL server - it worked.
Eventually it dawned on me to use the Devel query output to see what was actually being called and found the error - though by that time I was looking for it, having exhausted every other option.
Here's a quick tip for outputting the entire contents of an object which contains public and private properties:
dpm(print_r($object, TRUE));
I could have done without that today.
Tuesday, 12 April 2011
CSS and Overlays
This might seem obvious but if you need to modify the CSS for your admin pages which appear in overlays, you will be needing to create a subtheme.
Assuming you use Seven for your admin theme, create a subtheme based on Seven, let's say you call it SevenUp, and add your own CSS overrides, then change the default admin theme to SevenUp.
You'll get all the Seven settings and your overrides.
However when you enable your theme you'll have to go to the Block admin page and remove all the blocks which have now added themselves.
Assuming you use Seven for your admin theme, create a subtheme based on Seven, let's say you call it SevenUp, and add your own CSS overrides, then change the default admin theme to SevenUp.
You'll get all the Seven settings and your overrides.
However when you enable your theme you'll have to go to the Block admin page and remove all the blocks which have now added themselves.
Friday, 1 April 2011
drupal_get_destination and overlays
There may be a better way of doing this, but...
Had the situation where I had an admin page (set up to appear in an overlay) on which there were other links to forms. The problem was that when I clicked "Submit" on one of these forms it went back to the original page and not the previous overlay page.
If there is a destination in drupal_get_destination() this takes precedence over any form setting. So I needed to somehow force this to be what I wanted. The solution is relatively easy though might be considered a bit hacky.
What you need to do is this:
$_GET['destination'] = $_GET['q'];
which forces the current page to be the one you go back to. But this won;t always work because if drupal_get_destination() has already been called the destination will now be held in drupal_static(). So what we actually need is this:
drupal_static('drupal_get_destination', NULL, TRUE);
$_GET['destination'] = $_GET['q'];
This wipes out any older version and ensures you get what you want.
Had the situation where I had an admin page (set up to appear in an overlay) on which there were other links to forms. The problem was that when I clicked "Submit" on one of these forms it went back to the original page and not the previous overlay page.
If there is a destination in drupal_get_destination() this takes precedence over any form setting. So I needed to somehow force this to be what I wanted. The solution is relatively easy though might be considered a bit hacky.
What you need to do is this:
$_GET['destination'] = $_GET['q'];
which forces the current page to be the one you go back to. But this won;t always work because if drupal_get_destination() has already been called the destination will now be held in drupal_static(). So what we actually need is this:
drupal_static('drupal_get_destination', NULL, TRUE);
$_GET['destination'] = $_GET['q'];
This wipes out any older version and ensures you get what you want.
Thursday, 31 March 2011
Being too clever with forms
The situation I ran into was with a content type called "document", it's pretty simple: title, description, taxonomy term and a single file upload.
The tricky bit was the taxonomy: each document could belong to a different part of the site (represented by the top level of the taxonomy) let's say SectionA and SectionB, within each of those there'd be subsections: SubA1, SubA2... and SubB1, SubB2 etc.
The first level was selected by the place the document was created - so someone might be in Section A, they create a document - I add the taxonomy term to the URL, like this: node/add/document/X and, in a hook_form_alter(), I use that added value to modify the allowed options in the taxonomy selector to only include the subsections for the section we came from.
Which is fine.
Except when you factor in the file upload. What a file upload does is rebuild the form - which meant my code was getting called again and this was bad. The real code was a bit more complex than I've described here and reloading was very bad - basically it completely wiped out the taxonomy options on the second run through. Which then meant I got validation errors.
The solution is that you have to have some way of noting that you've been here before and not do the changes again. One thing know is that $form_state does not get rewritten so you can store a variable in there marking that you've done the changes already, it should look something like this (using hook_form_FORMID_alter()):
function mymodule_form_myform_alter(&$form, &$form_state) {
if (isset($form_state['mymodule_myform_processed'])) {
return;
}
$form_state['mymodule_myform_processed'] = TRUE;
...the rest of the code...
}
And that does the trick.
The tricky bit was the taxonomy: each document could belong to a different part of the site (represented by the top level of the taxonomy) let's say SectionA and SectionB, within each of those there'd be subsections: SubA1, SubA2... and SubB1, SubB2 etc.
The first level was selected by the place the document was created - so someone might be in Section A, they create a document - I add the taxonomy term to the URL, like this: node/add/document/X and, in a hook_form_alter(), I use that added value to modify the allowed options in the taxonomy selector to only include the subsections for the section we came from.
Which is fine.
Except when you factor in the file upload. What a file upload does is rebuild the form - which meant my code was getting called again and this was bad. The real code was a bit more complex than I've described here and reloading was very bad - basically it completely wiped out the taxonomy options on the second run through. Which then meant I got validation errors.
The solution is that you have to have some way of noting that you've been here before and not do the changes again. One thing know is that $form_state does not get rewritten so you can store a variable in there marking that you've done the changes already, it should look something like this (using hook_form_FORMID_alter()):
function mymodule_form_myform_alter(&$form, &$form_state) {
if (isset($form_state['mymodule_myform_processed'])) {
return;
}
$form_state['mymodule_myform_processed'] = TRUE;
...the rest of the code...
}
And that does the trick.
Subscribe to:
Posts (Atom)