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.
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..
*** This bug has been marked as a duplicate of bug 56098 ***
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!
*** Bug 56098 has been marked as a duplicate of this bug. ***
Backport to v2.4.38.
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
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
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); } }
Fixed in r1869785