Re: appsession fix

On Sat, Feb 16, 2008 at 12:12:30AM +0100, Aleksandar Lazic wrote:
> On Fre 15.02.2008 23:54, Willy Tarreau wrote:
> >On Thu, Feb 14, 2008 at 02:46:44PM -0700, Ryan Warnick wrote:
> >>Sounds good. How about that first change that I proposed (the bug
> >>fix)?
> >
> >I've just checked the code, and while I think you're right, I think
> >that there are two other places with exactly similar code which need
> >this patch too :
> >
> >- manage_client_side_cookies(), around line 4098.
>
> In this function the code is similar:
>
> ###
> 3971 /* only do insert, if
> lookup fails */
> 3972 asession_temp =
> appsession_hash_lookup(&(t->be->htbl_proxy), asession_temp->sessid);
> 3973 if (asession_temp == NULL) {
> ###
>
> >- manage_server_side_cookies(), around line 4564.
> >- and yours, get_srv_from_appsession() around line 4734.
> >
> >What do you think ? Aleks, do you agree with me ?
>
> For the other two yes, but isn't it the same?!
>
> get_srv_from_appsession()
>
> 4609 /* only do insert, if lookup fails */
> 4610 if (appsession_hash_lookup(&(t->be->htbl_proxy),
> asession_temp->sessid) == NULL) {
>
> in meta-code, as I understand it.
>
> ####
> $VAR = RETVAL_OF_FUNC;
> if($VAL ...)
>
> if(RETVAL_OF_FUNC ..
>
> ####

That's what I understand too, but since asession_temp was assigned to something else a few lines above, I was not sure.

> What I think is that we should have one method, algorithm which checks
> the (apsession-)?cookies.

It would be preferable, indeed.

> What I mean is that the both, manage_(server|client)_side_cookies,
> functios make mostly the same, please correct me if I'am wrong.

No, the only common thing you find there is the cookie parser. Both do very different things, because one is responsible for learning/rewriting/ inserting a cookie (server), the other one is responsible for checking/removing a cookie (client).

> Sorry that I can't code for the moment but I'am to busy ;-((

OK, don't be sorry :-)

Ryan, I will apply your fix and replicate it to the two other places. If, in the mean time you could confirm with your application that it still works as expected with the fix in the 3 places, it would be great !

Thanks!
Willy Received on 2008/02/16 07:22