Re: Small patch for the appsession feature
Hi Cyril, On Mon, Nov 30, 2009 at 12:53:34AM +0100, Cyril Bonté wrote: Could you please check on your side and confirm/infirm my doubts ? Basically I want to ensure we never dereference the buffer past its end, so begin+len bust always be below the buffer size. If you think a control is missing, we can merge it as a separate patch because it's already missing in current code then. Previously, the function was called with : get_srv_from_appsession(s, req-data[msg-som], msg-sl.rq.l); I assume that msg-sl.rq variables are already correctly calculated and that we can replace this call with : get_srv_from_appsession(s, req-data[msg-som + msg-sl.rq.u], msg-sl.rq.u_l); This allows to parse only the URL, skipping the http method and the protocol. I believe this should always be contained in the buffer size. The parser will then restrict to this area when it will extract the session value (It was missing in the previous code). Did I understand what you wanted to know ? Yes, that's fine then, thanks for the check. I'm just waiting for your response on this possible issue and I'm OK to merge it. Please tell me if you'd prefer to resend a different patch with the cosmetic changes or if I can do them myself. While you're at it, I noticed a mis-indented if statement alone in the remaining part. Also something I can fix if needed. It would be great if you can do this changes (to prevent several patch versions in case I still leave some mis-indented code or misplaced comments in the options constants). No problem, I will do that then. Anyway I would not ask you to resubmit for such minor things, but some people prefer to have the exact same version on their side as the one being committed, reason why I asked. Thanks! Willy
Re: Small patch for the appsession feature
On Tue, Nov 17, 2009 at 11:25:56PM +0100, Aleksandar Lazic wrote: And for now, I see 2 modes : - path-parameters (the default) - query-string (the one that made me look at the code :-) ) (and if needed weblogic) Looks ok for me. What I'am not sure now is how we can prevent that notsuccessful searches on long URLs have negative performance impact? I think that if it causes some trouble, we can optionally limit the position of the argument in the query-string. But right now I don't think it is much of a problem, because : 1) we don't check the query string when we find the cookie in the headers (at least I believe :-)) 2) when using the query-string mode, we're sure that almost all requests will have the string. 3) we're just parsing a long string once per request, there are other processing occuring on headers, cookies, etc... which are at least as expensive and which do not noticeably hurt performance. So all in all, it's not really a problem in my opinion. Regards, Willy
Re: Small patch for the appsession feature
On Mon 16.11.2009 08:52, Willy Tarreau wrote: Hi, On Sun, Nov 15, 2009 at 10:28:21PM +0100, Aleksandar Lazic wrote: Hi Cyril, On Fre 13.11.2009 22:50, Cyril Bonté wrote: Hello Willy, [snipp] First I added it as I did for request-learn but shouldn't it be better to define these options more haproxy like (using option and no option) ? Which would give : (no) option appsession-request-learn (no) option appsession-capture-cookie +1 [snipp] What you can do however is to create a new prefix keyword like we have for timeout or tcp-request and put the flags somewhere else. appsession would have been fine but it's already used. Maybe you can use appcookie ? Something like this : appcookie request-learn appcookie capture Both are ok for me. My point of view is to be able to explain and handle it straight forward. For me both looks ok, mybe we use appsession-option or app-op (= for abbreviation ;-) ) Due to the fact that not only the cookie is relevant for the application session. How about to add also a app* delimiterstart (default ;) app* delimiterstop (defautl =) in combination with the length you get the maximum flexibility for this module ;-) BR Aleks
Re: Small patch for the appsession feature
On Mon, Nov 16, 2009 at 10:08:56AM +0100, Aleksandar Lazic wrote: What you can do however is to create a new prefix keyword like we have for timeout or tcp-request and put the flags somewhere else. appsession would have been fine but it's already used. Maybe you can use appcookie ? Something like this : appcookie request-learn appcookie capture Both are ok for me. My point of view is to be able to explain and handle it straight forward. For me both looks ok, mybe we use appsession-option or app-op (= for abbreviation ;-) ) Due to the fact that not only the cookie is relevant for the application session. OK but then I'd prefer the long form. We've had too many trouble with abbreviations that nobody (including me) is able to spell correctly. How about to add also a app* delimiterstart (default ;) app* delimiterstop (defautl =) I'm sorry, I don't understand what you mean. Could you give an example of configuration using this ? Regards, Willy
Re: Small patch for the appsession feature
On Mon, Nov 16, 2009 at 01:04:17PM +0100, Aleksandar Lazic wrote: (...) app* delimiterstart (default ;) app* delimiterstop (defautl =) I'm sorry, I don't understand what you mean. Could you give an example of configuration using this ? For example: if in the url the delimiter changes from ';' to ',' then you should set the following appsession-option delimiterstart , and the end point is not '=' it is ';' then you should set the following appsession-option delimiterstop ; Ah OK I get it now, thanks. A better case is the BEA 'Cookie'. JSESSIONID=GyvN6gTy7LKR2tyXDHvSzBhCfNH34YMG2f60xhVkXJNrrs5QvMjx!1267565914 So the appsession-option delimiterstop is '!'. But maybe it can be handled with the length. After this talk I think the usecase is solved with the current available options. Most likely. Also, if we have other special cases later, it might be easier to implement some methods specific to some servers than to have the user explicitly specify a number of complex arguments they won't always be able to figure out. For instance : appsession-mode weblogic Ok, lets keep it in mind maybe someone else find a better usecase so that we can decide it better if we should add this options or not. agreed. Regards, Willy
Re: Small patch for the appsession feature
Hi Cyril, On Fre 13.11.2009 22:50, Cyril Bonté wrote: Hello Willy, sorry, I didn't have time to work on the patch as I wanted. Le jeudi 5 novembre 2009 06:19:41, Willy Tarreau a écrit : Sorry but I can't see in the haproxy sources how the cookie prefix can be used for appsession. capture cookie allows to find this cookie prefix but it seems there's no code to then use it in appsession. If it's the case, I can work on it these next days, if you want. OK you're indeed right. So since this feature has never been usable nor used, let's fix it as you initially proposed. It works well but I have a question before considering the patch finalized :) As using the captured cookie may add an unwanted behaviour in some existant configurations, I set it as an option. First I added it as I did for request-learn but shouldn't it be better to define these options more haproxy like (using option and no option) ? Which would give : (no) option appsession-request-learn (no) option appsession-capture-cookie +1 Aleks
Re: Small patch for the appsession feature
Hi, On Sun, Nov 15, 2009 at 10:28:21PM +0100, Aleksandar Lazic wrote: Hi Cyril, On Fre 13.11.2009 22:50, Cyril Bonté wrote: Hello Willy, sorry, I didn't have time to work on the patch as I wanted. Le jeudi 5 novembre 2009 06:19:41, Willy Tarreau a écrit : Sorry but I can't see in the haproxy sources how the cookie prefix can be used for appsession. capture cookie allows to find this cookie prefix but it seems there's no code to then use it in appsession. If it's the case, I can work on it these next days, if you want. OK you're indeed right. So since this feature has never been usable nor used, let's fix it as you initially proposed. It works well but I have a question before considering the patch finalized :) As using the captured cookie may add an unwanted behaviour in some existant configurations, I set it as an option. First I added it as I did for request-learn but shouldn't it be better to define these options more haproxy like (using option and no option) ? Which would give : (no) option appsession-request-learn (no) option appsession-capture-cookie +1 I'm not that much in favor of options for such uses, because options are inherited from default sections and will cause unexpected behaviours. In my opinion, options should be used when they impact the general behaviour and when it is desirable to inherit them from default sections. Right now, appsessions are limited to backends only and cannot be declared in defaults sections, so I think it could become a bit awkward to have such a split configuration. What you can do however is to create a new prefix keyword like we have for timeout or tcp-request and put the flags somewhere else. appsession would have been fine but it's already used. Maybe you can use appcookie ? Something like this : appcookie request-learn appcookie capture Just an idea. Regards, Willy
Re: Small patch for the appsession feature
Hello Willy, sorry, I didn't have time to work on the patch as I wanted. Le jeudi 5 novembre 2009 06:19:41, Willy Tarreau a écrit : Sorry but I can't see in the haproxy sources how the cookie prefix can be used for appsession. capture cookie allows to find this cookie prefix but it seems there's no code to then use it in appsession. If it's the case, I can work on it these next days, if you want. OK you're indeed right. So since this feature has never been usable nor used, let's fix it as you initially proposed. It works well but I have a question before considering the patch finalized :) As using the captured cookie may add an unwanted behaviour in some existant configurations, I set it as an option. First I added it as I did for request-learn but shouldn't it be better to define these options more haproxy like (using option and no option) ? Which would give : (no) option appsession-request-learn (no) option appsession-capture-cookie -- Cyril Bonté
Re: Small patch for the appsession feature
Hi Cyril,
On Sun, Nov 01, 2009 at 12:19:05AM +0100, Cyril Bonté wrote:
Hello Willy and Aleksandar,
If you agree, I would like to apply this new patch to add some more integrity
checking on appsession.
* the session value (provided by the URL or by the request/response cookie)
is now well delimited :
currently, setting len 52 on a 32 chars value has a bad effect on load
balancing (for example, a part of the query string is considered as the
session id ; same thing happens with the cookie).
= with the patch, len 52 is now a maximum length.
OK.
* it adds a verification on the '=' char :
currently (with appsession JSESSIONID for example), an URL like
http://haproxy/path;jsessionidfake=0123... matches the session id
ake=0123...
= with the patch, jsessionidfake won't be recognized.
I think we must not do this, because one of the reasons for not
checking the equal sign was because of people who have to use IIS
with its ASPSESSION cookies. The end of the cookie name is part
of the value. So it's important that we can check cookie prefixes.
Tell me if you're OK for the loop added in get_srv_from_appsession(). This
can be a little slower but I think this will help to eliminate some troubles
in production.
At first glance I'm not too much scared.
However, please avoid variable declaration in the middle of the
code such as below, this is not compatible with compilers before
gcc version 3.0.
@@ -4080,7 +4080,11 @@
/* first, let's see if the cookie is
our appcookie*/
/* Cool... it's the right one */
- manage_client_side_appsession(t, p3);
+ int value_len = p4 - p3;
+ if (value_len t-be-appsession_len) {
+ value_len =
t-be-appsession_len;
+ }
+ manage_client_side_appsession(t, p3,
value_len);
Cheers,
Willy
Re: Small patch for the appsession feature
Le lundi 2 novembre 2009 12:09:43, Willy Tarreau a écrit :
* it adds a verification on the '=' char :
currently (with appsession JSESSIONID for example), an URL like
http://haproxy/path;jsessionidfake=0123... matches the session id
ake=0123...
= with the patch, jsessionidfake won't be recognized.
I think we must not do this, because one of the reasons for not
checking the equal sign was because of people who have to use IIS
with its ASPSESSION cookies.
Interesting to know about this ASPSESSION cookies, I'm not an IIS user.
The end of the cookie name is part of the value.
Ok, note that the only modification I made is for the url part, not for the
cookie. So maybe this is not a problem.
After some searches, I've seen that IIS does'nt support natively cookieless
persistance : it needs an ISAPI filter to do so (named Cookie munge).
I've read this filter sources and it looks like the URL part for
ASPSESSIONID is -ASP= (without the left side value), which doesn't start
with the semi-colon awaited by the appsession code.
Maybe I'm wrong, I'm really not an ASP developper and the information I found
are maybe outdated (it concerned IIS 4.0/5.0).
So it's important that we can check cookie prefixes.
Sorry but I can't see in the haproxy sources how the cookie prefix can be used
for appsession.
capture cookie allows to find this cookie prefix but it seems there's no code
to then use it in appsession. If it's the case, I can work on it these next
days, if you want.
However, please avoid variable declaration in the middle of the
code such as below, this is not compatible with compilers before
gcc version 3.0.
@@ -4080,7 +4080,11 @@
/* first, let's see if the cookie is
our appcookie*/
/* Cool... it's the right one */
- manage_client_side_appsession(t, p3);
+ int value_len = p4 - p3;
+ if (value_len t-be-appsession_len) {
+ value_len =
t-be-appsession_len;
+ }
+ manage_client_side_appsession(t, p3,
value_len);
Oops, I missed it :)
--
Cyril Bonté
Re: Small patch for the appsession feature
Hello Willy and Aleksandar,
If you agree, I would like to apply this new patch to add some more integrity
checking on appsession.
* the session value (provided by the URL or by the request/response cookie) is
now well delimited :
currently, setting len 52 on a 32 chars value has a bad effect on load
balancing (for example, a part of the query string is considered as the session
id ; same thing happens with the cookie).
= with the patch, len 52 is now a maximum length.
* it adds a verification on the '=' char :
currently (with appsession JSESSIONID for example), an URL like
http://haproxy/path;jsessionidfake=0123... matches the session id
ake=0123...
= with the patch, jsessionidfake won't be recognized.
Tell me if you're OK for the loop added in get_srv_from_appsession(). This can
be a little slower but I think this will help to eliminate some troubles in
production.
Note : the patch is based on haproxy 1.4 snapshot 20091030.
--
Cyril Bonté
diff -Naur a/include/proto/proto_http.h b/include/proto/proto_http.h
--- a/include/proto/proto_http.h 2009-10-29 12:00:11.0 +0100
+++ b/include/proto/proto_http.h 2009-10-31 21:53:06.0 +0100
@@ -75,7 +75,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, const char *buf);
+void manage_client_side_appsession(struct session *t, const char *buf, int len);
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 a/src/proto_http.c b/src/proto_http.c
--- a/src/proto_http.c 2009-10-29 12:00:11.0 +0100
+++ b/src/proto_http.c 2009-10-31 23:40:39.0 +0100
@@ -2479,7 +2479,7 @@
/* It needs to look into the URI */
if (s-be-appsession_name) {
- get_srv_from_appsession(s, req-data[msg-som], msg-sl.rq.l);
+ get_srv_from_appsession(s, req-data[msg-som + msg-sl.rq.u], msg-sl.rq.u_l);
}
/*
@@ -3778,7 +3778,7 @@
* 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, const char *buf) {
+void manage_client_side_appsession(struct session *t, const char *buf, int len) {
struct http_txn *txn = t-txn;
appsess *asession = NULL;
char *sessid_temp = NULL;
@@ -3796,8 +3796,8 @@
return;
}
- memcpy(t-sessid, buf, t-be-appsession_len);
- t-sessid[t-be-appsession_len] = 0;
+ memcpy(t-sessid, buf, len);
+ t-sessid[len] = 0;
}
if ((sessid_temp = pool_alloc2(apools.sessid)) == NULL) {
@@ -3806,8 +3806,8 @@
return;
}
- memcpy(sessid_temp, buf, t-be-appsession_len);
- sessid_temp[t-be-appsession_len] = 0;
+ memcpy(sessid_temp, buf, len);
+ sessid_temp[len] = 0;
asession = appsession_hash_lookup((t-be-htbl_proxy), sessid_temp);
/* free previously allocated memory */
@@ -4080,7 +4080,11 @@
/* first, let's see if the cookie is our appcookie*/
/* Cool... it's the right one */
- manage_client_side_appsession(t, p3);
+ int value_len = p4 - p3;
+ if (value_len t-be-appsession_len) {
+ value_len = t-be-appsession_len;
+ }
+ manage_client_side_appsession(t, p3, value_len);
#if defined(DEBUG_HASH)
Alert(manage_client_side_cookies\n);
appsession_hash_dump((t-be-htbl_proxy));
@@ -4508,8 +4512,12 @@
send_log(t-be, LOG_ALERT, Not enough Memory process_srv():asession-sessid:malloc().\n);
return;
}
-memcpy(t-sessid, p3, t-be-appsession_len);
-t-sessid[t-be-appsession_len] = 0;
+int value_len = p4 - p3;
+if (value_len t-be-appsession_len) {
+ value_len = t-be-appsession_len;
+}
+memcpy(t-sessid, p3, value_len);
+t-sessid[value_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 */
@@ -4653,25 +4661,35 @@
*/
void get_srv_from_appsession(struct session *t, const char *begin, int len)
{
- char *request_line;
+ char *end_params, *first_param, *cur_param, *next_param;
if (t-be-appsession_name == NULL ||
(t-txn.meth != HTTP_METH_GET t-txn.meth != HTTP_METH_POST) ||
- (request_line = memchr(begin, ';', len)) == NULL ||
- ((1 + t-be-appsession_name_len + 1 + t-be-appsession_len) (begin + len - request_line)))
+ ((first_param = memchr(begin, ';', len)) == NULL))
return;
- /* skip ';' */
- request_line++;
-
- /* look if we have a jsessionid */
- if (strncasecmp(request_line, t-be-appsession_name, t-be-appsession_name_len) != 0)
- return;
+ if ((end_params =
Re: Small patch for the appsession feature
Le dimanche 1 novembre 2009 00:19:05, Cyril Bonté a écrit :
Hello Willy and Aleksandar,
If you agree, I would like to apply this new patch to add some more integrity
checking on appsession.
Forget the previous patch file, this new one also prevents forcing the load
balancing to one server when the url session value is empty
(http://haproxy/path;jsessionid=?querystring)
--
Cyril Bonté
diff -Naur a/include/proto/proto_http.h b/include/proto/proto_http.h
--- a/include/proto/proto_http.h 2009-10-29 12:00:11.0 +0100
+++ b/include/proto/proto_http.h 2009-10-31 21:53:06.0 +0100
@@ -75,7 +75,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, const char *buf);
+void manage_client_side_appsession(struct session *t, const char *buf, int len);
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 a/src/proto_http.c b/src/proto_http.c
--- a/src/proto_http.c 2009-10-29 12:00:11.0 +0100
+++ b/src/proto_http.c 2009-11-01 01:28:03.0 +0100
@@ -2479,7 +2479,7 @@
/* It needs to look into the URI */
if (s-be-appsession_name) {
- get_srv_from_appsession(s, req-data[msg-som], msg-sl.rq.l);
+ get_srv_from_appsession(s, req-data[msg-som + msg-sl.rq.u], msg-sl.rq.u_l);
}
/*
@@ -3778,7 +3778,7 @@
* 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, const char *buf) {
+void manage_client_side_appsession(struct session *t, const char *buf, int len) {
struct http_txn *txn = t-txn;
appsess *asession = NULL;
char *sessid_temp = NULL;
@@ -3796,8 +3796,8 @@
return;
}
- memcpy(t-sessid, buf, t-be-appsession_len);
- t-sessid[t-be-appsession_len] = 0;
+ memcpy(t-sessid, buf, len);
+ t-sessid[len] = 0;
}
if ((sessid_temp = pool_alloc2(apools.sessid)) == NULL) {
@@ -3806,8 +3806,8 @@
return;
}
- memcpy(sessid_temp, buf, t-be-appsession_len);
- sessid_temp[t-be-appsession_len] = 0;
+ memcpy(sessid_temp, buf, len);
+ sessid_temp[len] = 0;
asession = appsession_hash_lookup((t-be-htbl_proxy), sessid_temp);
/* free previously allocated memory */
@@ -4080,7 +4080,11 @@
/* first, let's see if the cookie is our appcookie*/
/* Cool... it's the right one */
- manage_client_side_appsession(t, p3);
+ int value_len = p4 - p3;
+ if (value_len t-be-appsession_len) {
+ value_len = t-be-appsession_len;
+ }
+ manage_client_side_appsession(t, p3, value_len);
#if defined(DEBUG_HASH)
Alert(manage_client_side_cookies\n);
appsession_hash_dump((t-be-htbl_proxy));
@@ -4508,8 +4512,12 @@
send_log(t-be, LOG_ALERT, Not enough Memory process_srv():asession-sessid:malloc().\n);
return;
}
-memcpy(t-sessid, p3, t-be-appsession_len);
-t-sessid[t-be-appsession_len] = 0;
+int value_len = p4 - p3;
+if (value_len t-be-appsession_len) {
+ value_len = t-be-appsession_len;
+}
+memcpy(t-sessid, p3, value_len);
+t-sessid[value_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 */
@@ -4653,25 +4661,37 @@
*/
void get_srv_from_appsession(struct session *t, const char *begin, int len)
{
- char *request_line;
+ char *end_params, *first_param, *cur_param, *next_param;
if (t-be-appsession_name == NULL ||
(t-txn.meth != HTTP_METH_GET t-txn.meth != HTTP_METH_POST) ||
- (request_line = memchr(begin, ';', len)) == NULL ||
- ((1 + t-be-appsession_name_len + 1 + t-be-appsession_len) (begin + len - request_line)))
+ ((first_param = memchr(begin, ';', len)) == NULL))
return;
- /* skip ';' */
- request_line++;
-
- /* look if we have a jsessionid */
- if (strncasecmp(request_line, t-be-appsession_name, t-be-appsession_name_len) != 0)
- return;
+ if ((end_params = memchr(first_param, '?', len - (begin - first_param))) == NULL) {
+ end_params = begin + len;
+ }
- /* skip jsessionid= */
- request_line += t-be-appsession_name_len + 1;
-
- manage_client_side_appsession(t, request_line);
+ cur_param = next_param = end_params;
+ while (cur_param first_param) {
+ cur_param--;
+ if (cur_param[0] == ';') {
+ if ((cur_param + t-be-appsession_name_len + 1 next_param)
+(cur_param[t-be-appsession_name_len + 1] == '=')
+(strncasecmp(cur_param + 1, t-be-appsession_name, t-be-appsession_name_len) == 0)) {
+cur_param +=
Re: Small patch for the appsession feature
On Thu, Oct 15, 2009 at 12:15:40AM +0200, Cyril Bonté wrote: OK, here comes the 2 patch files for haproxy-1.3.21 and haproxy-1.4-dev4. Please note 3 minor changes in those versions (proto_http.c / proto_http.h) : Cyril, I have merged your two patches. I applied a very minor change, I removed the appsession_option field and used the normal proxy options instead ; it was not necessary to add 32 bits to the proxy while there were plenty available for that purpose. I have also added a few lines in the doc about the new keyword, and committed with your first mail as a commit message. Thanks ! Willy
Re: Small patch for the appsession feature
Le dimanche 18 octobre 2009 12:05:55, Willy Tarreau a écrit : Cyril, I have merged your two patches. Thanks ! I'm thinking of working on a second patch. The documentation says that appsession looks for the session in the query string but this is not really the case. Currently, it parses the first path parameter (followed by ';'). I would like to add an option to decide if appsession parses path parameters (not only the first one) or the query string parameters so it can work for URLs like http://website/path?PHPSESSID=; for example. I applied a very minor change, I removed the appsession_option field and used the normal proxy options instead ; it was not necessary to add 32 bits to the proxy while there were plenty available for that purpose. You're right, this was not useful, I didn't want to conflict with some potential other patches. I have also added a few lines in the doc about the new keyword Yes, the most important part of the patch was missing :) -- Cyril Bonté
Re: Small patch for the appsession feature
On Tue, Oct 13, 2009 at 10:41:59PM +0200, Aleksandar Lazic wrote: On Die 13.10.2009 21:34, Cyril Bonté wrote: Le lundi 12 octobre 2009 23:17:43, Aleksandar Lazic a écrit : Yes, you're right, I missed it after several tests on different snapshots. Here comes a second patch to reintroduce these debug lines : Thanks. OK Aleks, so is Cyril's patch OK for you and should I merge it now ? Also, do you guys think it should it go to 1.3 too (balance affected users vs risk of regression) ? Thanks, Willy
Re: Small patch for the appsession feature
On Mit 14.10.2009 21:40, Willy Tarreau wrote: On Tue, Oct 13, 2009 at 10:41:59PM +0200, Aleksandar Lazic wrote: On Die 13.10.2009 21:34, Cyril Bonté wrote: Le lundi 12 octobre 2009 23:17:43, Aleksandar Lazic a écrit : Yes, you're right, I missed it after several tests on different snapshots. Here comes a second patch to reintroduce these debug lines : Thanks. OK Aleks, so is Cyril's patch OK for you and should I merge it now ? Yes please. Also, do you guys think it should it go to 1.3 too (balance affected users vs risk of regression) ? I think yes, if it is not too difficult. BR Aleks
Re: Small patch for the appsession feature
On Wed, Oct 14, 2009 at 10:20:04PM +0200, Aleksandar Lazic wrote: On Mit 14.10.2009 21:40, Willy Tarreau wrote: On Tue, Oct 13, 2009 at 10:41:59PM +0200, Aleksandar Lazic wrote: On Die 13.10.2009 21:34, Cyril Bonté wrote: Le lundi 12 octobre 2009 23:17:43, Aleksandar Lazic a écrit : Yes, you're right, I missed it after several tests on different snapshots. Here comes a second patch to reintroduce these debug lines : Thanks. OK Aleks, so is Cyril's patch OK for you and should I merge it now ? Yes please. Also, do you guys think it should it go to 1.3 too (balance affected users vs risk of regression) ? I think yes, if it is not too difficult. OK thanks for your review and comments Aleks! Cyril, care to send an updated patch with the latest changes ? Do it for either 1.3 or 1.4, I'll do the back/forward port. If you prefer to proceed on both, of course feel free to do so :-) Willy
Re: Small patch for the appsession feature
Le mercredi 14 octobre 2009 22:23:54, Willy Tarreau a écrit : Cyril, care to send an updated patch with the latest changes ? OK ! Do it for either 1.3 or 1.4, I'll do the back/forward port. If you prefer to proceed on both, of course feel free to do so :-) I can do both, this will let you work on something else ;) -- Cyril Bonté
Re: Small patch for the appsession feature
On Wed, Oct 14, 2009 at 10:36:49PM +0200, Cyril Bonté wrote: Le mercredi 14 octobre 2009 22:23:54, Willy Tarreau a écrit : Cyril, care to send an updated patch with the latest changes ? OK ! Do it for either 1.3 or 1.4, I'll do the back/forward port. If you prefer to proceed on both, of course feel free to do so :-) I can do both, this will let you work on something else ;) Nice, I appreciate it. thanks, Willy
Re: Small patch for the appsession feature
Le mercredi 14 octobre 2009 22:39:39, Willy Tarreau a écrit :
Do it for either 1.3 or 1.4, I'll do the back/forward port. If you
prefer to proceed on both, of course feel free to do so :-)
I can do both, this will let you work on something else ;)
Nice, I appreciate it.
OK, here comes the 2 patch files for haproxy-1.3.21 and haproxy-1.4-dev4.
Please note 3 minor changes in those versions (proto_http.c / proto_http.h) :
1. void manage_client_side_appsession(struct session *t, char *buf);
becomes
void manage_client_side_appsession(struct session *t, const char *buf);
2. An unused variable in get_srv_from_appsession(...) has been removed.
3. After some more tests, I've applied a fix on request_count (used for debug)
to have nearly the same behaviour in normal mode and request-learn mode.
--
Cyril Bonté
diff -Naur haproxy-1.4-dev4/include/proto/proto_http.h haproxy-1.4-dev4-appsession-final/include/proto/proto_http.h
--- haproxy-1.4-dev4/include/proto/proto_http.h 2009-10-12 06:40:53.0 +0200
+++ haproxy-1.4-dev4-appsession-final/include/proto/proto_http.h 2009-10-14 22:55:55.0 +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, const 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-final/include/types/proxy.h
--- haproxy-1.4-dev4/include/types/proxy.h 2009-10-12 06:40:53.0 +0200
+++ haproxy-1.4-dev4-appsession-final/include/types/proxy.h 2009-10-12 21:24:42.0 +0200
@@ -124,6 +124,9 @@
#define PR_O2_INDEPSTR 0x1000 /* independant streams, don't update rex on write */
#define PR_O2_SOCKSTAT 0x2000 /* collect provide separate statistics for sockets */
+/* bits for proxy-appsession_options */
+#define PR_O_AS_REQL 0x0001 /* 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-final/include/types/session.h
--- haproxy-1.4-dev4/include/types/session.h 2009-10-12 06:40:53.0 +0200
+++ haproxy-1.4-dev4-appsession-final/include/types/session.h 2009-10-12 21:25:15.0 +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-final/src/cfgparse.c
--- haproxy-1.4-dev4/src/cfgparse.c 2009-10-12 06:40:53.0 +0200
+++ haproxy-1.4-dev4-appsession-final/src/cfgparse.c 2009-10-12 21:26:10.0 +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-final/src/client.c
--- haproxy-1.4-dev4/src/client.c 2009-10-12 06:40:53.0 +0200
+++ haproxy-1.4-dev4-appsession-final/src/client.c 2009-10-12 21:26:38.0 +0200
@@ -184,7 +184,7 @@
*/
s-be = s-fe = p;
- s-req = s-rep = NULL; /* will be allocated later */
+
Re: Small patch for the appsession feature
Le lundi 12 octobre 2009 23:17:43, Aleksandar Lazic a écrit : Hi Cyril, good catch. +} + +/* -#if defined(DEBUG_HASH) -Alert(manage_client_side_cookies\n); - appsession_hash_dump((t-be-htbl_proxy)); -#endif After a quick look I think it would be nice to dump the sessions also in the client. Yes, you're right, I missed it after several tests on different snapshots. Here comes a second patch to reintroduce these debug lines : diff -Naur haproxy-1.4-dev4-appsession/src/proto_http.c haproxy-1.4-dev4-appsession2/src/proto_http.c --- haproxy-1.4-dev4-appsession/src/proto_http.c2009-10-12 21:56:38.0 +0200 +++ haproxy-1.4-dev4-appsession2/src/proto_http.c 2009-10-13 21:26:19.0 +0200 @@ -3862,6 +3862,10 @@ /* Cool... it's the right one */ manage_client_side_appsession(t, p3); +#if defined(DEBUG_HASH) + Alert(manage_client_side_cookies\n); + appsession_hash_dump((t-be-htbl_proxy)); +#endif }/* end if ((t-proxy-appsession_name != NULL) ... */ } @@ -4450,6 +4454,10 @@ request_line += t-be-appsession_name_len + 1; manage_client_side_appsession(t, request_line); +#if defined(DEBUG_HASH) + Alert(get_srv_from_appsession\n); + appsession_hash_dump((t-be-htbl_proxy)); +#endif } -- Cyril Bonté
Re: Small patch for the appsession feature
On Die 13.10.2009 21:34, Cyril Bonté wrote: Le lundi 12 octobre 2009 23:17:43, Aleksandar Lazic a écrit : Yes, you're right, I missed it after several tests on different snapshots. Here comes a second patch to reintroduce these debug lines : Thanks.
