Hello,
as discussed previously with Willy Tarreau, I'd like to propose a patch for the 
appsession code.
This patch has 2 goals :

1. I wanted to test the appsession feature with a small PHP code, using 
PHPSESSID.
The problem is that when PHP gets an unknown session id, it creates a new one 
with this ID.
So, when sending an unknown session to PHP, persistance is broken : haproxy 
won't see any new cookie in the response and will never attach this session to 
a specific server.
This also apen when you restart haproxy : the internal hash becomes empty and 
all sessions loose their persistance (load balancing the requests on all 
backend servers, creating a new session on each one).
For a user, it's like the service is unusable.

The patch modifies the code to make haproxy also learn the persistance from the 
client :
if no session is sent from the server, then the session id found in the client 
part (using the URI or the client cookie) is used to associated the server that 
gave the response.

As it's probably not a feature usable in all cases, I added an option to enable 
it (by default it's disabled).
The syntax of appsession becomes :
appsession <cookie> len <length> timeout <holdtime> [request-learn]

This helps haproxy repair the persistance (with the risk of losing its session 
at the next request, as the user will probably not be load balanced to the same 
server the first time).

2. This patch also tries to reduce the memory usage.
Here is a little example to explain the current behaviour :
- Take a Tomcat server where /session.jsp is valid.
- Send a request using a cookie with an unknown value AND a path parameter with 
another unknown value :
curl -b "JSESSIONID=12345678901234567890123456789012" 
http://<haproxy>/session.jsp;jsessionid=00000000000000000000000000000001
(I know, it's unexpected to have a request like that on a live service)
Here, haproxy finds the URI session ID and stores it in its internal hash (with 
no server associated)
But it also finds the cookie session ID and stores it again.
- As a result, session.jsp sends a new session ID also stored in the internal 
hash, with a server associated.

=> For 1 request, haproxy has stored 3 entries, with only 1 which will be usable

The patch modifies the behaviour to store only 1 entry (maximum).

In attachment, a patch file for haproxy-1.4-dev4.

I hope this helps.

--
Cyril Bonté
diff -Naur haproxy-1.4-dev4/include/proto/proto_http.h haproxy-1.4-dev4-appsession/include/proto/proto_http.h
--- haproxy-1.4-dev4/include/proto/proto_http.h	2009-10-12 06:40:53.000000000 +0200
+++ haproxy-1.4-dev4-appsession/include/proto/proto_http.h	2009-10-12 21:23:38.000000000 +0200
@@ -74,6 +74,7 @@
 int apply_filter_to_req_line(struct session *t, struct buffer *req, struct hdr_exp *exp);
 int apply_filters_to_request(struct session *t, struct buffer *req, struct hdr_exp *exp);
 int apply_filters_to_response(struct session *t, struct buffer *rtr, struct hdr_exp *exp);
+void manage_client_side_appsession(struct session *t, char *buf);
 void manage_client_side_cookies(struct session *t, struct buffer *req);
 void manage_server_side_cookies(struct session *t, struct buffer *rtr);
 void check_response_for_cacheability(struct session *t, struct buffer *rtr);
diff -Naur haproxy-1.4-dev4/include/types/proxy.h haproxy-1.4-dev4-appsession/include/types/proxy.h
--- haproxy-1.4-dev4/include/types/proxy.h	2009-10-12 06:40:53.000000000 +0200
+++ haproxy-1.4-dev4-appsession/include/types/proxy.h	2009-10-12 21:24:42.000000000 +0200
@@ -124,6 +124,9 @@
 #define PR_O2_INDEPSTR	0x00001000	/* independant streams, don't update rex on write */
 #define PR_O2_SOCKSTAT	0x00002000	/* collect & provide separate statistics for sockets */
 
+/* bits for proxy->appsession_options */
+#define PR_O_AS_REQL	0x00000001      /* learn the session id from the request */
+
 struct error_snapshot {
 	struct timeval when;		/* date of this event, (tv_sec == 0) means "never" */
 	unsigned int len;		/* original length of the last invalid request/response */
@@ -177,6 +180,7 @@
 	char *appsession_name;			/* name of the cookie to look for */
 	int  appsession_name_len;		/* strlen(appsession_name), computed only once */
 	int  appsession_len;			/* length of the appsession cookie value to be used */
+	int  appsession_options;		/* options for appsession */
 	struct appsession_hash htbl_proxy;	/* Per Proxy hashtable */
 	char *capture_name;			/* beginning of the name of the cookie to capture */
 	int  capture_namelen;			/* length of the cookie name to match */
diff -Naur haproxy-1.4-dev4/include/types/session.h haproxy-1.4-dev4-appsession/include/types/session.h
--- haproxy-1.4-dev4/include/types/session.h	2009-10-12 06:40:53.000000000 +0200
+++ haproxy-1.4-dev4-appsession/include/types/session.h	2009-10-12 21:25:15.000000000 +0200
@@ -162,6 +162,7 @@
 	int conn_retries;			/* number of connect retries left */
 	int flags;				/* some flags describing the session */
 	unsigned term_trace;			/* term trace: 4*8 bits indicating which part of the code closed */
+	char *sessid;			/* the session id, if found in the request or in the response */
 	struct buffer *req;			/* request buffer */
 	struct buffer *rep;			/* response buffer */
 	struct stream_interface si[2];          /* client and server stream interfaces */
diff -Naur haproxy-1.4-dev4/src/cfgparse.c haproxy-1.4-dev4-appsession/src/cfgparse.c
--- haproxy-1.4-dev4/src/cfgparse.c	2009-10-12 06:40:53.000000000 +0200
+++ haproxy-1.4-dev4-appsession/src/cfgparse.c	2009-10-12 21:26:10.000000000 +0200
@@ -1517,6 +1517,7 @@
 		}
 	}
 	else if (!strcmp(args[0], "appsession")) {  /* cookie name */
+		int cur_arg;
 
 		if (warnifnotcap(curproxy, PR_CAP_BE, file, linenum, args[0], NULL))
 			err_code |= ERR_WARN;
@@ -1546,6 +1547,14 @@
 			err_code |= ERR_ALERT | ERR_ABORT;
 			goto out;
 		}
+
+		cur_arg = 6;
+		while (*(args[cur_arg])) {
+			if (!strcmp(args[cur_arg], "request-learn")) {
+				curproxy->appsession_options |= PR_O_AS_REQL;
+			}
+			cur_arg++;
+		}
 	} /* Url App Session */
 	else if (!strcmp(args[0], "capture")) {
 		if (warnifnotcap(curproxy, PR_CAP_FE, file, linenum, args[0], NULL))
diff -Naur haproxy-1.4-dev4/src/client.c haproxy-1.4-dev4-appsession/src/client.c
--- haproxy-1.4-dev4/src/client.c	2009-10-12 06:40:53.000000000 +0200
+++ haproxy-1.4-dev4-appsession/src/client.c	2009-10-12 21:26:38.000000000 +0200
@@ -184,7 +184,7 @@
 		 */
 		s->be = s->fe = p;
 
-		s->req = s->rep = NULL; /* will be allocated later */
+		s->sessid = s->req = s->rep = NULL; /* will be allocated later */
 
 		s->si[0].state = s->si[0].prev_state = SI_ST_EST;
 		s->si[0].err_type = SI_ET_NONE;
diff -Naur haproxy-1.4-dev4/src/proto_http.c haproxy-1.4-dev4-appsession/src/proto_http.c
--- haproxy-1.4-dev4/src/proto_http.c	2009-10-12 06:40:53.000000000 +0200
+++ haproxy-1.4-dev4-appsession/src/proto_http.c	2009-10-12 21:56:38.000000000 +0200
@@ -3557,6 +3557,71 @@
 
 
 /*
+ * Try to retrieve the server associated to the appsession.
+ * If the server is found, it's assigned to the session.
+ */
+void manage_client_side_appsession(struct session *t, char *buf) {
+	struct http_txn *txn = &t->txn;
+	appsess *asession = NULL;
+	char *sessid_temp = NULL;
+
+	if (t->be->appsession_options & PR_O_AS_REQL) {
+		/* request-learn option is enabled : store the sessid in the session for future use */
+		if (t->sessid != NULL) {
+			/* free previously allocated memory as we don't need the session id found in the URL anymore */
+			pool_free2(apools.sessid, t->sessid);
+		}
+
+		if ((t->sessid = pool_alloc2(apools.sessid)) == NULL) {
+			Alert("Not enough memory process_cli():asession->sessid:malloc().\n");
+			send_log(t->be, LOG_ALERT, "Not enough memory process_cli():asession->sessid:malloc().\n");
+			return;
+		}
+
+		memcpy(t->sessid, buf, t->be->appsession_len);
+		t->sessid[t->be->appsession_len] = 0;
+	}
+
+	if ((sessid_temp = pool_alloc2(apools.sessid)) == NULL) {
+		Alert("Not enough memory process_cli():asession->sessid:malloc().\n");
+		send_log(t->be, LOG_ALERT, "Not enough memory process_cli():asession->sessid:malloc().\n");
+		return;
+	}
+
+	memcpy(sessid_temp, buf, t->be->appsession_len);
+	sessid_temp[t->be->appsession_len] = 0;
+
+	asession = appsession_hash_lookup(&(t->be->htbl_proxy), sessid_temp);
+	/* free previously allocated memory */
+	pool_free2(apools.sessid, sessid_temp);
+	
+	if (asession != NULL) {
+		asession->expire = tick_add_ifset(now_ms, t->be->timeout.appsession);
+		/* asession->request_count will be incremented in the server part */
+
+		if (asession->serverid != NULL) {
+			struct server *srv = t->be->srv;
+			while (srv) {
+				if (strcmp(srv->id, asession->serverid) == 0) {
+					if (srv->state & SRV_RUNNING || t->be->options & PR_O_PERSIST) {
+						/* we found the server and it's usable */
+						txn->flags &= ~TX_CK_MASK;
+						txn->flags |= TX_CK_VALID;
+						t->flags |= SN_DIRECT | SN_ASSIGNED;
+						t->srv = srv;
+						break;
+					} else {
+						txn->flags &= ~TX_CK_MASK;
+						txn->flags |= TX_CK_DOWN;
+					}
+				}
+				srv = srv->next;
+			}
+		}
+	}
+}
+
+/*
  * Manage client-side cookie. It can impact performance by about 2% so it is
  * desirable to call it only when needed.
  */
@@ -3567,9 +3632,6 @@
 	char *del_colon, *del_cookie, *colon;
 	int app_cookies;
 
-	appsess *asession_temp = NULL;
-	appsess local_asession;
-
 	char *cur_ptr, *cur_end, *cur_next;
 	int cur_idx, old_idx;
 
@@ -3799,67 +3861,7 @@
 					/* first, let's see if the cookie is our appcookie*/
 
 					/* Cool... it's the right one */
-
-					asession_temp = &local_asession;
-			  
-					if ((asession_temp->sessid = pool_alloc2(apools.sessid)) == NULL) {
-						Alert("Not enough memory process_cli():asession->sessid:malloc().\n");
-						send_log(t->be, LOG_ALERT, "Not enough memory process_cli():asession->sessid:malloc().\n");
-						return;
-					}
-
-					memcpy(asession_temp->sessid, p3, t->be->appsession_len);
-					asession_temp->sessid[t->be->appsession_len] = 0;
-					asession_temp->serverid = NULL;
-
-					/* only do insert, if lookup fails */
-					asession_temp = appsession_hash_lookup(&(t->be->htbl_proxy), asession_temp->sessid);
-					if (asession_temp == NULL) {
-						if ((asession_temp = pool_alloc2(pool2_appsess)) == NULL) {
-							/* free previously allocated memory */
-							pool_free2(apools.sessid, local_asession.sessid);
-							Alert("Not enough memory process_cli():asession:calloc().\n");
-							send_log(t->be, LOG_ALERT, "Not enough memory process_cli():asession:calloc().\n");
-							return;
-						}
-
-						asession_temp->sessid = local_asession.sessid;
-						asession_temp->serverid = local_asession.serverid;
-						asession_temp->request_count = 0;
-						appsession_hash_insert(&(t->be->htbl_proxy), asession_temp);
-					} else {
-						/* free previously allocated memory */
-						pool_free2(apools.sessid, local_asession.sessid);
-					}
-					if (asession_temp->serverid == NULL) {
-						/* TODO redispatch request */
-						Alert("Found Application Session without matching server.\n");
-					} else {
-						struct server *srv = t->be->srv;
-						while (srv) {
-							if (strcmp(srv->id, asession_temp->serverid) == 0) {
-								if (srv->state & SRV_RUNNING || t->be->options & PR_O_PERSIST) {
-									/* we found the server and it's usable */
-									txn->flags &= ~TX_CK_MASK;
-									txn->flags |= TX_CK_VALID;
-									t->flags |= SN_DIRECT | SN_ASSIGNED;
-									t->srv = srv;
-									break;
-								} else {
-									txn->flags &= ~TX_CK_MASK;
-									txn->flags |= TX_CK_DOWN;
-								}
-							}
-							srv = srv->next;
-						}/* end while(srv) */
-					}/* end else if server == NULL */
-
-					asession_temp->expire = tick_add_ifset(now_ms, t->be->timeout.appsession);
-					asession_temp->request_count++;
-#if defined(DEBUG_HASH)
-					Alert("manage_client_side_cookies\n");
-					appsession_hash_dump(&(t->be->htbl_proxy));
-#endif
+					manage_client_side_appsession(t, p3);
 				}/* end if ((t->proxy->appsession_name != NULL) ... */
 			}
 
