plack / Plack-Middleware-Session

A very minimalist session library for Plack
Other
39 stars 28 forks source link

Storable deserealization fail should cause 500 #20

Open hmepas opened 10 years ago

hmepas commented 10 years ago

If you move your code from old server with 32bit arch and old Storable to a new one, and an old client comes in with old cookie it'll cause Storable::thaw to die on that data. Aint good behavior for a site to throw out 500 like this.

Here is the quick patch i propose at the line 24:

package Plack::Middleware::Session::Cookie;
...
    $self->deserializer(sub {
        my $result;
        eval {
            $result = Storable::thaw(MIME::Base64::decode($_[0]))
        };
        return if $@;
        return $result;
    }) unless $self->deserializer;
forwardever commented 10 years ago

+1

by the way, I use something like this

  serializer => sub {
      MIME::Base64::encode(
          JSON::XS::encode_json($_[0])
      )
  },
  deserializer => sub {
      my $result;

      eval {
          $result = JSON::XS::decode_json(MIME::Base64::decode($_[0]))
      };

      # legacy cookies
      if ($@) {
          eval {
              $result = Storable::thaw(MIME::Base64::decode($_[0]))
          };
          return if $@;
      }
      return $result;
  }
abh commented 9 years ago

FWIW I ran into this, too (and also prefer the JSON serialization for don't-break-so-easily-in-the-future-ness).