Patch to fix email regression, flood bug, and to enhance email subject and template
benoit.borrel - November 5, 2009 - 13:41
| Project: | Forward |
| Version: | 6.x-1.10 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Jump to:
Description
Hi,
This patch applies the following on forward.module file:
- fix the regression on email subject + message (caused by modification of line 682)
- fix the bug in flood control message (caused by
%numberwhich I replaced with!number) - add the new email subject placeholder
!title(until token support) - add the new template variable
$raw_link - fix minor coding standard issues (revealed by coder module)
One has to resubmit the Forward admin settings form if applying this patch in order to replace obsolete permanent variables forward_page_subject and forward_page_message with forward_email_subject and forward_email_message respectively.
| Attachment | Size |
|---|---|
| forward.module.patch | 11.53 KB |
#1
+++ sites/all/modules/forward/forward_fixed.module 2009-11-05 13:27:06.000000000 +0000@@ -634,7 +634,7 @@ function forward_form_validate($form, &$
if (!flood_is_allowed('forward', variable_get('forward_flood_control', 10) - count($recipient_addresses) + 1)) {
- form_set_error('recipients', t(variable_get('forward_flood_error', 'You can\'t send more than %number messages per hour. Please try again later.'), array('%number' => variable_get('forward_flood_control', 10))));
+ form_set_error('recipients', t(variable_get('forward_flood_error', 'You can\'t send more than !number messages per hour. Please try again later.'), array('!number' => variable_get('forward_flood_control', 10))));
}
t() shouldn't be used on user-supplied text, only on the string literal supplied as the default.
Also, what is the flood message bug you mentioned? Using %number seems like it would work ok there. If you're going to use !number, then the output of variable_get needs to be run through check_plain to prevent an XSS vulnerability.
I'm on crack. Are you, too?
#2
Hi grendzy,
>t() shouldn't be used on user-supplied text, only on the string literal supplied as the default.
I totally agree.
>Also, what is the flood message bug you mentioned?
When I reached the flood limit, the line 637 was triggered but displayed
You can't send more than !number messages per hour.instead ofYou can't send more than 10 messages per hour.. This was because of a mismatch between placeholders used in variable_get() and t(): variable_get('forward_flood_error', ...) uses !number whereas the t(...) uses %number. And because this is a computed number, ie. not a user submitted data, I fixed the issue by using !number in t(...).On another matter, note that t() shouldn't use escaping quotation: ie not
t('...can\'t...')butt("...can't...").