[Bug 60910] New: Do not send Set-Cookie twice

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug 60910] New: Do not send Set-Cookie twice

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60910

            Bug ID: 60910
           Summary: Do not send Set-Cookie twice
           Product: Apache httpd-2
           Version: 2.5-HEAD
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: mod_session_cookie
          Assignee: [hidden email]
          Reporter: [hidden email]
  Target Milestone: ---

Created attachment 34874
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34874&action=edit
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.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60910] Do not send Set-Cookie twice

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60910

[hidden email] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |PatchAvailable

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60910] Do not send Set-Cookie twice

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60910

[hidden email] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34874|0                           |1
           is patch|                            |

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60910] Do not send Set-Cookie twice

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60910

--- Comment #1 from Luca Toscano <[hidden email]> ---
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..

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60910] Do not send Set-Cookie twice

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60910

Christophe JAILLET <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |DUPLICATE
             Status|NEW                         |RESOLVED

--- Comment #2 from Christophe JAILLET <[hidden email]> ---


*** This bug has been marked as a duplicate of bug 56098 ***

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60910] Do not send Set-Cookie twice

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60910

Luca Toscano <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|DUPLICATE                   |---
             Status|RESOLVED                    |REOPENED

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60910] Do not send Set-Cookie twice

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60910

--- Comment #3 from Luca Toscano <[hidden email]> ---
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!

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60910] Do not send Set-Cookie twice

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60910

Luca Toscano <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|PatchAvailable              |FixedInTrunk

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60910] Do not send Set-Cookie twice

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60910

Luca Toscano <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--- Comment #4 from Luca Toscano <[hidden email]> ---
*** Bug 56098 has been marked as a duplicate of this bug. ***

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60910] Do not send Set-Cookie twice

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60910

Graham Leggett <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|REOPENED                    |RESOLVED

--- Comment #5 from Graham Leggett <[hidden email]> ---
Backport to v2.4.38.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60910] Do not send Set-Cookie twice

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60910

Lubos Uhliarik <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

--- Comment #6 from Lubos Uhliarik <[hidden email]> ---
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

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60910] Do not send Set-Cookie twice

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60910

--- Comment #7 from Lubos Uhliarik <[hidden email]> ---
Created attachment 36883
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=36883&action=edit
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

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60910] Do not send Set-Cookie twice

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60910

--- Comment #8 from Lubos Uhliarik <[hidden email]> ---
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);
        }
    }

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 60910] Do not send Set-Cookie twice

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=60910

--- Comment #9 from Joe Orton <[hidden email]> ---
Fixed in r1869785

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]