@@ -4133,9 +4135,6 @@
 	struct http_txn *txn = &t->txn;
 	char *p1, *p2, *p3, *p4;
 
-	appsess *asession_temp = NULL;
-	appsess local_asession;
-
 	char *cur_ptr, *cur_end, *cur_next;
 	int cur_idx, old_idx, delta;
 
@@ -4276,60 +4275,64 @@
 
 				/* Cool... it's the right one */
 
-				size_t server_id_len = strlen(t->srv->id) + 1;
-				asession_temp = &local_asession;
-		      
-				if ((asession_temp->sessid = pool_alloc2(apools.sessid)) == NULL) {
+				if (t->sessid != NULL) {
+					/* free previously allocated memory as we don't need it anymore */
+					pool_free2(apools.sessid, t->sessid);
+				}
+				/* Store the sessid in the session for future use */
+				if ((t->sessid = pool_alloc2(apools.sessid)) == NULL) {
 					Alert("Not enough Memory process_srv():asession->sessid:malloc().\n");
 					send_log(t->be, LOG_ALERT, "Not enough Memory process_srv():asession->sessid:malloc().\n");
 					return;
 				}
-				memcpy(asession_temp->sessid, p3, t->be->appsession_len);
-				asession_temp->sessid[t->be->appsession_len] = 0;
-				asession_temp->serverid = NULL;
-
-				/* only do insert, if lookup fails */
-				asession_temp = appsession_hash_lookup(&(t->be->htbl_proxy), asession_temp->sessid);
-				if (asession_temp == NULL) {
-					if ((asession_temp = pool_alloc2(pool2_appsess)) == NULL) {
-						Alert("Not enough Memory process_srv():asession:calloc().\n");
-						send_log(t->be, LOG_ALERT, "Not enough Memory process_srv():asession:calloc().\n");
-						return;
-					}
-					asession_temp->sessid = local_asession.sessid;
-					asession_temp->serverid = local_asession.serverid;
-					asession_temp->request_count = 0;
-					appsession_hash_insert(&(t->be->htbl_proxy), asession_temp);
-				} else {
-					/* free wasted memory */
-					pool_free2(apools.sessid, local_asession.sessid);
-				}
-
-				if (asession_temp->serverid == NULL) {
-					if ((asession_temp->serverid = pool_alloc2(apools.serverid)) == NULL) {
-						Alert("Not enough Memory process_srv():asession->sessid:malloc().\n");
-						send_log(t->be, LOG_ALERT, "Not enough Memory process_srv():asession->sessid:malloc().\n");
-						return;
-					}
-					asession_temp->serverid[0] = '\0';
-				}
-		      
-				if (asession_temp->serverid[0] == '\0')
-					memcpy(asession_temp->serverid, t->srv->id, server_id_len);
-		      
-				asession_temp->expire = tick_add_ifset(now_ms, t->be->timeout.appsession);
-				asession_temp->request_count++;
-#if defined(DEBUG_HASH)
-				Alert("manage_server_side_cookies\n");
-				appsession_hash_dump(&(t->be->htbl_proxy));
-#endif
+				memcpy(t->sessid, p3, t->be->appsession_len);
+				t->sessid[t->be->appsession_len] = 0;
 			}/* end if ((t->proxy->appsession_name != NULL) ... */
 			break; /* we don't want to loop again since there cannot be another cookie on the same line */
 		} /* we're now at the end of the cookie value */
