You've obviously put a lot of time in this and I really wanted to take a closer look, but there are some issues with the code the prevent me from delving into it. To be fair, I've been pretty busy at work lately so having something that takes more than a couple of minutes to dig into is problematic. Ironically, I wound up spending about a quarter hour dissecting your code rather than looking for the source of your problem. You clearly grasp many of the basics, so I hope you don't take the following as a complete slam.
You have the right idea to put together a test to isolate the problem. However, well-designed code is typically quick to debug and there are some of issues with your code. Improving these will make the problem clearer.
1. Create a minimal test case
You've obviously put a lot of time and effort into creating a Web form to assist your research. In this case, strip all of that out. It only gets in the way of seeing the problem. Quite often, by stripping away all of the extraneous details, the bug (if any) stands out clearly. You have around 170 lines of code. That is too many to really call a minimal test case.
2. Eliminate globals
At the top of your code, we see the following:
$main::cookie_name = $cgi->param('cookie_name'); $main::full_url = $ENV{SERVER_URL} . $ENV{SCRIPT_NAME}; $main::domain = $cgi->param('domain'); $main::path = $cgi->param('path'); $main::cookie_value = $cgi->param('cookie_value'); $main::domain ||= '.mybc.com'; $main::path ||= '/'; $main::cookie_value ||= 'na';
Those globals can be problematic. They're indicative of some deep scoping issues that you may not be aware of. At different part of your code, you refer to $cookie and it's not immediately clear if you are referring to a lexically scoped variable or the global one. Also, though you didn't appear to have any, this can increase the chance for typos. At the very least, use the vars pragma.
$main::cookie_value ||= 'na';
Later, in your remove_cookie() subroutine, you have the following items being passed to CGI::cookie(), along with an interesting comment (extra comments removed for clarity):
my $cookie = $cgi->cookie( -name => $main::cookie_name, -value => 1, # also this one can't be 0 or undef! -expires=> 1, -path => $main::path, -domain => $main::domain, );
Your comment says that value "can't be 0 or undef", but there's no problem with a cookie value of 0 or even an empty value. However, your ||= assignment earlier really isn't a "default" assignment operator. It's a "assign a new value if the old value is false" operator. Since both 0 and undef are false...
3. Clean up your scoping
Your scoping is inconsistent and difficult to follow. Contrast the following two subroutines and the scoping on $cookie:
sub set_cookie { my $cgi = shift; unless ($main::cookie) { $main::cookie = $cgi->cookie( -name => $main::cookie_name, -value => $main::cookie_value, -expires=> '+1h', -path => $main::path, -domain => $main::domain, ); print CGI::redirect(-location => $main::full_url, -cookie +=> [$main::cookie]); exit; } }
Now take a look at the get_cookie routine:
sub remove_cookie { my $cgi = shift; $DB::single = 1; if ($main::cookie) { my $cookie = $cgi->cookie( -name => $main::cookie_name, -value => 1, # also this one can't be 0 or undef! -expires=> 1, -path => $main::path, -domain => $main::domain, ); print CGI::redirect(-location => $main::full_url, -cookie +=> [$cookie]); exit; } }
In the first example, you assign the new cookie to $main::cookie. In the second example, you have a lexically scoped $cookie variable. The first subroutine overwrites your global variable and the second example does not.
These routines also show that there are multiple paths to exit this program. That is very tough to follow.
4. Avoid "tricks" when trying to debug a problem
In your parse_all_cookies() subroutine, the first line is this:
my ($r_html_table, $r_html_select) = @{$_[0]}{qw(html_table ht +ml_select)};
What's that? I use references all the time, but this gave me some serious pause. You're using a hash slice to get to some scalar references so you can assign to the referents rather just return the results. Here's the full subroutine:
sub parse_all_cookies { my ($r_html_table, $r_html_select) = @{$_[0]}{qw(html_table ht +ml_select)}; $$r_html_table = qq| <table border=0 cellpadding=2 cellspacing=3 bgcolor=li +ghtgrey> <tr><td><b>Cookie Name</b></td><td><b>Content</b></td> +</tr> |; foreach (keys %main::cookies) { $$r_html_select .= "<option value='$_'>$_</option>"; $$r_html_table .= "<tr><td>$_</td><td>".$main::cookies{$_} +."</td></tr>"; } $$r_html_table .= "</table>"; }
And it's called like this:
my ($html_cookie_table, $html_cookie_select); parse_all_cookies({ html_table => \$html_cookie_table, html_select => \$html_cookie_select, });
How about doing this (ignoring the fact that creating an elaborate table and form is overkill)?
use HTML::Entities; my ($html_cookie_table, $html_cookie_select) = parse_all_cookies( +\%cookies ); sub parse_all_cookies { my $cookies = shift; my $table = qq| <table border="0" cellpadding="2" cellspacing="3" bgco +lor="lightgrey"> <tr><td><b>Cookie Name</b></td><td><b>Content</b></td> +</tr> |; my $select = ''; while (my ( $name, $value ) = each %$cookies) { encode_entities( $name ); encode_entities( $value ); $select .= "<option value='$name'>$name</option>"; $table .= "<tr><td>$name</td><td>".$value."</td></tr>"; } $table .= "</table>"; return ( $table, $select ); }
That's a quick, untested hack, but it's easier to follow. Also note the use of HTML::Entities. If you have any problematic characters (such as quote marks), then you'll have problems if you don't quote them.
Conclusion
I suspect that the actual problem may be fairly basic, but I found myself focusing so much on some of the code issues that I really didn't focus on the problem at hand. Cleaning this up and reducing it to a minimal test case could help tremendously. Also, have you considered the possibility that your use of JavaScript may cause some of the cross-browser issues?
Cheers,
Ovid
Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.
In reply to (Ovid - code review) Re: CGI::cookie()
by Ovid
in thread CGI::cookie() bugged? problems with removing cookies..
by vladb
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |