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

I have a script that processes user sign in/out request. One of a few things this script has to do is remove/set appropriate cookies to control user sessions. The problem that I've encountered with the script was that while setting a cookie works 100% of the time, removing a cookie is sometimes problematic.

In order to investigate this problem, I've written a simple cookie manipulation test script. It would display a form with a single submit button, which function is to either 'Remove Cookie' or 'Set Cookie'. The code for this test page is included at the end of this post.

To remove a cookie, I simply create one using CGI's cookie() method and passing it appropriate cookie parameters/values. Finally, I invoke CGI::redirect to create a redirect header containing cookie string (I redirect to the page itself). Having played with the test script a few times (on Mozilla 5.0 rv:0.9.7) I have made a somewhat 'chilly' discovery. Setting -expires option of an existing cookie to 'now' (which will get translated to time() right?) wouldn't remove it immediately. And, I end up having to retry a few times (by hitting the 'Remove Cookie' button) in order to actually make the cookie go. However, setting the -expires option to 1, makes things work the first time.

I also noticed that the 'now' value wouldn't remove my cookie on IE5.0 browsers at all ;/, no matter how many times you try. Frankly, I'm buffled by this problem. I looked at redirect header data produced in two cases, and they don't seem to be all that different (unless I missed something ;-), except for expire times. However, now did produce 'current' expire time.. which had to work, logically.

Any idea why, contrary to perldoc CGI documentation, setting -expires to 'now' wouldn't work? Actually, I've learnt that -value property has to be set to a 'non-false' value (neither 0 nor undef is allowed), otherwise, cookie wouldn't get removed either.

Thanks.

Here goes the source code for cookie_test.cgi
use strict; use lib qw(.); use CGI; use CGI::Cookie; my $cgi = new CGI; my $op = $cgi->param('op'); $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'; #$main::domain = $ENV{HTTP_HOST}; #($main::path) = ($ENV{SCRIPT_NAME} =~ m|(.*)/[^/]+$|); $main::cookie = $cgi->cookie($main::cookie_name); #$op='rm';$main::cookie=1; set_cookie($cgi) if ($op eq 'set'); remove_cookie($cgi) if ($op eq 'rm'); %main::cookies = fetch CGI::Cookie; my ($html_cookie_table, $html_cookie_select); parse_all_cookies({ html_table => \$html_cookie_table, html_select => \$html_cookie_select, }); print $cgi->header(); print qq| <html> <title>Cookie Test</title> <body> <pre> Cookie set/remove test. <form method=POST> <input type=hidden name=op value="$op"> <table border=0> <tr> <td></td><td></td> </tr> <tr> <td align=left>Name:</td> <td align=left nowrap> <input name=cookie_name value="$main::cookie_name"> &nbsp; <script> function select_cookie(sel_obj) { if (sel_obj.selectedIndex) { sel_obj.form.cookie_name.value = sel_obj.options[sel_obj.s +electedIndex].value; } else { sel_obj.form.cookie_name.value = ""; } } </script> <select name=cookie_name_select onChange="select_cookie(this);"> <option value=0>Select existing ...</option> $html_cookie_select </select> </td> </tr> <tr> <td>Value:</td><td><input name=cookie_value value="$main::cookie_valu +e"></td> </tr> <tr> <td align=left>Domain:</td><td align=left><input name=domain value="$ +main::domain"></td> </tr> <tr> <td align=left>Path:</td><td align=left> <input name=path value="$mai +n::path"></td> </tr> <tr><td colspan=2 align=center> <input type=button value="Remove Cookie" onclick="this.form.op.value += 'rm';this.form.submit();"> <input type=button value="Set Cookie" onclick="this.form.op.value = ' +set';this.form.submit();"> <input type=submit value="Refresh" onclick="this.form.op.value = '';" +> </td></tr> </table> </form> </pre> <hr> $html_cookie_table </body> </html> |; exit; ## ## SUBS ## 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 => [$mai +n::cookie]); exit; } } 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! # Setting expires to 'now' doesn't quite + work. # I have to hit the 'Remove Cookie' butt +on # 2/3 times (# of remove attempts) to ac +tually # remove the cookie. # -expires => 'now', # however, this works well all the time. # what's the trick? -expires=> 1, -path => $main::path, -domain => $main::domain, ); # print $cgi->header(); # print qq| # <pre> # REMOVING COOKIE: # -name => $main::cookie_name, # -value => 'foobar', # -expires=> 'now', # -path => $main::path, # -domain => $main::domain, # </pre> # |; print CGI::redirect(-location => $main::full_url, -cookie => [$coo +kie]); exit; } } sub parse_all_cookies { my ($r_html_table, $r_html_select) = @{$_[0]}{qw(html_table html_s +elect)}; $$r_html_table = qq| <table border=0 cellpadding=2 cellspacing=3 bgcolor=lightg +rey> <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>"; }


"There is no system but GNU, and Linux is one of its kernels." -- Confession of Faith

Replies are listed 'Best First'.
(Ovid - code review) Re: CGI::cookie()
by Ovid (Cardinal) on Feb 20, 2002 at 21:26 UTC

    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.

Re: CGI::cookie() bugged? problems with removing cookies..
by screamingeagle (Curate) on Feb 20, 2002 at 20:41 UTC
    you might have tried this already : instead of setting the expires to "now", try setting it to a time somewhere in the past, say 2 weeks back...