-
 		/* keep the link from this header to next one */
 		old_idx = cur_idx;
 	} /* end of cookie processing on this header */
+	
+	if (t->sessid != NULL) {
+		appsess *asession = NULL;
+		/* only do insert, if lookup fails */
+		asession = appsession_hash_lookup(&(t->be->htbl_proxy), t->sessid);
+		if (asession == NULL) {
+			if ((asession = pool_alloc2(pool2_appsess)) == NULL) {
+				Alert("Not enough Memory process_srv():asession:calloc().\n");
+				send_log(t->be, LOG_ALERT, "Not enough Memory process_srv():asession:calloc().\n");
+				return;
+			}
+			if ((asession->sessid = pool_alloc2(apools.sessid)) == NULL) {
+				Alert("Not enough Memory process_srv():asession->sessid:malloc().\n");
+				send_log(t->be, LOG_ALERT, "Not enough Memory process_srv():asession->sessid:malloc().\n");
+				return;
+			}
+			memcpy(asession->sessid, t->sessid, t->be->appsession_len);
+			asession->sessid[t->be->appsession_len] = 0;
+
+			size_t server_id_len = strlen(t->srv->id) + 1;
+			if ((asession->serverid = pool_alloc2(apools.serverid)) == NULL) {
+				Alert("Not enough Memory process_srv():asession->sessid:malloc().\n");
+				send_log(t->be, LOG_ALERT, "Not enough Memory process_srv():asession->sessid:malloc().\n");
+				return;
+			}
+			asession->serverid[0] = '\0';
+			memcpy(asession->serverid, t->srv->id, server_id_len);
+			
+			asession->request_count = 0;
+			appsession_hash_insert(&(t->be->htbl_proxy), asession);
+		}
+      
+		asession->expire = tick_add_ifset(now_ms, t->be->timeout.appsession);
+		asession->request_count++;
+	}
+		      
+#if defined(DEBUG_HASH)
+	Alert("manage_server_side_cookies\n");
+	appsession_hash_dump(&(t->be->htbl_proxy));
+#endif
 }
 
 
