Escape characters in log names through quick forms

[Skipper-is]

I'm using the milk quick form to create a log.
If I've got an asset with the name "Marge's kid" , the apostrophe comes out as the escape character, rather than ' in the log title.
This was also an issue in my quick weight form, which was based on the milk log, but when I changed this to the farm_livestock_weight_set($assets,$qty,$units, $timestamp); function, it fixed the issue.

So the quick milk form is creating a new log with the log name as:
$log_name = t('Milk @asset: @qty @units', array('@asset' => entity_label('farm_asset', $asset), '@qty' => $form_state['values']['quantity'], '@units' => $form_state['values']['units']));

If the livestock_weight_set function is used (in my quick weight form), it does not use entity_label for the asset name, it goes through farm_log_entity_label_summary, but as far as I can see, all this does is pull the label from exactly the same function:
$label = entity_label($entity_type, $entity);

[mstenta]

Thanks for reporting this @Skipper-is !

I finally had some time to dig into this, and I committed a fix: 27f657d

Explanation:

$log_name = t('Milk @asset: @qty @units', array('@asset' => entity_label('farm_asset', $asset), '@qty' => $form_state['values']['quantity'], '@units' => $form_state['values']['units']));

This line is actually doing more than it appears. It is not simply replacing @asset with entity_label('farm_asset', $asset). The t() function delegates to the format_string() function, which replaces the variable placeholders, but is also used to perform sanitation and character escaping. The @ symbol means "run this text through the check_plain() function to escape characters.

https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/format_string/7.x

In general, this is good. Whenever you are outputting user-generated text (in this case the asset names) to the page, you MUST run it through something like check_plain() first, to prevent cross-site scripting (XSS).

https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/check_plain/7.x

In this particular case, however, we DON'T need to do that. Because we are saving the string to the database (in the log name). We are NOT displaying it on the page (that is done later, and is already being sanitized in those cases). Generally speaking, user input should not be sanitized on save... only on display. (Also note: this is not the same as database injection sanitization, which is also already taken care of by Drupal's database query API.)

So... I fixed this by changing the @ symbols to ! symbols, which tells format_string() "don't sanitize these".

(I also refactored the usage of the t() translation function at the same time, because I don't think it needs to include those other parts... just "Milk".)

So you can use the same approach in your custom quick form to avoid this issue as well.

[mstenta]

There are probably other places where we've used check_plain() where we don't need to... (I guess it's better to habitually over-use check_plain() than not) so if you find other escaped characters in other contexts, open a new issue and maybe refer back to this one. We can fix them on a case-by-case.

[mstenta]

@Skipper-is also note: I made a few other related changes to the milk quick form that you will want to replicate in your quick form too...

• 0944d92
• ba90a84

[Skipper-is]

Oh perfect. The use of the array in the second won't work for mine, as I'm using farm_livestock_weight_set rather than just submitting a quantity