Re^3: Annoying whitespace(s)...

The folks at http://nms-cgi.sourceforge.net/ have written many replacement scripts for the scripts written by Matt Wright, the author of Matts Script Archive), because formmail.pl is the mother of all spammer-abused mailscripts.

The scripts offered there are self contained, single-function scripts that try to do one function and that function well, at least that's the impression I got from them.

Here are some things that I found horrible about the code:

You don't give Matt Wright credit even though you admit you copied the code from him before you modified it. This is bad form.

In the "comments" section:

## the email address from the webpage form code. If in doubt,       ##
+###################
## hardcode the email address into the script by excluding the      ##
+###################
## 'listemail' parameter from the webpage form script code, and     ##
+###################
[download]

That's completely bogus. There is no doubt. A spammer will automatically scan websites for any pages with an embedded hidden field containing an email address, and then submit that form with an email address of their own to check it. Any HTML page that uses a non-hardcoded email address will be used by spammers. Period. Defensive behaviour dicates that this option must always be disabled, as people will not read that far or understand what malice is possible through this.

The hardcoded email address:

$LIST_EMAIL = "reaction";
$LIST_EMAIL .= "\@";
$LIST_EMAIL .= "optionalreaction.com";
[download]

is build in a very confusing way. Not splitting the address in three parts and using single quotes makes usage of the address much much better:

$LIST_EMAIL = q(reaction@optionalreaction.com);
[download]

The single quotes prevent that the @ is interpreted as an array, as single quotes do not interpolate variables. See the use strict; and the use warnings; remark further down.

The templates for the webpages are just horrible:

...
$WEBPAGE_COMPONENT[7] = "<br><br>[Expected email frequency is: MONTHLY
+ VARIABLE]";
$WEBPAGE_COMPONENT[8] = "<br><br>[No further action is required]";
$WEBPAGE_COMPONENT[9] = "<br><br>[Your email address will not be sent,
+ given, or sold, to any third party]";
...
[download]

Use "here documents" or external files, but don't paste them together from arrays that get assigned line by line. Or use one of the many templating systems to make your pages fully customizable.

Also, the parametrization of that part is very bad, as the two lines

$WEBPAGE_COMPONENT[1] = "'</b><br><br>has been <b>ADDED</b> to<br><br>
+<b>'";
$WEBPAGE_COMPONENT[2] = "'</b><br><br>has been <b>REMOVED</b> from<br>
+<br><b>'";
[download]

show. They differ only in the action that was taken and should thus be collapsed to a single line which is then filled with the action taken:

my %messages = (
  Add    => '<b>ADDED</b> to',
  Remove => '<b>REMOVED</b> from',
);
$ACTION_MESSAGE = "'</b><br><br>has been $messages{$action}<br><br><b>
+'";
[download]

Also, what's with the weird quoting? Double quotes ("") or single quotes (''), but not both intermixed.

The writing and rewriting of the file by accumulating stuff into $Buffer is just horrible. Write into the file line by line. And if you insist on using $Buffer for writing to the file, then use it everywhere instead of writing the buffer first, and then a single line with the newly added email after it.

The script is not running under taint mode, is not using strict and is not using warnings. All of these help Perl to protect you from stupid things like typos, misconceptions and input data not being what you expect. You should use them, especially when you are starting out with Perl. They are powerfull tools to protect you.

If you can't track such an error as a spurious space in front of your output in your script, then you obviously haven't programmed much. And obviously, you don't know much about Perl, because Perl does not create "empty" buffers by writing a backslash and a zero into it. If you had double-quoted that string, it would have contained a binary zero character, but that would put a binary zero at the start of your file - highly unlikely that you want that.

Update: Added paragraph about use strict; and use warnings;

Comment on Re^3: Annoying whitespace(s)... Select or Download Code

Replies are listed 'Best First'.
Re^4: Annoying whitespace(s)... by reaction (Acolyte) on Sep 05, 2004 at 22:28 UTC
Thanks Corion, I stand corrected. It is nice to see the differences in coding and grammer that are used around the globe. My mistake... `$Buffer = '/0';` [download] ... unless it is impossible to write a NULL character to a character string. Thanks for the nod for `use strict` and `use warnings`. I am new to Perl, and I may take this opportunity to say, Perl is wierd but necessary. I will investigate 'taint' mode. Like I said the 'read into buffer and then write out' method was inherited from FormMail, I thought it was dodgy at the time, but then thought that's the way perl scripts work. I realise that a large mailing list would ruin a cache in microseconds. The reason I didn't include his name was that there is <20% of his code remaining. The rest of your comments I don't really agree with, prbably primarily because I want to keep this script to a single file. Thanks again, R	[reply] [d/l] [select]
•Re^5: Annoying whitespace(s)... by merlyn (Sage) on Sep 05, 2004 at 22:34 UTC
prbably primarily because I want to keep this script to a single file. Uh, why? Your computer doesn't care how many files the program is. Unless (gasp) you are writing this to share. Ugh. Please don't. We already have enough mediocre Perl code written by budding newbies. And you still have a lot to learn... your updated code is still wrong: `$Buffer = '/0';` [download] So you aren't even paying attention to what you're being told. This makes you dangerous. Please stop now. Go get a good book or two on Perl, read up, then go back to coding, and don't share your code to unsuspecting individuals until you've had a thorough audit by an expert. -- Randal L. Schwartz, Perl hacker Be sure to read my standard disclaimer if this is a reply.	[reply] [d/l]
A reply falls below the community's threshold of quality. You may see it by logging in.
Re^5: Annoying whitespace(s)... by Errto (Vicar) on Sep 05, 2004 at 22:38 UTC
To further clarify, the reason why `$Buffer = '/0';` is still wrong: First of all, you need to understand the difference between single quotes and double quotes in Perl. If you wanted to populate the buffer with a single null character, you would say `$Buffer = "\0";`. Note the double quotes and the backslash instead of forward slash. Second, you don't want to populate a buffer with a null character. In Perl this is not necessary. Perl takes care of allocating memory for strings for you. This was mentioned by Corion but you seem to have missed that part of the reply.	[reply] [d/l] [select]


Don't ask to ask, just ask
	PerlMonks