In the print_mail_form_submit function, the message 'Thank you for spreading the word about !site' is output on a successful e-mail. Could the text of this message be made a configurable variable, to be set on the admin/settings/print/email form? I have several clients for whom this "spreading the word" text doesn't really apply to their sites target audiences.

Comments

Jeroen Coumans’s picture

Status: Active » Fixed

You can actually adjust this message by translating it using the localization module.

jcnventura’s picture

Status: Fixed » Active

While the translation 'hack' it's a possibility, I don't think that most users will want to create a new language just to change the strings in this module..

I am planning to add some configuration fields in the settings form to make this user-configurable..

João

jcnventura’s picture

Status: Active » Fixed

Today's dev versions will include the ability to change these strings in the e-mail settings page.

João

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

mikhailian’s picture

Version: 5.x-4.0 » 6.x-1.9

Is it really necessary to run check_plain on the message? I'd rather not.

--- print_mail/print_mail.inc   (revision 1)
+++ print_mail/print_mail.inc   (revision 2)
@@ -332,7 +332,7 @@
         watchdog('print_mail', '%name [%from] sent %page to [%to]', array('%name' => $form_state['values']['fld_from_name'], '%from' => $form_state['values']['fld_from_addr'], '%page' => $form_state['values']['path'], '%to' => $form_state['values']['txt_to_addrs']));
         $site_name = variable_get('site_name', t('us'));
         $print_mail_text_confirmation = variable_get('print_mail_text_confirmation', t('Thank you for spreading the word about !site.'));
-        drupal_set_message(check_plain(t($print_mail_text_confirmation, array('!site' => $site_name))));
+        drupal_set_message(t($print_mail_text_confirmation, array('!site' => $site_name)));
 
         $nodepath = drupal_get_normal_path($form_state['values']['path']);
         db_query("UPDATE {print_mail_page_counter} SET sentcount = sentcount + %d, sent_timestamp = %d WHERE path = '%s'", count($addresses), time(), $nodepath);
jcnventura’s picture

Considering that I had to create a Security release to add that check_plain, I'm afraid it really is necessary.

mikhailian’s picture

AFAICS, the security release talks about user-supplied variables, this is different . I may be mistaken, but it looks like you overreacted in that security release.

jcnventura’s picture

No, that variable is exactly one of the 'user-supplied variables'. Feel free to browse the original vulnerability disclosure: http://www.madirish.net/236

You'll see the "The print module also contains a XSS vulnerability due to the fact that the 'site name' is not properly sanitized when displaying e-mail confirmation in the "Thank you for spreading the word about [site_name]" area. The print module also contains a XSS vulnerability due to the fact that it does not properly sanitize the 'Thank You Message:' input."

And seeing as you're posting this in the original request to make this confirmation message be user-configurable, it should be obvious that the fix is the proper one. In the 7.x-2.x branch, I actually dropped the ability to do this, considering that the String Overrides module allows the same functionality with none of the security hassles.