Home > Archive > PERL CGI Freelance > June 2004 > How to format $mail_from with 2 variables separated by a comma?
You are viewing an archived Text-only version of the thread.
To view this thread in it's original format and/or if you want to reply to
this thread please [click here]
| Author |
How to format $mail_from with 2 variables separated by a comma?
|
|
| Jason Miles 2004-06-03, 7:13 pm |
| I have a script which is processing form input. What I want is when the
script sends an e-mail with the form contents, the From field in the e-mail
should be
last_name,first_name
The code i'm running currently produces
last_name;first_name
Here is the line of code I'm using
$mail_from = $query->param("family_name").",".$query->param("given_name");
I've also tried
$mail_from = $query->param("family_name").','.$query->param("given_name");
Can someone tell me what to change to get the desired result of
last_name,first_name ?
Thanks in advance!
| |
| Vorxion 2004-06-03, 7:13 pm |
| In article <OErtc.4637$9q1.5299@news20.bellglobal.com>, Jason Miles wrote:
>I have a script which is processing form input. What I want is when the
>script sends an e-mail with the form contents, the From field in the e-mail
>should be
>last_name,first_name
>
>The code i'm running currently produces
>last_name;first_name
>
>Here is the line of code I'm using
>$mail_from = $query->param("family_name").",".$query->param("given_name");
Odd. In theory that should be fine.
One thing...switch to single-quotes in the param() arguments. If you don't
need interpolation, why slow down your code by invoking it?
>Can someone tell me what to change to get the desired result of
>last_name,first_name ?
Try:
$mail_from = join(',',$query->param('family_name'),$query->param('given_name'));
If -that- doesn't work correctly, you have an issue somewhere else. I tend
to think you already do, as both of your examples should work just fine.
--
Vorxion - Member of The Vortexa Elite
| |
| John Bokma 2004-06-03, 7:13 pm |
| Vorxion wrote:
> In article <OErtc.4637$9q1.5299@news20.bellglobal.com>, Jason Miles wrote:
>
>
> Odd. In theory that should be fine.
It's not. The values are not checked.
> One thing...switch to single-quotes in the param() arguments. If you don't
> need interpolation, why slow down your code by invoking it?
How much do you think it slows down? The overhead of CGI.pm is
*significant* compared to single or double quotes :-D. (On the other
hand, I agree, but more because I think ' is more readable).
>
> Try:
>
> $mail_from = join(',',$query->param('family_name'),$query->param('given_name'));
Wrong, since it doesn't check anything. Moreover, if use strict and use
warnings is used (as should), it can report a warning since any of those
params can be undefined.
--
John MexIT: http://johnbokma.com/mexit/
personal page: http://johnbokma.com/
Experienced Perl programmer available: http://castleamber.com/
| |
| chanio 2004-06-03, 7:13 pm |
| John Bokma (alt.comp.perlcgi.freelance) dijo...
> Vorxion wrote:
>
>
> It's not. The values are not checked.
>
>
> How much do you think it slows down? The overhead of CGI.pm is
> *significant* compared to single or double quotes :-D. (On the other
> hand, I agree, but more because I think ' is more readable).
>
>
> Wrong, since it doesn't check anything. Moreover, if use strict and use
> warnings is used (as should), it can report a warning since any of those
> params can be undefined.
>
why not simply and clearly...
my $name=$query->param(given_name) or ' ';
....etc? And use strict.
If it doesn't change the result, I would suggest the dirtiest way:
swap the variables' contents ;)) . At least, it works.
And if not, you'll experience a change in the definition of your problem,
don't you think?
Hope it helps!
--
.------------------. -.-.-.-.-.-.-.-.-.-.-.-.-.-.-.-.-.-.-.-.-.-.
| ___ _ _ _ _ | ALBERTO ADRIAN SCHIANO - ARGENTINA - 2004
|/ __/ | \ || | | | <ALETIA@2VIAS.COM.AR> # 34-34S 058-25W(z-3)
|||_< \| || ' | | +------------+------------------------------
|`____/|_\_|`___' | LINUX COUNTER: 240 133 ~ machine : 119 401
| _ _ _ __ _ | +------------+----------+-------------------
|| | | \ |\ \/ | AMD Athlon 6 |RAM 512Mb.|krnl.: 2.6.3-10mdk
|| |_ | | \ \ | i586-mandrake-linux-gnu |MDK 9.2 - KDE 3.13
||___||_\_|_/\_\ | +-----------------------+-------------------
| __ __ ___ _ _ | Maxtor #4D040H2 32Gb. |DISPLAY_VGA SiS 630
|| \ \| . \| / | ------------------------+--+----------------
|| || | || \ | PCI Audio snd-trident 7018 | ViewSonic E771
||_|_|_||___/|_\_ | ---------------------------+----------------
| | http://perlmonks.org/index.pl?node_id=245320
'------------------' -.-.-.-.-.-.-.-.-.-.-.-.-.-.-.-.-.-.-.-.-.-.
| |
| Vorxion 2004-06-03, 7:13 pm |
| In article <40b67b17$0$209$58c7af7e@news.kabelfoon.nl>, John Bokma wrote:
>Vorxion wrote:
>
>
>It's not. The values are not checked.
We don't know that. All he posted was his concatenation line. We have
-zero- context for what he has before this point. I assumed he would have
checked them.
>
>How much do you think it slows down? The overhead of CGI.pm is
>*significant* compared to single or double quotes :-D. (On the other
>hand, I agree, but more because I think ' is more readable).
Not enough to be very noticeable. OTOH, the more times you invoke it,
through more incarnations on a busy system, the more overhead you're
generating. It's cumulative.
>
>Wrong, since it doesn't check anything. Moreover, if use strict and use
>warnings is used (as should), it can report a warning since any of those
>params can be undefined.
My personal method is to forget taint and use my own sanity checking. I
perform all my checks on defined() ahead of time. But there's no way we
can assume he did or didn't check for the definedness of those even in the
line before this, and branch to an error handling segment that we're not
aware of. You're presuming too much about his code without evidence.
Besides, and undefined value on either side would not yield a semicolon in
place of a comma, as his original stated problem indicates. It would be in
addition to, if it was part of the data, but not strictly replacing it
entirely. Something odd was happening there, and not enough code was given
to make a proper analysis, honestly.
--
Vorxion - Member of The Vortexa Elite
| |
| Dave Cross 2004-06-03, 7:13 pm |
| On Thu, 27 May 2004 17:03:27 -0400, Vorxion wrote:
> One thing...switch to single-quotes in the param() arguments. If you don't
> need interpolation, why slow down your code by invoking it?
Actually, it doesn't slow it down at all. Perl is clever enough to
realiase that it's a fixed string and changes the double quotes to single
quotes at compilation:
$ perl -MO=Deparse -le'print "hello"'
BEGIN { $/ = "\n"; $\ = "\n"; }
print 'hello';
-e syntax OK
Of course, there is still another good reason for using the correct type
of quotes - which is that it makes it more obvious what is going on for
your maintenance programmer.
Dave...
| |
| John Bokma 2004-06-03, 7:13 pm |
| Vorxion wrote:
> In article <40b67b17$0$209$58c7af7e@news.kabelfoon.nl>, John Bokma wrote:
>
>
>
> We don't know that. All he posted was his concatenation line. We have
> -zero- context for what he has before this point. I assumed he would have
> checked them.
$query->param("family_name") can be undefined, so the concat gives a
warning. Or do you assume he changes the value as held by CGI.pm? I
doubt that.
>
> Not enough to be very noticeable. OTOH, the more times you invoke it,
> through more incarnations on a busy system, the more overhead you're
> generating. It's cumulative.
It's insignificant compared to CGI.pm. One extra comment line is
probably more overhead.
>
> My personal method is to forget taint and use my own sanity checking.
Has nothing to do with taint.
> I
> perform all my checks on defined() ahead of time. But there's no way we
> can assume he did or didn't check for the definedness of those even in the
> line before this, and branch to an error handling segment that we're not
> aware of. You're presuming too much about his code without evidence.
So, you assume he first checks the definedness of $query->param(...)? I
bet he doesn't check.
> Besides, and undefined value on either side would not yield a semicolon in
> place of a comma, as his original stated problem indicates. It would be in
> addition to, if it was part of the data, but not strictly replacing it
> entirely. Something odd was happening there, and not enough code was given
> to make a proper analysis, honestly.
On the latter I agree. I only recommend checking param stuff. Never
assume that only your form calls the script, see:
http://johnbokma.com/websitedesign/forms.html
--
John MexIT: http://johnbokma.com/mexit/
personal page: http://johnbokma.com/
Experienced Perl programmer available: http://castleamber.com/
| |
| John Bokma 2004-06-03, 7:13 pm |
| Dave Cross wrote:
> On Thu, 27 May 2004 17:03:27 -0400, Vorxion wrote:
>
>
> Actually, it doesn't slow it down at all. Perl is clever enough to
> realiase that it's a fixed string and changes the double quotes to single
> quotes at compilation:
Which takes some time, however *insignificant*. CGI.pm overhead makes
this overhead even more insignificant. Probably a comment line has more
overhead in time. Optimise for readability. If the script is slow,
improve the algorithm. Those weird optimisations are rarely needed if at
all.
--
John MexIT: http://johnbokma.com/mexit/
personal page: http://johnbokma.com/
Experienced Perl programmer available: http://castleamber.com/
| |
| Vorxion 2004-06-03, 7:13 pm |
| In article <40b6db8f$0$210$58c7af7e@news.kabelfoon.nl>, John Bokma wrote:
>Vorxion wrote:
>
>
>$query->param("family_name") can be undefined, so the concat gives a
>warning. Or do you assume he changes the value as held by CGI.pm? I
>doubt that.
Okay, I thought this was obvious from what I said but since it's not,
imagine he had something like this:
if (defined($query->param('family_name')) and defined($query->param('given_name')) {
# His concatenation line.
}
IOW, you may have just slammed him for something he wasn't even doing
wrong, when he posted one line with an entire lack of context. Immediately
you assume a lack of skill, lack of dilligence, or some other bullshit.
Frankly, I'd expect this kind of crap in c.l.p.m from the resident purists
who must have things done Their One True Way[tm], but not here.
>
>It's insignificant compared to CGI.pm. One extra comment line is
>probably more overhead.
Exactly -what- are you talking about? Interpolation on an expression will
happen whenever it's invoked. Comments are stripped at precompile to the
best of my knowledge and aren't even no-ops, they just plain don't exist in
the pre-compiled version that's executed.
>So, you assume he first checks the definedness of $query->param(...)? I
>bet he doesn't check.
I give him the benefit of the doubt unless he proves otherwise. That would
be the difference between me and the holier-than-thou schmucks you'll see
populating c.l.p.m, who can't be bothered to even read code that wasn't
written character-for-character the way THEY would do it.
>
>On the latter I agree. I only recommend checking param stuff. Never
>assume that only your form calls the script, see:
>http://johnbokma.com/websitedesign/forms.html
And I wholeheartedly concur. Check:
http://duran.fairlite.com/cgi-security.html
That would be -my- security checklist. You're preaching to the choir on
security issues, but you haven't given this person the benefit of the
doubt, based on one line of posted code, taken out of context. I don't
agree with that stance at all. It's your right, of course, but I don't
agree that he's guilty of poor design unless there's context that proves
it. Would I code it the way he's doing it? No. But if he did the checks
ahead of time and insited on reiterating the method to obtain the values
again, so be it--it would still be technically correct. The point is, we
don't -know-, and you've already hung him for it.
I'm sick and tired of "experts" telling people that they're less
than acceptable based on flimsy conjecture and differences based on
personal preference. Us bitching about people not even offering to pay
for assistance in this group is one thing. But we're supposed to be
professionals. How about we leave the religious preaching to the 'priests'
over in c.l.p.m, shall we? I see no place for it here. I'm not sure which
are worse--the zealots in the perl community or in the linux community.
And I've spent enough time in both camps to know just how bad it can get.
Neither group's people does their respective environment any favours by
association.
--
Vorxion - Member of The Vortexa Elite
| |
| John Bokma 2004-06-03, 7:13 pm |
| Vorxion wrote:
> In article <40b6db8f$0$210$58c7af7e@news.kabelfoon.nl>, John Bokma wrote:
[..]
> Okay, I thought this was obvious from what I said but since it's not,
> imagine he had something like this:
>
> if (defined($query->param('family_name')) and defined($query->param('given_name')) {
> # His concatenation line.
> }
>
> IOW, you may have just slammed him for something he wasn't even doing
> wrong, when he posted one line with an entire lack of context. Immediately
> you assume a lack of skill, lack of dilligence, or some other bullshit.
>
> Frankly, I'd expect this kind of crap in c.l.p.m from the resident purists
> who must have things done Their One True Way[tm], but not here.
I still bet he doesn't check. Most CGI scripts are badly written,
especially by people who ask basic questions like the OP did.
>
> Exactly -what- are you talking about? Interpolation on an expression will
> happen whenever it's invoked.
Crap. As someone else already made clear, the Perl interpreter is way
smarter than you think, "bla" is changed internally as 'bla'. Do you
really think that "bla" is every time checked for magic? Don't you know
anything about optimization, or do you assume $a = 5 + 7; is also
evaluated every time in a loop? Perl is not JavaScript :-D.
> Comments are stripped at precompile to the
> best of my knowledge and aren't even no-ops, they just plain don't exist in
> the pre-compiled version that's executed.
Ofcourse, but they must be *ignored by the parser*. And yes, that takes
time, but again, *insignificant*, as is the optimalisation of "foobar".
>
> I give him the benefit of the doubt unless he proves otherwise. That would
> be the difference between me and the holier-than-thou schmucks you'll see
> populating c.l.p.m, who can't be bothered to even read code that wasn't
> written character-for-character the way THEY would do it.
Again you are wrong. But yes, the OP would be serious flamed, because
the posting doesn't show the *ACTUAL* code, nor the way how he obtained
the *wrong* result.
> That would be -my- security checklist. You're preaching to the choir on
> security issues, but you haven't given this person the benefit of the
> doubt, based on one line of posted code, taken out of context. I don't
> agree with that stance at all. It's your right, of course, but I don't
> agree that he's guilty of poor design unless there's context that proves
> it. Would I code it the way he's doing it? No. But if he did the checks
> ahead of time and insited on reiterating the method to obtain the values
> again, so be it--it would still be technically correct. The point is, we
> don't -know-, and you've already hung him for it.
Still, I am 100% sure the OP doesn't check. That he thought that a
change from "," to ',' would make any difference clearly shows that the
OP lacks a lot of basic Perl skills. I am also convinced that the OP
just uses the obtained $mail_from without checking anything (like a
sensible size for family and given name, and/or characters).
> I'm sick and tired of "experts" telling people that they're less
> than acceptable based on flimsy conjecture and differences based on
> personal preference.
This is not a personal preference but required in a CGI script. I am
sick and tired of all those crappy CGI scripts being downloaded and
used, and turning into tools used by spammers.
Too often I see postings, I don't understand Perl, but I found this
script somewhere (no URL given, and the script is clearly crap), who can
help me (for free) to turn it into what I want.
Usenet is not a helpdesk.
--
John MexIT: http://johnbokma.com/mexit/
personal page: http://johnbokma.com/
Experienced Perl programmer available: http://castleamber.com/
| |
| Jason Miles 2004-06-03, 7:13 pm |
| I seem to have started a pissing contest. So, here is the whole code, and I
appologize for not posting it all in the first place. Comments welcome.
Thanks for the help and I take note of all your comments...
#!/usr/bin/perl
use strict;
use CGI qw(:standard);
use Net::SMTP;
my($query,$mailhost,$mail_from,$subject,
$confirmation_email_to,$confirmation
_email_from,$confirmation_subject,$thank
you_url,$smtp,$f,$r,$confirmation_bo
dy);
my @mail_to;
my @field_order;
my %fields;
$query = new CGI;
$mailhost = "smtp.domain.com";
$mail_from = $query->param("family_name").",".$query->param("given_name");
@mail_to = ('lawyers@domain.com');
$subject = "Assessment Submission";
$confirmation_email_to = $query->param("email_address");
$confirmation_email_from = 'lawyers@domain.com';
$confirmation_subject = ("Free Assessment Confirmation");
$confirmation_body = "Thank you for completing the Preliminary Assessment
Questionnaire.\n
One of our staff lawyers will review your assessment
and will reply to you within 72 hours.\n
Please be assured that the information you provide
will be kept in the strictest of confidence
and will not be released to any third party for any
reason without your express written consent.";
@field_order =
("heard_by","heard_by_country","referred_by","referred_to","salutation","giv
en_name","family_name","email_address","nationality","mailing_address","stat
us","home_phone","work_phone","fax","dob_day","dob_month","dob_year","marita
l_status","childrens_ages","speaks_english","reads_english","writes_english"
,"speaks_french","reads_french","writes_french","years_of_education","educat
ion_certificate","field_of_study","number_of_years","education_country","add
itional_certificates","current_occupation","job_1_from_year","job_1_to_year"
,"job_1_company","job_1_duties","job_2_from_year","job_2_to_year","job_2_com
pany","job_2_duties","job_3_from_year","job_3_to_year","job_3_company","job_
3_duties","comments_on_employment","spouse_salutation","spouse_given_name","
spouse_family_name","spouse_nationality","spouse_dob_day","spouse_dob_month"
,"spouse_dob_year","spouse_speaks_english","spouse_reads_english","spouse_wr
ites_english","spouse_speaks_french","spouse_reads_french","spouse_writes_fr
ench","spouse_years_of_education","spouse_education_certificate","spouse_fie
ld_of_study","spouse_number_of_years","spouse_country","spouse_additional_ce
rtificates","spouse_current_occupation","spouse_job_1_from_year","spouse_job
_1_to_year","spouse_job_1_company","spouse_job_1_duties","spouse_job_2_from_
year","spouse_job_2_to_year","spouse_job_2_company","spouse_job_2_duties","s
pouse_job_3_from_year","spouse_job_3_to_year","spouse_job_3_company","spouse
_job_3_duties","spouse_comments_on_employment","relative_relationship","net_
worth","employment_offer","employment_offer_explanation","serious_disease","
convicted_or_charged","previously_applied_for_visa","previously_visited_cana
da","able_to_obtain_visa","previous_business_nature","previous_duties","perc
entage_of_ownership","number_of_employees","3_year_history_year_1","3_year_h
istory_year_1_turnover","3_year_history_year_1_net_profit","3_year_history_y
ear_2","3_year_history_year_2_turnover","3_year_history_year_2_net_profit","
3_year_history_year_3","3_year_history_year_3_turnover","3_year_history_year
_3_net_profit","business_net_worth_list","asset_value","funds_available_for_
transfer","questions_or_comments");
#Thank you page - This is the page the user is redirected to.
$thankyou_url = "http://www.domain.com/thankyou.htm";
print $query->header;
my $field;
foreach $field (@required_fields) {
if(!$query->param($field)) {
print "<script>
alert(\"Please supply the following information: $fields{$field}\");
history.back();</script>";
exit;
}
}
$smtp = Net::SMTP->new($mailhost);
$smtp->mail($ENV{USER});
foreach $r (@mail_to)
{
$smtp->to($r);
}
$smtp->data();
$smtp->datasend("From: $mail_from\n");
$smtp->datasend("To: ".join(",",@mail_to)."\n");
$smtp->datasend("Subject: $subject\n");
$smtp->datasend("The following request has been received:\n\n");
my $outline;
foreach $f (@field_order)
{
$outline = sprintf("%s: %s",$f,$query->param($f));
$smtp->datasend($outline."\n");
}
$smtp->dataend();
$smtp->quit;
#Send out a Confirmation email
if($confirmation_email_to =~ /\@/) {
my $smtp = Net::SMTP->new($mailhost);
$smtp->mail($ENV{USER});
$smtp->to("$confirmation_email_to");
$smtp->data();
$smtp->datasend("From: $confirmation_email_from\n");
$smtp->datasend("To: $confirmation_email_to\n");
$smtp->datasend("Subject: $confirmation_subject\n");
$smtp->datasend("\n");
$smtp->datasend("$confirmation_body\n\n");
my $outline;
foreach $f (@field_order) {
$outline = sprintf("%s: %s",$f,$query->param($f));
$smtp->datasend($outline."\n");
}
$smtp->dataend();
$smtp->quit;
}
print "<META HTTP-EQUIV=refresh content=\"0;URL=$thankyou_url\">\n";
"Jason Miles" <ryder01@bellnexxia.net> wrote in message
news:OErtc.4637$9q1.5299@news20.bellglobal.com...
> I have a script which is processing form input. What I want is when the
> script sends an e-mail with the form contents, the From field in the
e-mail
> should be
> last_name,first_name
>
> The code i'm running currently produces
> last_name;first_name
>
> Here is the line of code I'm using
> $mail_from = $query->param("family_name").",".$query->param("given_name");
>
> I've also tried
> $mail_from = $query->param("family_name").','.$query->param("given_name");
>
> Can someone tell me what to change to get the desired result of
> last_name,first_name ?
>
> Thanks in advance!
>
>
| |
| Vorxion 2004-06-03, 7:13 pm |
| In article <40b71ff4$0$195$58c7af7e@news.kabelfoon.nl>, John Bokma wrote:
>Vorxion wrote:
>
>
>[..]
>
>
>I still bet he doesn't check. Most CGI scripts are badly written,
>especially by people who ask basic questions like the OP did.
You can bet. I simply don't consider it polite to crucify him based on
just that--a bet. If he admits he didn't, fine, I'll help you string him
up. :)
>
>Crap. As someone else already made clear, the Perl interpreter is way
>smarter than you think, "bla" is changed internally as 'bla'. Do you
>really think that "bla" is every time checked for magic? Don't you know
>anything about optimization, or do you assume $a = 5 + 7; is also
>evaluated every time in a loop? Perl is not JavaScript :-D.
*chuckle* Of course not to the last example. However, I wasn't aware it
would rewrite the interpolations until the other poster cited his example.
>
>Ofcourse, but they must be *ignored by the parser*. And yes, that takes
>time, but again, *insignificant*, as is the optimalisation of "foobar".
Yeah, but dropping comments is pretty much by and by as it's parsing for
precompile. I'd wholly agree that this is insignificant.
>
>Again you are wrong. But yes, the OP would be serious flamed, because
>the posting doesn't show the *ACTUAL* code, nor the way how he obtained
>the *wrong* result.
On which point am I wrong here? You just agreed with me that he'd get
flamed there. What, that they can't be bothered to read code that isn't
character-for-character how they'd write it? Hell, I've -been- through
that. I'm a big believer in consistant use of curly braces on rvalue
variables in -any- context, even where not necessary. I went through this
about a month and a half ago over there. They entirly ignored the
technical issue even though I -did- post the EXACT code they asked for, and
all but one ignored the technical issue in question in favour of critiquing
my coding style. Don't tell me I'm wrong--I've been there and seen it.
>
>Still, I am 100% sure the OP doesn't check. That he thought that a
>change from "," to ',' would make any difference clearly shows that the
>OP lacks a lot of basic Perl skills. I am also convinced that the OP
>just uses the obtained $mail_from without checking anything (like a
>sensible size for family and given name, and/or characters).
I don't deny it shows a lack of understanding. -I- am at a loss as to how
to explain why he gets a ; instead of a , even -if- he's not checking.
Either he should get ;, ,; or just plain , but NOT just plain ; in any
event. And that's assuming that you're right about his failure to check.
That it would also point to a bug in perl is another question entirely, as
a concatenation on an undef value is supposed to kick back a warning if
warnings are turned on, but otherwise basically skip over it as if it were
defined but empty.
I'd love to know what join() did inside his unknown context.
>
>This is not a personal preference but required in a CGI script. I am
>sick and tired of all those crappy CGI scripts being downloaded and
>used, and turning into tools used by spammers.
I'm not saying checking isn't necessary. Did you read my page? I read
yours. Christ, I couldn't agree more. I also couldn't agree more about
seeing all the poorly designed programs out there that get abused. It's
dismaying at best.
All I'm saying is that, your "bet" notwithstanding, you've tried and
convicted the guy without proof. Mexico may not have all the standards
we're used to stateside, but around here we have something called "innocent
until proven guilty". That aside, it's nice to give someone the benefit of
the doubt. You sound more jaded than I feel, geez. :)
>Too often I see postings, I don't understand Perl, but I found this
>script somewhere (no URL given, and the script is clearly crap), who can
>help me (for free) to turn it into what I want.
Hey, again, you're preaching to the choir. And I see it in mailing lists
as well, and it's in far more than just perl, believe me. You should have
seen the sendmail thread I was in with someone the other day. I had MAJOR
urges to whip out my LART and smack the cluebie with it, I can assure you.
>Usenet is not a helpdesk.
Depends on the group. This one certainly isn't meant to be. Other groups
are. There's a vast difference between c.l.p.m and c.l.p.tk for example.
The latter is actually made up of people that -are- willing to help. The
former appears to be there mostly for a lot of self-aggrandisement, so far
as I can tell.
I've nothing against helping people. But this really is the wrong group
for free advice, agreed. And it's not like one or two of us have said
it only once or twice. It's been a recurring theme for the last year or
so, and probably far before I bothered subbing and joining in on that
viewpoint.
In this case, I normally wouldn't have bothered, but the behaviour seemed
anomalous, I was curious, and your explanation of undef'd values, even if
he's not checking (which he SHOULD be, I agree), does not explain the lack
of a , entirely.
I don't suppose the OP would be so kind as to say whether he's later doing
a substitution of s/,/;/ that he's since forgotten about and overlooked?
That's the only thing I can think of that would rationally explain the
results he got. That, or post the whole script.
--
Vorxion - Member of The Vortexa Elite
| |
| Vorxion 2004-06-03, 7:13 pm |
| In article <eaHtc.41068$GYd.32615@news04.bloor.is.net.cable.rogers.com>, Jason Miles wrote:
>I seem to have started a pissing contest. So, here is the whole code, and I
>appologize for not posting it all in the first place. Comments welcome.
>Thanks for the help and I take note of all your comments...
Okay, I now agree with John--you have significant security issues.
In addition, on line 78 of your script, the @require_fields did not have a
my() or our() lexical scoping, so I'm going to assume you tacked that in at
the last minute so nobody would complain about you not using strict.
>#!/usr/bin/perl
>
>use strict;
>use CGI qw(:standard);
>use Net::SMTP;
>
> my($query,$mailhost,$mail_from,$subject,
$confirmation_email_to,$confirmation
> _email_from,$confirmation_subject,$thank
you_url,$smtp,$f,$r,$confirmation_bo
>dy);
>my @mail_to;
>my @field_order;
>my %fields;
>
>$query = new CGI;
>
>$mailhost = "smtp.domain.com";
>
>$mail_from = $query->param("family_name").",".$query->param("given_name");
As John said, you should be checking to make sure these are even populated.
However, if I insert the following at this point:
print("Content-type: text/plain\n\n$mail_from\n"); exit;
....I get correct results whether the fields are populated or not.
"Correct" being a relative term in that I will get a single comma if
neither field is populated, but if I plug in John Smith, I will indeed get
Smith,John as a result.
So while you do need to clean this up, the problem does not exist at this
point. Unfortunately, it doesn't appear that you do anything after this
point to actually alter the value. Have you tried using debugging on the
Net::SMTP module to see if that's where it gets thrown?
What version of perl are you running? I'm testing with 5.6.1 on Solaris at
the moment, although I use 5.8.3 in most places now.
> $smtp->mail($ENV{USER});
That's an issue. NEVER assume your environment. Make allowances for that
variable not even existing. This would flat-out cause bounced mails due to
a malformed address if that variable doesn't exist. it would not exist on
a win9x platform.
> foreach $field (@required_fields) {
> if(!$query->param($field)) {
> print "<script>
> alert(\"Please supply the following information: $fields{$field}\");
> history.back();</script>";
> exit;
> }
> }
The condition is wrong. It should be:
if (not defined($query->param($field))) {
Your entire scenario for dealing with missing fields is cracker-prone,
however. You tell them what fields they should fill in. That's basically
like writing a mini-tutorial on how to abuse your script from someplace
other than your form. It should ideally say "missing fields" and that's
about it. It should not say which ones are missing, IMNSHO. That's a
point of security.
You also have issues with $confirmation_mail_to, in that it's not
guaranteed to be filled in with an RFC822-compliant address. There are SO
many variations on address formats that you're going to have a hard time
addressing them all, but that simple check for an @ sign does -not-
constitute even close to the minimum regex I'd use.
At this point, I'd recommend pretty much a rewrite of your entire script.
It would address several major points and make it more versatile and
secure.
No offense, of course.
Okay, John, you called it. *sigh* Sorry to be proven wrong, really I am.
However, I can't even duplicate the results he says he's getting at that
specific point, given his actual script, even with him not checking. It
works fine to that point here on this server.
I'm sorry, Jason, but we now must string you up and crucify you for writing
sloppy and insecure CGI code. Sorry about the nails and all that...
At this point, I would suggest hiring one of these fine folks to fix (ie.,
rewrite) your script for you. It would be the most expedient solution.
I'm personally not interested--I have other fish to fry at the moment,
having postponed some system security audits for a couple of w s while
I've been ill.
Luck.
--
Vorxion - Member of The Vortexa Elite
| |
| John Bokma 2004-06-03, 7:13 pm |
| Vorxion wrote:
> In article <40b71ff4$0$195$58c7af7e@news.kabelfoon.nl>, John Bokma wrote:
>
[color=darkred]
>
> You can bet. I simply don't consider it polite to crucify him based on
> just that--a bet. If he admits he didn't, fine, I'll help you string him
> up. :)
He did, he did :-D.
> *chuckle* Of course not to the last example. However, I wasn't aware it
> would rewrite the interpolations until the other poster cited his example.
Perl does a lot of clever things under the hood :-D. And JavaScript,
when I tested it some time ago, *really* evaluates 5 * 7 in a loop,
everytime (can't remember which browser, either IE or Firebird)
> about a month and a half ago over there. They entirly ignored the
> technical issue even though I -did- post the EXACT code they asked for, and
> all but one ignored the technical issue in question in favour of critiquing
> my coding style. Don't tell me I'm wrong--I've been there and seen it.
I agree that the clp.* people are sometimes not nice (am there a lot),
but often they have a reason. About coding styles one can talk for hours
:-D.
> I don't deny it shows a lack of understanding. -I- am at a loss as to how
> to explain why he gets a ; instead of a , even -if- he's not checking.
> Either he should get ;, ,; or just plain , but NOT just plain ; in any
> event. And that's assuming that you're right about his failure to check.
> That it would also point to a bug in perl is another question entirely, as
> a concatenation on an undef value is supposed to kick back a warning if
> warnings are turned on, but otherwise basically skip over it as if it were
> defined but empty.
I guess the OP is seeing the weird behaviour somewhere else, not in the
script. Unless the param contains a backspace and a ; which overwrites
the , which I doubt :-D:
> I'd love to know what join() did inside his unknown context.
The same. (Another bet ;-) )
> I'm not saying checking isn't necessary. Did you read my page? I read
> yours.
Thanks :-D Probably add a link to yours :-D.
> All I'm saying is that, your "bet" notwithstanding, you've tried and
> convicted the guy without proof. Mexico may not have all the standards
> we're used to stateside, but around here we have something called "innocent
> until proven guilty". That aside, it's nice to give someone the benefit of
> the doubt. You sound more jaded than I feel, geez. :)
Ah, but you are mistaken, I am Dutch, living in Mexico :-D.
> In this case, I normally wouldn't have bothered, but the behaviour seemed
> anomalous, I was curious, and your explanation of undef'd values, even if
> he's not checking (which he SHOULD be, I agree), does not explain the lack
> of a , entirely.
I agree.
> I don't suppose the OP would be so kind as to say whether he's later doing
> a substitution of s/,/;/ that he's since forgotten about and overlooked?
> That's the only thing I can think of that would rationally explain the
> results he got. That, or post the whole script.
He did, and I am going to look at it *shiver*
--
John MexIT: http://johnbokma.com/mexit/
personal page: http://johnbokma.com/
Experienced Perl programmer available: http://castleamber.com/
| |
| John Bokma 2004-06-03, 7:13 pm |
| Jason Miles wrote:
> I seem to have started a pissing contest.
Nah, just a normal Usenet discussion.
> So, here is the whole code, and I
> appologize for not posting it all in the first place. Comments welcome.
> Thanks for the help and I take note of all your comments...
>
> #!/usr/bin/perl
>
> use strict;
add use warnings;
I prefer -T after the she-bang too.
> use CGI qw(:standard);
> use Net::SMTP;
>
> my($query,$mailhost,$mail_from,$subject,
$confirmation_email_to,$confirmation
> _email_from,$confirmation_subject,$thank
you_url,$smtp,$f,$r,$confirmation_bo
> dy);
Don't do it like that, but "my" when you use them.
> my @mail_to;
> my @field_order;
> my %fields;
>
> $query = new CGI;
I never understood why this is always called query, often it is not the
query it interacts with. I prefer my $cgi = new CGI;
>
> $mailhost = "smtp.domain.com";
I would put this one more below.
>
> $mail_from = $query->param("family_name").",".$query->param("given_name");
I recommend using something like:
my $family_name = $cgi->param('family_name');
$family_name or die 'family_name not given';
more sanity checks, like the length of family name, and if it contains
garbage. I recommend to die if it is out of specs, not silently fix it
(I assume you use fatals to browser, see CGI documentation)
same for given_name
You could use die $mail_form; here and I doubt you will see a ; at this
point.
> @mail_to = ('lawyers@domain.com');
>
> $subject = "Assessment Submission";
>
> $confirmation_email_to = $query->param("email_address");
>
> $confirmation_email_from = 'lawyers@domain.com';
>
> $confirmation_subject = ("Free Assessment Confirmation");
>
> $confirmation_body = "Thank you for completing the Preliminary Assessment
> Questionnaire.\n
> One of our staff lawyers will review your assessment
> and will reply to you within 72 hours.\n
> Please be assured that the information you provide
> will be kept in the strictest of confidence
> and will not be released to any third party for any
> reason without your express written consent.";
my $confirmation_body = <<'CONFIRM';
Thank you....
One of ...
Please ...
and ...
CONFIRM
Is one way to do this. Beware: no spaces after CONFIRM
> @field_order =
> ("heard_by","heard_by_country","referred_by","referred_to","salutation","giv
> en_name","family_name","email_address","nationality","mailing_address","stat
> us","home_phone","work_phone","fax","dob_day","dob_month","dob_year","marita
> l_status","childrens_ages","speaks_english","reads_english","writes_english"
> ,"speaks_french","reads_french","writes_french","years_of_education","educat
> ion_certificate","field_of_study","number_of_years","education_country","add
> itional_certificates","current_occupation","job_1_from_year","job_1_to_year"
> ,"job_1_company","job_1_duties","job_2_from_year","job_2_to_year","job_2_com
> pany","job_2_duties","job_3_from_year","job_3_to_year","job_3_company","job_
> 3_duties","comments_on_employment","spouse_salutation","spouse_given_name","
> spouse_family_name","spouse_nationality","spouse_dob_day","spouse_dob_month"
> ,"spouse_dob_year","spouse_speaks_english","spouse_reads_english","spouse_wr
> ites_english","spouse_speaks_french","spouse_reads_french","spouse_writes_fr
> ench","spouse_years_of_education","spouse_education_certificate","spouse_fie
> ld_of_study","spouse_number_of_years","spouse_country","spouse_additional_ce
> rtificates","spouse_current_occupation","spouse_job_1_from_year","spouse_job
> _1_to_year","spouse_job_1_company","spouse_job_1_duties","spouse_job_2_from_
> year","spouse_job_2_to_year","spouse_job_2_company","spouse_job_2_duties","s
> pouse_job_3_from_year","spouse_job_3_to_year","spouse_job_3_company","spouse
> _job_3_duties","spouse_comments_on_employment","relative_relationship","net_
> worth","employment_offer","employment_offer_explanation","serious_disease","
> convicted_or_charged","previously_applied_for_visa","previously_visited_cana
> da","able_to_obtain_visa","previous_business_nature","previous_duties","perc
> entage_of_ownership","number_of_employees","3_year_history_year_1","3_year_h
> istory_year_1_turnover","3_year_history_year_1_net_profit","3_year_history_y
> ear_2","3_year_history_year_2_turnover","3_year_history_year_2_net_profit","
> 3_year_history_year_3","3_year_history_year_3_turnover","3_year_history_year
> _3_net_profit","business_net_worth_list","asset_value","funds_available_for_
> transfer","questions_or_comments");
use qw as follows:
my @field_order = qw(heard_by heard_by_country ...);
> #Thank you page - This is the page the user is redirected to.
> $thankyou_url = "http://www.domain.com/thankyou.htm";
>
> print $query->header;
Yup, and that's why I prefer $cgi. How can you print a header to a query
:-D.
> my $field;
>
> foreach $field (@required_fields) {
for my $field (@required_fields) {
> if(!$query->param($field)) {
> print "<script>
> alert(\"Please supply the following information: $fields{$field}\");
> history.back();</script>";
> exit;
> }
> }
I would use something like:
defined ($query->param($field)) and next;
print <<MISSING_FIELD
<script>
alert....
</script>
Including HTML that works when JavaScript doesn't work
MISSING_FIELD
> $smtp = Net::SMTP->new($mailhost);
I would put mailhost on top of your script (under the use part) and
either use:
use constant MAIL_HOST => 'your.mail.host';
or
my $MAIL_HOST = 'your.mail.host';
Since it's a config constant.
>
> $smtp->mail($ENV{USER});
> foreach $r (@mail_to)
> {
> $smtp->to($r);
> }
> $smtp->data();
> $smtp->datasend("From: $mail_from\n");
> $smtp->datasend("To: ".join(",",@mail_to)."\n");
> $smtp->datasend("Subject: $subject\n");
> $smtp->datasend("The following request has been received:\n\n");
If you do the To join more up in a scalar, you can use
$smtp->datasend(<<HEADER);
From ...
To: ...
etc.
HEADER
And I guess you need two newlins after Subject...
> my $outline;
> foreach $f (@field_order)
> {
> $outline = sprintf("%s: %s",$f,$query->param($f));
$smtp->datasend("$f: " . $cgi->param($f) . "\n");
I would probably use a map and a join, and datasend all in one go
together with the headers.
> $smtp->datasend($outline."\n");
> }
> $smtp->dataend();
> $smtp->quit;
>
> #Send out a Confirmation email
>
> if($confirmation_email_to =~ /\@/) {
>
> my $smtp = Net::SMTP->new($mailhost);
>
> $smtp->mail($ENV{USER});
> $smtp->to("$confirmation_email_to");
>
> $smtp->data();
> $smtp->datasend("From: $confirmation_email_from\n");
> $smtp->datasend("To: $confirmation_email_to\n");
> $smtp->datasend("Subject: $confirmation_subject\n");
> $smtp->datasend("\n");
> $smtp->datasend("$confirmation_body\n\n");
> my $outline;
> foreach $f (@field_order) {
> $outline = sprintf("%s: %s",$f,$query->param($f));
> $smtp->datasend($outline."\n");
> }
> $smtp->dataend();
> $smtp->quit;
>
> }
I would make one sub that sends an email. Also, should't you check if
things went ok?
> print "<META HTTP-EQUIV=refresh content=\"0;URL=$thankyou_url\">\n";
Use $cgi->location() (IIRC, see redirecting in CGI doc) (Note that you
already printed the header, you should do that in the fields check part,
not before if you use redirection)
You see the ; in your email? I guess it's replaced by SMTP.
--
John MexIT: http://johnbokma.com/mexit/
personal page: http://johnbokma.com/
Experienced Perl programmer available: http://castleamber.com/
| |
| John Bokma 2004-06-03, 7:13 pm |
| Vorxion wrote:
> So while you do need to clean this up, the problem does not exist at this
> point. Unfortunately, it doesn't appear that you do anything after this
> point to actually alter the value. Have you tried using debugging on the
> Net::SMTP module to see if that's where it gets thrown?
My best guess is the OP sees it in the email he receives. And that SMTP
(or something on the way) changes the , into a ;
> Your entire scenario for dealing with missing fields is cracker-prone,
> however. You tell them what fields they should fill in. That's basically
> like writing a mini-tutorial on how to abuse your script from someplace
> other than your form. It should ideally say "missing fields" and that's
> about it. It should not say which ones are missing, IMNSHO. That's a
> point of security.
Basically through obscurity. A view source could reveal all fields.
> You also have issues with $confirmation_mail_to, in that it's not
> guaranteed to be filled in with an RFC822-compliant address. There are SO
> many variations on address formats that you're going to have a hard time
> addressing them all, but that simple check for an @ sign does -not-
> constitute even close to the minimum regex I'd use.
Randall has a nice one :-D.
> I'm sorry, Jason, but we now must string you up and crucify you for writing
> sloppy and insecure CGI code. Sorry about the nails and all that...
And the hot coals :-D.
> At this point, I would suggest hiring one of these fine folks to fix (ie.,
> rewrite) your script for you. It would be the most expedient solution.
Me.. Me... :-D. Although I helped already a bit here and there.
> I'm personally not interested--I have other fish to fry at the moment,
> having postponed some system security audits for a couple of w s while
> I've been ill.
Get well soon.
--
John MexIT: http://johnbokma.com/mexit/
personal page: http://johnbokma.com/
Experienced Perl programmer available: http://castleamber.com/
| |
| Vorxion 2004-06-03, 7:13 pm |
| In article <40b766f6$0$207$58c7af7e@news.kabelfoon.nl>, John Bokma wrote:
>Vorxion wrote:
>
>
>My best guess is the OP sees it in the email he receives. And that SMTP
>(or something on the way) changes the , into a ;
Oh, Christ on a crutch! John, you hit it. I'd lay money.
HOW does OutHouse [Express] delineate multiple addresses? Not with
a comma like regular MUA's in *nix-land (or anything else sane) but
with a semicolon. It -requires- semicolons when inputting multiple
addresses. If he's viewing it with Outlook or OE, and it translates
"intelligently" (NOT!) figuring that it's getting mail from its ilk, I bet
it goes through raw just fine but you're exactly right about seeing it
displayed incorrectly. Your thoughts triggered my exact memory of the one
thing I know that would make that -specific- substitution.
Jason, can you confirm you're using a flavour of Outlook to view the
results?
I was onto it when I suggested debug on Net::SMTP as well. I just couldn't
put my finger on why -that- would do it, since I use it all the time and
it's never misbehaved. I don't think it's in the transport, I think it's
in the MUA on the other end!
>
>Basically through obscurity. A view source could reveal all fields.
Not necessarily. It will reveal all fields on the form, yes. It won't
tell you exactly which ones MUST be filled in, however. A nice trick is to
make the submit button -mandatory- (and force the event handler to disallow
text fields from submitting on 013). So many will copy every form field,
but not the submit field. And not only make sure the submit field is
there, but that the contents -match- that of the button you use to submit
it with. This will cut down on abuse even further.
>
>Randall has a nice one :-D.
I didn't want to bother with that, personally. Half the address schemes
out there are obsolete. When was the last time you saw user%host@host? It
was about '94 for me. It's valid, but who uses it anymore? I stick with
something very similar to: /^[^ @]+@[^ @\.]+\..*/
That's not my official one, and I'd actually have to look it up. I
basically make sure there's a username, at sign, and at least one non-TLD
field to the domain name before the TLD. Good enough for a quickie, and
better than what he had. Not in full RFC822 compliance, and I know it. I
just don't like to bloat things unnecessarily. :) I've heard of the other
official module, I've just never really felt it -that- necessary to support
outmoded addresses. Just something moderately sane is fine for me for most
purposes in this regard.
>
>And the hot coals :-D.
You do that before or after the actual nailing? :)
>
>Me.. Me... :-D. Although I helped already a bit here and there.
*chuckle* Indeed. A bit of strain aside, it's been a nice collaboration
looking at it with you.
>
>Get well soon.
Thanks muchly. Trying. :)
--
Vorxion - Member of The Vortexa Elite
|
|
|
|
|