[ic] Fixed rare [read-cookie] bug

David Christensen david at endpoint.com
Wed Mar 9 08:19:41 UTC 2011


Folks,

Primarily for the archives, I recently hunted down a very obscure set of circumstances in which [read-cookie <name>] can result in an incorrect value for the specific cookie.  This came up for a client in which ~ 1% of orders were having an invalid affiliate field, which itself had been populated by this cookie.  From the commit message:

commit d32ba730f652480ed506d5cd0409e1f72881e330
Author: David Christensen <david at endpoint.com>
Date:   Wed Mar 9 01:25:31 2011 -0600

   Fix a bug in read_cookie's code path when using the single-arg form

   This issue was caused by a bug in the interchange read_cookie codepath
   which was being too lentient about its parsing of $CGI::cookie when
   looking up a specific cookie's value.

   Certain $CGI::cookie strings and requested cookie names can result in
   returning the wrong value for the cookie given the following
   circumstances: $CGI::cookie contains a value portion of the keyvalue
   pairs which include a word-break character, the (case-insensitive)
   target name and then an equals sign.  Additionally, this matching
   substring would need to appear before the actual cookie for the key in
   question.

   Example:

   Given $ENV{HTTP_COOKIE}:
     'foo.tracker={"url":"http://www.site.com/?mv_source=blah","count":3}; MV_SOURCE=foo'

   [read-cookie] without arguments would correctly parse and return the
   expected keypairs, however [read-cookie MV_SOURCE] would scan the
   $CGI::cookie string for a word-break, the specific cookie name, a
   literal '=' and then proceed to return the literal:

     MV_SOURCE => 'blah","count":3}'

   This fix tightens up the parsing to only look at the start of the
   string or immediately after a ';' (with optional whitespace between)
   when parsing a specific cookie value.

   ---
   Some additional comments:

   I had difficulty locating a specification for the cookie keys/values
   themselves, but I wonder if we should remove the /i regex modifier, as
   I'd personally expect cookie names to be case-sensitive.  Left in for
   backwards-compatibility.

   Additionally, the setter of the aforementioned cookie should likely
   have used some form of uriencoding instead of having the raw '{}='
   characters, however that's no excuse for us to barf on bad behavior.

Any thoughts/opinions on whether [read_cookie foocookie] should be case-insensitive?

Regards,

David
--
David Christensen
End Point Corporation
david at endpoint.com







More information about the interchange-users mailing list