[BUG] appsession should match the whole cookie name

I met a strange behaviour with appsession.

I firstly thought this was a regression due to one of my previous patch
but after testing with a 1.3.15.12 version, I also could reproduce it.

To illustrate, the configuration contains :
  appsession PHPSESSID len 32 timeout 1h

Then I call a short PHP script containing :
  setcookie("P", "should not match")

When calling this script thru haproxy, the cookie "P" matches the appsession rule :
Dumping hashtable 0x11f05c8
        table[1572]:    should+not+match

Shouldn't it be ignored ?
If you confirm, I'll send a patch for 1.3 and 1.4 branches to check that the
cookie length is equal to the appsession name length.

This is due to the comparison length, where the cookie length is took into
account instead of the appsession name length. Using the appsession name
length would allow ASPSESSIONIDXXX (+ check that memcmp won't go after the
buffer size).

Also, while testing, I noticed that HEAD requests where not available for
URIs containing the appsession parameter. 1.4.3 patch fixes an horrible
segfault I missed in a previous patch when appsession is not in the
configuration and HAProxy is compiled with DEBUG_HASH.

diff --git a/src/proto_http.c b/src/proto_http.c
index 6e8fa411f20e2646d048c5d4bae13be7a307c932..ce8448b052019996c8729287c736bbb2f22eb9fe 100644 (file)
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -5780,7 +5780,8 @@ void manage_client_side_cookies(struct session *t, struct buffer *req)
                                        }
 
                                        /* let's see if the cookie is our appcookie */
-                                       if (memcmp(p1, t->be->appsession_name, cmp_len) == 0) {
+                                       if ((cmp_len == t->be->appsession_name_len) &&
+                                           (memcmp(p1, t->be->appsession_name, t->be->appsession_name_len) == 0)) {
                                                /* Cool... it's the right one */
                                                manage_client_side_appsession(t, value_begin, value_len);
                                        }
@@ -6222,7 +6223,8 @@ void manage_server_side_cookies(struct session *t, struct buffer *rtr)
                                        value_len = MIN(t->be->appsession_len, p4 - p3);
                                }
 
-                               if (memcmp(p1, t->be->appsession_name, cmp_len) == 0) {
+                               if ((cmp_len == t->be->appsession_name_len) &&
+                                   (memcmp(p1, t->be->appsession_name, t->be->appsession_name_len) == 0)) {
                                        /* Cool... it's the right one */
                                        if (txn->sessid != NULL) {
                                                /* free previously allocated memory as we don't need it anymore */
@@ -6283,8 +6285,10 @@ void manage_server_side_cookies(struct session *t, struct buffer *rtr)
        }
 
 #if defined(DEBUG_HASH)
-       Alert("manage_server_side_cookies\n");
-       appsession_hash_dump(&(t->be->htbl_proxy));
+       if (t->be->appsession_name) {
+               Alert("manage_server_side_cookies\n");
+               appsession_hash_dump(&(t->be->htbl_proxy));
+       }
 #endif
 }
 
@@ -6390,7 +6394,7 @@ void get_srv_from_appsession(struct session *t, const char *begin, int len)
        int mode = t->be->options2 & PR_O2_AS_M_ANY;
 
        if (t->be->appsession_name == NULL ||
-           (t->txn.meth != HTTP_METH_GET && t->txn.meth != HTTP_METH_POST)) {
+           (t->txn.meth != HTTP_METH_GET && t->txn.meth != HTTP_METH_POST && t->txn.meth != HTTP_METH_HEAD)) {
                return;
        }