if ($tainted =~ /\A([a-zA-Z0-9]{1-16})\z/i)
My immediate thought was, if I wanted to check the length of a string, I would use the built-in length() function.
My second thought was, as this line
$tainted =~ s/\W//g;
removes all non-word characters, why bother checking for them all again except one (the underscore) in the next line? Especially as the sub as it stands will return, say, 'NotaNumber' if it's fed with 'Not-a-Number' but will print 'failure' if it gets 'Not_a_Number'. Is this what you really want?
My third thought was that even if your sub prints 'failure', it returns 1 (assuming that the print is successful), which by your definition ('usernames composed of letters or numbers from 1-16 characters') is a valid username :-)
Maybe something like this would be better:
sub untaint_username {
my $str = shift;
$str =~ s/[\W_]//g; # remove all non word characters (incl. unders
+core)
if ( $str and length $str < 17 ) {
return $str;
}
else {
return 'failure!';
}
}
Note the exclamation mark in return 'failure!', which is there solely to allow you to deal with the remote possibility that one of your users might decide to call themselves 'failure'...
dave
Update: Just realised that, by your definition, '0' (zero) is also a valid username. Change the if... line to:
if ( defined $str and length $str < 17 ) {
Update 2: Better still:
if ( $str and defined $str and length $str < 17 ) { |