@@ -4428,8 +4431,6 @@
 void get_srv_from_appsession(struct session *t, const char *begin, int len)
 {
 	struct http_txn *txn = &t->txn;
-	appsess *asession_temp = NULL;
-	appsess local_asession;
 	char *request_line;
 
 	if (t->be->appsession_name == NULL ||
@@ -4448,69 +4449,7 @@
 	/* skip jsessionid= */
 	request_line += t->be->appsession_name_len + 1;
 	
-	/* First try if we already have an appsession */
-	asession_temp = &local_asession;
-	
-	if ((asession_temp->sessid = pool_alloc2(apools.sessid)) == NULL) {
-		Alert("Not enough memory process_cli():asession_temp->sessid:calloc().\n");
-		send_log(t->be, LOG_ALERT, "Not enough Memory process_cli():asession_temp->sessid:calloc().\n");
-		return;
-	}
-	
-	/* Copy the sessionid */
-	memcpy(asession_temp->sessid, request_line, t->be->appsession_len);
-	asession_temp->sessid[t->be->appsession_len] = 0;
-	asession_temp->serverid = NULL;
-	
-	/* only do insert, if lookup fails */
-	asession_temp = appsession_hash_lookup(&(t->be->htbl_proxy), asession_temp->sessid);
-	if (asession_temp == NULL) {
-		if ((asession_temp = pool_alloc2(pool2_appsess)) == NULL) {
-			/* free previously allocated memory */
-			pool_free2(apools.sessid, local_asession.sessid);
-			Alert("Not enough memory process_cli():asession:calloc().\n");
-			send_log(t->be, LOG_ALERT, "Not enough memory process_cli():asession:calloc().\n");
-			return;
-		}
-		asession_temp->sessid = local_asession.sessid;
-		asession_temp->serverid = local_asession.serverid;
-		asession_temp->request_count=0;
-		appsession_hash_insert(&(t->be->htbl_proxy), asession_temp);
-	}
-	else {
-		/* free previously allocated memory */
-		pool_free2(apools.sessid, local_asession.sessid);
-	}
-
-	asession_temp->expire = tick_add_ifset(now_ms, t->be->timeout.appsession);
-	asession_temp->request_count++;
-
-#if defined(DEBUG_HASH)
-	Alert("get_srv_from_appsession\n");
-	appsession_hash_dump(&(t->be->htbl_proxy));
-#endif
-	if (asession_temp->serverid == NULL) {
-		/* TODO redispatch request */
-		Alert("Found Application Session without matching server.\n");
-	} else {
-		struct server *srv = t->be->srv;
-		while (srv) {
-			if (strcmp(srv->id, asession_temp->serverid) == 0) {
-				if (srv->state & SRV_RUNNING || t->be->options & PR_O_PERSIST) {
-					/* we found the server and it's usable */
-					txn->flags &= ~TX_CK_MASK;
-					txn->flags |= TX_CK_VALID;
-					t->flags |= SN_DIRECT | SN_ASSIGNED;
-					t->srv = srv;
-					break;
-				} else {
-					txn->flags &= ~TX_CK_MASK;
-					txn->flags |= TX_CK_DOWN;
-				}
-			}
-			srv = srv->next;
-		}
-	}
+	manage_client_side_appsession(t, request_line);
 }
 
 
diff -Naur haproxy-1.4-dev4/src/session.c haproxy-1.4-dev4-appsession/src/session.c
--- haproxy-1.4-dev4/src/session.c	2009-10-12 06:40:53.000000000 +0200
+++ haproxy-1.4-dev4-appsession/src/session.c	2009-10-12 21:41:23.000000000 +0200
@@ -77,6 +77,9 @@
 	pool_free2(pool2_buffer, s->req);
 	pool_free2(pool2_buffer, s->rep);
 
+	if (s->sessid)
+		pool_free2(apools.sessid, s->sessid);
+
 	if (fe) {
 		pool_free2(fe->hdr_idx_pool, txn->hdr_idx.v);
 

Reply via email to