http://qs321.pair.com?node_id=388658

reaction has asked for the wisdom of the Perl Monks concerning the following question:

Hello,

I have written (modified) a simple mailing list script. It works OK, but whenever a mail address is appended to the datafile, a <SPACE> is added to the beginning of the datafile aswell.

Can anyone tell me what is wrong? The script can be downloaded from http://optionalreaction.net/cgiperl; it is called 'goodfoot.pl'.

Thanks for any help, R

Replies are listed 'Best First'.
Re: Annoying whitespace(s)...
by Corion (Patriarch) on Sep 05, 2004 at 21:23 UTC

    This is because in the script, the variable holding the new datafile is initialized to a space first:

    # Buffer for temporary swap store. $Buffer = ' ';

    Initialize the buffer to be empty like the following to solve your immediate problem:

    # Buffer for temporary swap store. $Buffer = '';

    A word of advice:That script is horrible code, and while I didn't see a wide gaping spammer hole in it, the sister script http://optionalreaction.net/cgiperl/nomail.pl is a really wide open invitation for spammers to abuse it. As it seems, you don't have much experience programming and debugging other peoples programs. I can only recommend you to consult somebody who understands these things instead of copying and pasting scripts from the web in the hope that they work as you want and can't be abused to the detriment of others. The folks at http://nms-cgi.sourceforge.net/ have well tested and supported CGI scripts that try to cater to many needs. Give those scripts a look instead.

      Thanks for the advice,

      I actually trimmed the script down (and made it much easier to read, install and use) from FormMail (MAtt's Script Archive), aparently the most popular mailing list script on the web.

      I do not want a hugely complicated script from sourceforge that will give me far functions more than I need.

      Would you mind explaining what is 'horrible' about the code (which I doubt), and why you assume I know nothing about programming?

      BTW, shouldn't it be...

      $Buffer = '\0';

      Regards, R

        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 ## +###################
        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";
        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);
        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]"; ...
        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>'";
        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> +'";
        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;

        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.

        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 = ();

        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"; }

        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

        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
        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.

Re: Annoying whitespace(s)...
by ikegami (Patriarch) on Sep 05, 2004 at 21:16 UTC

    Could it be because you're using
    $Buffer = ' ';
    and not
    $Buffer = '';?

Re: Annoying whitespace(s)...
by Rhys (Pilgrim) on Sep 05, 2004 at 21:20 UTC
    It's because it doesn't append to the file. It reads the entire file, then writes the entire file and writes the new addy after re-writing the previous contents.

    The space gets in there because the author uses $Buffer to hold the previous contents. $Buffer is initialized with:

    $Buffer = ' ';

    Pretty silly. Just look for this line and remove the space.

    --J