Duplication Of Cookies [rt.cpan.org #75897]

Migrated from rt.cpan.org#75897 (status was 'open')

Requestors:

• vaibhavkhunger@gmail.com

Attachments:

• [Error report](https://rt.cpan.org/Ticket/Attachment/1052365/550310/Error report)

From vaibhavkhunger@gmail.com on 2012-03-20 06:24:48:

Bug in HTTP::Cookies version 6.01
Perl Version 5.10.1
Operating System: Ubuntu 10.04

BUG:
In package HTTP::Cookies, the function add_cookie_header() has a bug 
that it copies prevoiusly existing cookies and concatenates them to the 
request header. This sometimes causes duplication of cookies.
The Bug causes error 400 Bad Request, due to the request header being 
too long when trying to log in to a server.This condition does not cause 
an error until and unless the cookies are too long.


Workaround:
Rather than appending the existing cookies to the new cookies we should 
check if the cookie already exists than skip appending it, otherwise 
append it to the new cookies.



My Scenario:
I was trying to log on to an AD-FS server using NTLM Authentication.
We recieve 4 NTLM Request Cookies which are base64 encoded. But during 
Authentication there are 3 redirections which are handled using 
request() in LWP::UserAgent.
This make a indirect call to HTTP::Cookies each time.
Therefore, the same cookies are repeated 3 times.
As the cookies are too long we recieve an HTTP Bad Request, Error 400.



Place of Bug in Code:

    if (@cval) {
        if (my $old = $request->header("Cookie")) {
            unshift(@cval, $old);
        }
        $request->header(Cookie => join("; ", @cval));
    }

Fix:
This patch checks whether the cookie in @oldcookie has alredy been 
included in @cval, if yes than it skips appending it to @cval,

if (@cval) {
	if (my $old = $request->header("Cookie")) {
		my @oldcookie = split(/;/, $old);
		my $cookieflag;
		my $ocookie;
		my $ncookie;
		foreach(@oldcookie){
			$ocookie = $_;
			$ocookie=~ s/^\s*//;
			$ocookie=~ s/\s*$//;
			chomp($ocookie);
			$cookieflag = 1;
			foreach(@cval){
 				$ncookie = $_;
				$ncookie=~ s/^\s*//;
				$ncookie=~ s/\s*$//;
				chomp($ncookie);
				if($ncookie eq $ocookie)
				{	
					$cookieflag=0;
				}
			}
			if($cookieflag==1)
			{    
			unshift(@cval, $ocookie);
			}
		}
	}
	$request->header(Cookie => join("; ", @cval));
    }

From olaf@wundersolutions.com on 2016-10-04 19:45:04:

On Tue Mar 20 02:24:48 2012, vaibhavkhunger wrote:
> 
> Bug in HTTP::Cookies version 6.01
> Perl Version 5.10.1
> Operating System: Ubuntu 10.04
> 
> BUG:
> In package HTTP::Cookies, the function add_cookie_header() has a bug 
> that it copies prevoiusly existing cookies and concatenates them to the 
> request header. This sometimes causes duplication of cookies.
> The Bug causes error 400 Bad Request, due to the request header being 
> too long when trying to log in to a server.This condition does not cause 
> an error until and unless the cookies are too long.
> 
> 
> Workaround:
> Rather than appending the existing cookies to the new cookies we should 
> check if the cookie already exists than skip appending it, otherwise 
> append it to the new cookies.
> 
> 
> 
> My Scenario:
> I was trying to log on to an AD-FS server using NTLM Authentication.
> We recieve 4 NTLM Request Cookies which are base64 encoded. But during 
> Authentication there are 3 redirections which are handled using 
> request() in LWP::UserAgent.
> This make a indirect call to HTTP::Cookies each time.
> Therefore, the same cookies are repeated 3 times.
> As the cookies are too long we recieve an HTTP Bad Request, Error 400.
> 
> 
> 
> Place of Bug in Code:
> 
>     if (@cval) {
>         if (my $old = $request->header("Cookie")) {
>             unshift(@cval, $old);
>         }
>         $request->header(Cookie => join("; ", @cval));
>     }
> 
> Fix:
> This patch checks whether the cookie in @oldcookie has alredy been 
> included in @cval, if yes than it skips appending it to @cval,
> 
> if (@cval) {
> 	if (my $old = $request->header("Cookie")) {
> 		my @oldcookie = split(/;/, $old);
> 		my $cookieflag;
> 		my $ocookie;
> 		my $ncookie;
> 		foreach(@oldcookie){
> 			$ocookie = $_;
> 			$ocookie=~ s/^\s*//;
> 			$ocookie=~ s/\s*$//;
> 			chomp($ocookie);
> 			$cookieflag = 1;
> 			foreach(@cval){
>  				$ncookie = $_;
> 				$ncookie=~ s/^\s*//;
> 				$ncookie=~ s/\s*$//;
> 				chomp($ncookie);
> 				if($ncookie eq $ocookie)
> 				{	
> 					$cookieflag=0;
> 				}
> 			}
> 			if($cookieflag==1)
> 			{    
> 			unshift(@cval, $ocookie);
> 			}
> 		}
> 	}
> 	$request->header(Cookie => join("; ", @cval));
>     }

Looks like this is still an issue.  See https://github.com/libwww-perl/WWW-Mechanize/issues/52

Start by inspecting add_cookie_header() in HTTP::Cookies and the existing handling of request Cookie headers. Reproduce the redirect scenario described with overlapping cookies, then add coverage showing that repeated cookies are not emitted and verify the resulting header.