Bug 60910

Summary: Do not send Set-Cookie twice
Product: Apache httpd-2 Reporter: manu
Component: mod_session_cookieAssignee: Apache HTTPD Bugs Mailing List <bugs>
Status: REOPENED ---    
Severity: normal CC: erik
Priority: P2 Keywords: FixedInTrunk
Version: 2.5-HEAD   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: Do not send Set-Cookie twice
Patch fixing the bug

Description manu 2017-03-24 04:50:03 UTC
Created attachment 34874 [details]
Do not send Set-Cookie twice

mod_session_cookie uses ap_cookie_write() with r->headers_out and r->err_headers_out. The former causes a Set-Cookie header to be added on successful requests, while the later causes a Set-Cookie header to always be added.

As a result, successful requests get a duplicated Set-Cookie header and it confuses some clients. The attached patch fixes that by using only r->err_headers_out, which as its name does not suggests, causes Set-Cookie to be always added, regardless of error status.
Comment 1 Luca Toscano 2017-11-27 11:53:10 UTC
Hi! 

Thanks a lot for the code patch. I am not super expert so this question might be obvious: is Set-Cookie supposed to be added always (even on error/redirects)? At a first glance I'd add the header only to r->headers_out..
Comment 2 Christophe JAILLET 2018-08-03 05:10:58 UTC

*** This bug has been marked as a duplicate of bug 56098 ***
Comment 3 Luca Toscano 2018-10-10 05:25:37 UTC
Committed part of the patch sent in http://svn.apache.org/r1843244 (trunk). The rationale for the 'partial' is to leave the cookie removal as it is and to only avoid to add the cookie twice via err_headers_out/headers_out.

Going to write some tests and then finally propose for backport if everything looks good. Thanks for the patch and sorry for the lag!
Comment 4 Luca Toscano 2018-10-10 05:26:55 UTC
*** Bug 56098 has been marked as a duplicate of this bug. ***
Comment 5 Graham Leggett 2018-11-23 15:35:52 UTC
Backport to v2.4.38.
Comment 6 Lubos Uhliarik 2019-10-24 14:48:10 UTC
I'm experiencing weird behavior. With the following configuration file:

# cat /etc/httpd/conf.d/session.conf 
Session On
SessionCookieName test_session path=/

I'm still getting answer with 2 Set-Cookie headers:

# curl -v localhost/hello.html
*   Trying ::1:80...
* TCP_NODELAY set
* Connected to localhost (::1) port 80 (#0)
> GET /hello.html HTTP/1.1
> Host: localhost
> User-Agent: curl/7.65.3
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Date: Thu, 24 Oct 2019 14:44:35 GMT
< Server: Apache/2.4.39 (Fedora)
< Set-Cookie: test_session=;Max-Age=0;path=/
< Last-Modified: Thu, 24 Oct 2019 12:24:50 GMT
< ETag: "6-595a71eb1ffdc"
< Accept-Ranges: bytes
< Content-Length: 6
< Cache-Control: no-cache
< Set-Cookie: test_session=;Max-Age=0;path=/
< Content-Type: text/html; charset=UTF-8
< 
HELLO
* Connection #0 to host localhost left intact

Version of httpd apache is 2.4.39 in this case. But the fix mentioned here should be already present in this version (per comment 5, it should be fixed in version 2.4.38). Anyway, I also tried it with the newest version (2.4.41), but I got no Set-Cookie header at all:

# curl -v localhost/hello.html
*   Trying ::1:80...
* TCP_NODELAY set
* Connected to localhost (::1) port 80 (#0)
> GET /hello.html HTTP/1.1
> Host: localhost
> User-Agent: curl/7.65.3
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Date: Thu, 24 Oct 2019 14:39:21 GMT
< Server: Apache/2.4.41 (Fedora)
< Cache-Control: no-cache, private
< Last-Modified: Thu, 24 Oct 2019 12:24:50 GMT
< ETag: "6-595a71eb1ffdc"
< Accept-Ranges: bytes
< Content-Length: 6
< Content-Type: text/html; charset=UTF-8
< 
HELLO
* Connection #0 to host localhost left intact
Comment 7 Lubos Uhliarik 2019-11-13 11:54:09 UTC
Created attachment 36883 [details]
Patch fixing the bug

Fix duplication also in case of ap_cookie_remove* call. 

Conf:
Session On
SessionCookieName test_session path=/


Curl output before patch:
# curl -v localhost/hello.html           
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 80 (#0)
> GET /hello.html HTTP/1.1
> Host: localhost
> User-Agent: curl/7.61.1
> Accept: */*
> 
< HTTP/1.1 200 OK
< Date: Tue, 12 Nov 2019 18:17:33 GMT
< Server: Apache/2.4.34 (Red Hat) OpenSSL/1.0.2k-fips mod_auth_kerb/5.4
< Set-Cookie: test_session=;Max-Age=0;path=/
< Last-Modified: Tue, 12 Nov 2019 17:01:53 GMT
< ETag: "6-597293478963d"
< Accept-Ranges: bytes
< Content-Length: 6
< Cache-Control: no-cache
< Set-Cookie: test_session=;Max-Age=0;path=/
< Content-Type: text/html; charset=UTF-8
< 
HELLO
* Connection #0 to host localhost left intact




Curl output after patch:
# curl -v localhost/hello.html
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 80 (#0)
> GET /hello.html HTTP/1.1
> Host: localhost
> User-Agent: curl/7.61.1
> Accept: */*
> 
< HTTP/1.1 200 OK
< Date: Wed, 13 Nov 2019 11:45:50 GMT
< Server: Apache/2.4.34 (Red Hat) OpenSSL/1.0.2k-fips mod_auth_kerb/5.4
< Set-Cookie: test_session=;Max-Age=0;path=/
< Last-Modified: Tue, 12 Nov 2019 17:01:53 GMT
< ETag: "6-597293478963d"
< Accept-Ranges: bytes
< Content-Length: 6
< Cache-Control: no-cache
< Content-Type: text/html; charset=UTF-8
< 
HELLO
* Connection #0 to host localhost left intact
Comment 8 Lubos Uhliarik 2019-11-13 12:38:51 UTC
Also found out, why http://svn.apache.org/repos/asf/httpd/test/framework/trunk/t/modules/session_cookie.t is passing:

in your testconfig, there is "SessionMaxAge 1" directive, which adds "expiry" keypair to Set-Cookie header field, therefore ap_cookie_write is triggered instead of ap_cookie_remove (z->encoded is not null). 

From mod_session_cookie.c:

    /* create RFC2109 compliant cookie */
    if (conf->name_set) {
        if (z->encoded && z->encoded[0]) {
            ap_cookie_write(r, conf->name, z->encoded, conf->name_attrs,
                            z->maxage, r->err_headers_out,
                            NULL);
        }
        else {
            ap_cookie_remove(r, conf->name, conf->name_attrs,
                             r->err_headers_out, NULL);
        }
    }
Comment 9 Joe Orton 2019-11-19 15:46:46 UTC
Fixed in r1869785