plack/Plack

Multiple cookie handling is broken in Plack::Test::MockHTTP 1.0044

fgabolde opened this issue · 3 comments

Plack seems to turn multiple cookie headers into a single header with values separated by commas, e.g.:

use strict;
use warnings;
use Data::Dumper;
use HTTP::Message::PSGI qw(req_to_psgi);
use HTTP::Request;
use Plack::Request;

my $req = HTTP::Request->new(GET => "http://localhost",
                             [ Cookie => 'foo=bar',
                               Cookie => 'baz=quux' ]);

my $env = req_to_psgi $req;
my $plack_req = Plack::Request->new($env);
print Dumper($plack_req->cookies);

will print {'foo' => 'bar', 'baz' => 'quux'} correctly using Cookie::Baker 0.007, but 0.008 disallows commas as value separators so we get {'foo' => 'bar, baz=quux'}.

This breaks any tests that use HTTP::Request and multiple cookie values, e.g.

my $request = HTTP::Request->new(GET => '/',
                                 [ Cookie => 'foo=bar',
                                   Cookie => 'baz=quux' ]);
my $response = $instance->request($request);

This is a deliberate change in Cookie::Baker 0.08 to fix some security issues, which I forgot the details about. @kazeburo ?

https://tools.ietf.org/html/rfc6265 prohibits the use of multiple Cookie headers sent from clients anyway, so the test can be rewritten using Cookie => 'foo=bar;baz=quux' .

When the user agent generates an HTTP request, the user agent MUST
NOT attach more than one Cookie header field.

As @miyagawa points out this is an error of the application. In addition to RFC 6265, RFC 7230 also states:

A sender MUST NOT generate multiple header fields with the same field name in a message unless either the entire field value for that header field is defined as a comma-separated list [i.e., #(values)] or the header field is a well-known exception (as noted below).

The exception noted here is Set-Cookie.

Ah thanks, I learned something today.