[ic] BUG: Vend::Order::route_order() breaks tie between $::Values and $Vend::Session->{values}

Peter peter at pajamian.dhs.org
Mon Nov 4 19:21:10 UTC 2013


On 11/05/2013 03:45 AM, Mike Heins wrote:
> 
> This was all intentional. The point was to allow individual routes to
> modify $Values without affecting the state of the hash at the next
> route. I would be surprised if we didn't later reconnect
> $Vend::Session->{values}, but I suppose it is possible

It's definately not reconnected at line 1963 of Order.pm and I don't see
anyplace after that in route_order() when it would be reconnected either.

> -- this is
> intended to be the last part of a process of submitting an order and
> the only processing left to do should be presenting a receipt (from
> the original point of view, anyway).

Right, this came about because I wanted to do a bit of cleanup on the
session after a transaction went through, and so after checking docs and
code decided that commit on the main route was the place to do it, but
what I found, to my surprise is that any changes made to $Values in the
commit perl block don't stick in the session, but if I changed
$Session->{values} instead then they stuck.

> This code has been in operation like this since 1997. If we mess with
> it, we would undoubtedly have the very real potential to break any
> site upgrading.

Maybe the best thing to do, then is to just document the workaround of
using $Session->{values} in the docs for commit and rollback.

Note that this is somewhat of a security bug as well, since this change
at line 1994 which attempts to sanitize the credit card info would not
be saved to the session:

         # Get rid of this puppy
         $::Values->{mv_credit_card_info}
                         =~ s/^(\s*\w+\s+)(\d\d)[\d ]+(\d\d\d\d)/$1$2
NEED ENCRYPTION $3/;

...instead the original value for mv_credit_card_info would get saved,
but I think that should at least be encrypted at this point anyways.  It
might be worthwhile for me to double check what exactly gets into the
session file under that variable after checkout is complete, though.


Peter



More information about the interchange-users mailing list