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

Replies are listed 'Best First'.
Re^3: Annoying whitespace(s)... by Corion (Patriarch) on Sep 05, 2004 at 22:00 UTC
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;`	[reply] [d/l] [select]
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]
�Re^3: Annoying whitespace(s)... by merlyn (Sage) on Sep 05, 2004 at 22:20 UTC
why you assume I know nothing about programming? ... `$Buffer = '\0';` This would be a clue that you know very little about Perl. Perhaps you do indeed know something about programming in general, but not Perl. -- Randal L. Schwartz, Perl hacker Be sure to read my standard disclaimer if this is a reply.	[reply] [d/l]
Re^3: Annoying whitespace(s)... by Rhys (Pilgrim) on Sep 05, 2004 at 22:24 UTC
Corion goes into detail about the rest of this, but as far as the $Buffer variable goes, no. Using the NULL character is C-style thinking. In Perl, if you want something to be NULL, don't put anything there. :-) E.g.: `$Buffer = ''; @Buffer = (); %Buffer = ();` [download] NOTE: A variable that has a NULL value is slightly different from an undefined value: `my $notdef; my $defnull; # like the pun? $defull = ''; if ( defined $notdef ) { # This will never happen. } if ( defined $defnull ) { print "DEFNULL is $defnull.\n"; }` [download] This is why it's often wise to declare and initialize your variables at the same time. You can use a variable with a NULL value - as a hash key, for example - but not an undefined one. --J	[reply] [d/l] [select]
Re^3: Annoying whitespace(s)... by ikegami (Patriarch) on Sep 05, 2004 at 22:43 UTC
FormMail (MAtt's Script Archive), aparently the most popular mailing list script on the web. It's definitely the most popular with spammers, because it can be used to send emails to arbitrary addresses.	[reply]
Re^3: Annoying whitespace(s)... by DamnDirtyApe (Curate) on Sep 05, 2004 at 22:19 UTC
Corion's analysis was bang-on (yes, this really is horrible code.) One more suggestion - take this script, and rewrite it using Template Toolkit to generate the content and MIME::Lite to manage the email. You'd probably cut the script to a third the length it is now, and you'd be amazed how much more readable and maintainable it would be. Just my $0.02, _______________ DamnDirtyApe Those who know that they are profound strive for clarity. Those who would like to seem profound to the crowd strive for obscurity. --Friedrich Nietzsche	[reply]
Re^4: Annoying whitespace(s)... by reaction (Acolyte) on Sep 05, 2004 at 22:33 UTC
Thanks, I wanted to try to keep this simple... R	[reply]
Re^5: Annoying whitespace(s)... by Rhys (Pilgrim) on Sep 05, 2004 at 22:51 UTC
I don't know anything myself about templating - another one of those programming tools I'll have to learn at some point - but I took a quick look at MIME::Lite just now, and it will make your script simpler, not more complicated. `mybox# perl -eshell -MCPAN cpan> install MIME::Lite ...buncha installation messages... cpan> exit mybox# exit mybox> perldoc MIME::Lite` [download] Very cool stuff, and pretty easy to use. With Perl, if you can find a module on CPAN that does something for you, it's often best to install and learn that module, rather than code it yourself. Since you appear to have a C background, think of modules as installing a bunch of .h files with some easy-to-use functions. --J	[reply] [d/l]