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
Le dimanche 29 novembre 2009 23:43:15, Willy Tarreau a écrit :
> The suggested way to to that is to enclose the values between the comment
> and the mask like here (though any other proposal might fit) :
>
> /* appsession */
> #define PR_O2_AS_REQL 0x4000 /* learn the session id from the
> request */
> #define PR_O2_AS_PFX 0x8000 /* match on the cookie prefix */
>
> /* Encoding of appsession cookie matching modes : 2 possible values => 1 bit
> */
> #define PR_O2_AS_M_PP 0x /* path-parameters mode (the default
> mode) */
> #define PR_O2_AS_M_QS 0x0001 /* query-string mode */
> #define PR_O2_AS_M_ANY0x0001 /* mask covering all
> PR_O2_AS_M_* values */
I'll remember that if I have more patches to send ;)
> > 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;
> > + char separator;
> > + int value_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) ||
> > - (request_line = memchr(begin, ';', len)) == NULL ||
> > - ((1 + t->be->appsession_name_len + 1 + t->be->appsession_len) >
> > (begin + len - request_line)))
> > + (t->txn.meth != HTTP_METH_GET && t->txn.meth != HTTP_METH_POST)) {
> > return;
> > + }
>
> I may be wrong but I think we lost a length check above and that
> we'll always onsider we have bytes available in the buffer
> so that memchr(begin, ';', len) has no chance of going beyond the
> buffer's limit. Oh well, maybe it was already like that :-/
>
> 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 ?
> 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).
Thanks a lot.
--
Cyril Bonté
Re: Small patch for the "appsession" feature
Hi Cyril,
On Sun, Nov 29, 2009 at 08:04:48PM +0100, Cyril Bonté wrote:
> Hello,
> considering what we said in this thread, here is the complete patch.
OK. First, let me say I'm really satisfied by the quality of this
patch, I suppose you have spent quite a part of your sunday on it.
I just have a few comments (as usual) below :
> +/* appsession */
> +#define PR_O2_AS_REQL0x4000 /* learn the session id from
> the request */
> +#define PR_O2_AS_PFX 0x8000 /* match on the cookie prefix */
> +#define PR_O2_AS_M_PP0x /* path-parameters mode (the
> default mode) */
> +#define PR_O2_AS_M_QS0x0001 /* query-string mode */
> +#define PR_O2_AS_M_ANY (PR_O2_AS_M_PP | PR_O2_AS_M_QS)
It's dangerous to set the PR_O2_AS_M_* values together with the other ones
without a comment first and an explicit bit field for the mask later. The
risk is that if one day we add another value, the mask may or may not be
enlarged to support it, and it is possible that someone will believe
the 0x is an error and will like to fix it.
The suggested way to to that is to enclose the values between the comment
and the mask like here (though any other proposal might fit) :
/* appsession */
#define PR_O2_AS_REQL 0x4000 /* learn the session id from the
request */
#define PR_O2_AS_PFX0x8000 /* match on the cookie prefix */
/* Encoding of appsession cookie matching modes : 2 possible values => 1 bit */
#define PR_O2_AS_M_PP 0x /* path-parameters mode (the default
mode) */
#define PR_O2_AS_M_QS 0x0001 /* query-string mode */
#define PR_O2_AS_M_ANY 0x0001 /* mask covering all PR_O2_AS_M_*
values */
> 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;
> + char separator;
> + int value_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) ||
> - (request_line = memchr(begin, ';', len)) == NULL ||
> - ((1 + t->be->appsession_name_len + 1 + t->be->appsession_len) >
> (begin + len - request_line)))
> + (t->txn.meth != HTTP_METH_GET && t->txn.meth != HTTP_METH_POST)) {
> return;
> + }
I may be wrong but I think we lost a length check above and that
we'll always onsider we have bytes available in the buffer
so that memchr(begin, ';', len) has no chance of going beyond the
buffer's limit. Oh well, maybe it was already like that :-/
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.
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.
Thanks!
Willy
Re: Small patch for the "appsession" feature
Hello, considering what we said in this thread, here is the complete patch. To sum up : - len : it's now the max number of characters for the value, preventing garbaged results. - a new option "prefix" is added, this allows to use dynamic cookie names (e.g. ASPSESSIONIDXXX). Previously in the thread, I wanted to use the value found with "capture cookie" but when i started to update the documentation, I found this solution quite weird. I've made a small rework to not depend on "capture cookie". - There's the posssiblity to define the URL parser mode (path parameters or query string). This patch is based on the haproxy-ss-20091129 snapshot. Le mercredi 18 novembre 2009 06:40:52, Willy Tarreau a écrit : > 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 :-)) Well, after a last minute check, the URL parameters were parsed before the client cookies. I've interverted the 2 steps so that URL parameters are only parsed if no client cookie is found. Aleksandar, Willy, is this ok for you ? -- Cyril Bonté diff -Naur haproxy-ss-20091129/doc/configuration.txt haproxy-ss-20091129-appsession-options/doc/configuration.txt --- haproxy-ss-20091129/doc/configuration.txt 2009-11-28 08:21:29.0 +0100 +++ haproxy-ss-20091129-appsession-options/doc/configuration.txt 2009-11-29 19:57:34.0 +0100 @@ -861,7 +861,8 @@ See section 7 about ACL usage. -appsession len timeout [request-learn] +appsession len timeout + [request-learn] [prefix] [mode ] Define session stickiness on an existing application cookie. May be used in sections : defaults | frontend | listen | backend no|no| yes | yes @@ -869,7 +870,7 @@ this is the name of the cookie used by the application and which HAProxy will have to learn for each new session. - this is the number of characters that will be memorized and + this is the max number of characters that will be memorized and checked in each cookie value. this is the time after which the cookie will be removed from @@ -884,13 +885,34 @@ the application's session and the correct server is selected. It is recommended to specify this option to improve reliability. +prefix When this option is specified, haproxy will match on the cookie + prefix (or URL parameter prefix). The appsession value is the + data following this prefix. + + Example : + appsession ASPSESSIONID len 64 timeout 3h prefix + + This will match the cookie ASPSESSIONID=X, + the appsession value will be =X. + +mode This option allows to change the URL parser mode. + 2 modes are currently supported : + - path-parameters : + The parser looks for the appsession in the path parameters + part (each parameter is separated by a semi-colon), which is + convenient for JSESSIONID for example. + This is the default mode if the option is not set. + - query-string : + In this mode, the parser will look for the appsession in the + query string. + When an application cookie is defined in a backend, HAProxy will check when the server sets such a cookie, and will store its value in a table, and associate it with the server's identifier. Up to characters from the value will be retained. On each connection, haproxy will look for this - cookie both in the "Cookie:" headers, and as a URL parameter in the query - string. If a known value is found, the client will be directed to the server - associated with this value. Otherwise, the load balancing algorithm is + cookie both in the "Cookie:" headers, and as a URL parameter (depending on + the mode used). If a known value is found, the client will be directed to the + server associated with this value. Otherwise, the load balancing algorithm is applied. Cookies are automatically removed from memory when they have been unused for a duration longer than . diff -Naur haproxy-ss-20091129/include/proto/proto_http.h haproxy-ss-20091129-appsession-options/include/proto/proto_http.h --- haproxy-ss-20091129/include/proto/proto_http.h 2009-11-28 08:21:29.0 +0100 +++ haproxy-ss-20091129-appsession-options/include/proto/proto_http.h 2009-11-29 18:38:54.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, str
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 22:52, Cyril Bonté wrote: Le lundi 16 novembre 2009 13:41:14, Willy Tarreau a écrit : On Mon, Nov 16, 2009 at 01:04:17PM +0100, Aleksandar Lazic wrote: [snipp] > 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 ; (...) Yes, this was exactly why I first thought of adding options on several lines. Having all the options on the same like can quickly make the configuration difficult to read/debug. Imagine a line like : "appsession PHPSESSID len 32 timeout 1h request-learn cookie-capture \ start-delimiters ?& stop-delimiters &" I found it not really user friendly... (well, my idea to split the line in several options looks not much better) ;-) (...) 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 (...) I really like this idea. This would make the configuration simpler and the algorithm to parse the URL can be easily optimized for each mode. I agree. Based on this idea, maybe we can stay with all the options on the same line (looks like the "server" configuration part). The most complex line would be : "appsession PHPSESSID len 32 timeout 1h mode query-string request-learn cookie-capture" Looks ok for me. 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? BR Aleks
Re: Small patch for the "appsession" feature
Le lundi 16 novembre 2009 13:41:14, Willy Tarreau a écrit : > 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 ; > (...) Yes, this was exactly why I first thought of adding options on several lines. Having all the options on the same like can quickly make the configuration difficult to read/debug. Imagine a line like : "appsession PHPSESSID len 32 timeout 1h request-learn cookie-capture start-delimiters ?& stop-delimiters &" I found it not really user friendly... (well, my idea to split the line in several options looks not much better) > (...) > 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 > (...) I really like this idea. This would make the configuration simpler and the algorithm to parse the URL can be easily optimized for each mode. Based on this idea, maybe we can stay with all the options on the same line (looks like the "server" configuration part). The most complex line would be : "appsession PHPSESSID len 32 timeout 1h mode query-string request-learn cookie-capture" 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") -- Cyril Bonté
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
On Mon 16.11.2009 10:22, Willy Tarreau wrote: On Mon, Nov 16, 2009 at 10:08:56AM +0100, Aleksandar Lazic wrote: [snipp] 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. Ok. 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 ? 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 ; 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. 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. 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 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
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
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
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
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:///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
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:///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 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:///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->app
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:///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-
Re: Small patch for the "appsession" feature
On Son 18.10.2009 12:46, Cyril Bonté wrote: 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. Our need was to catch the jsessionid which comes before the query string and was delimited with '!' => Bea Weblogic Cookie. I have thought to change this but things changes ;-( The idea was: 1.) appsession per server, if necessary 2.) add delimiter, to be able to be more flexible ;-) 3.) tell the engine where to search for the session, path || query string. ATTENTION: What happen when the query string is very long => Performance, Security, ... 4.) add to stat page the appsession to see how much we already have 5.) add to stat page how often (request_count) and how long a appsession exists. Reason: tune the application setup for the session timeout. Maybe this could be also interesting for the 'normal' cookies ;-) 6.) add the appsession hash dump into the sig_dump_state(). BTW: does the SIGHUP is moved to reload the config or will it stay to dump the states? Afaik the most server use HUP for reload. You see there was a lot todos and ideas. I'am happy that anybody have pickedup this feature, thanks Cyril. aleks
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 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 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; /* wil
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: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: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
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 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 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.
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
Hi Cyril,
good catch.
On Mon 12.10.2009 23:03, Cyril Bonté wrote:
Hello,
as discussed previously with Willy Tarreau, I'd like to propose a patch
for the appsession code.
This patch has 2 goals :
[snipp]
diff -Naur haproxy-1.4-dev4/src/proto_http.c
haproxy-1.4-dev4-appsession/src/proto_http.c
+++ haproxy-1.4-dev4-appsession/src/proto_http.c2009-10-12
21:56:38.0 +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) {
[snipp]
+}
+
+/*
-#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.
BR
Aleks
