Reactor wrote:
I'm not going to answer your question the way you asked because well, I don't claim to be a Perl guru and in many cases my style of coding will assure I probably never become one. ;)I am fairly new to perl, so this is probably looks like a silly question: I am trying to write text to a file as specified by the query string environment variable. Since the file names are all numbers, I'm using a regex to strip anything other than a digit from the variable, and assign the new value to a variable. I've R-ed a few different FM's for the way to do this, and it says to use the regex memory value, which isn't tainted. When I try this using my current regex it leaves the $1 variable undefined. Code snipet:@temp = split(/=/, $ENV{'QUERY_STRING'}); $temp[0] =~ s/([^0-9])//g; $filename = $1; I made a sort of mini-debug function that prints out each variable. It prints the unprocessed query string after spliting and the value of $temp[0] after processing (which is all numbers) correctly, but the variable $filename doesn't have a value... Not sure where I went wrong with this... Unless the $1 is null because the matched pattern is deleted... or does the $1 hold the return value?
What do I mean by this?
I guess I am a bigger fan of clear code. I think that Perl is an exquisite language with neat side effects and ways to do several things with one statement.
But in the end, I have to say, that personally, I much prefer if someone doesn't code using "double meaning" and instead breaks it into more lines so that it is more readable like a book.
How does this relate to your question?
Well, I guess it relates because I think I wouldn't write the code the same way. Let's look at it ...
> @temp = split(/=/, $ENV{'QUERY_STRING'});
> $temp[0] =~ s/([^0-9])//g;
> $filename = $1;
Line 2 says that you are not only trying to get $1, but you are also performing this substitution operation for some reason (perhaps there is something else about this variable that you want to keep left over?)
I would rewrite the above to be closer to the following (style wise)
@http_form_input = split(/=/, $ENV{'QUERY_STRING'});
# explicitly check for the first complete string of digits in the
# http form input
if ($http_form_input[0] =~ /([0-9]+)/) {
$filename = $1;
} else {
die("Filename consisting of numbers expected but not found");
}
# strip away anything that is not a digit just as before
$http_form_input[0] =~ s/[^0-9]//g;
Here there are a couple things to note:
1. Variable names like @temp is not that descriptive in saying what you are actually trying to extract or do.
Note: It is possible one bug in your code (and hence also in mine) is that you are performing the search/replace on the 0th element. But what is the 0th element? If your form submission was filename=343242
Then the 0th element will equal the string "filename" not the string "343242" so you are doing the operation on the wrong element. Maybe you should be doing it on $http_form_input[1]??
This is why it tends to be nicer to use libraries like CGI.pm to do your query string processing for you. It's hard to make a mistake like this (if indeed this was a mistake) than when you manually deconstruct the query string.
Also, CGI.pm can transparently hide the difference between whether the variable was POSTed, GETed, or is interpreted via Mod_perl. So your script can automatically work in many more environments.
2. I separate the operation of untainting from the operation of search and replace.
In fact, this may also be a place where you have a bug. Your search and replace is removing all non digits. But what you actually want $1 to contain only digits.
So the regex that returns only digits is very different from the regex that destroys digits via search/replace.
Because you were attempting to do two things, one of the operations worked, but you got confused about why the 2nd one didn't work (untainting). The reason they don't work is that they aren't the same operation, so attempting to combine them as one actually did not work for you...
By separating your operations, things can be made cleaner and easier to interpret when you go back to look at your own code or it comes down to debugging.
Anyway, go ahead and do whatever you want... It's not my place to "preach", but hopefully I have answered your question about why the original code wasn't doing what you wanted, even if I answered it in a roundabout way.
Good Luck...
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
