Re: [Re: jakarta-tomcat cvs diff added session support using URL rewrite]

[G�bor Lipt�k <ga...@usa.net>]

Hans Bergsten <ha...@gefionsoftware.com> wrote:
> "G�bor Lipt�k" wrote:
> > 
> > Hans Bergsten <ha...@gefionsoftware.com> wrote:
> > > Hans Bergsten wrote:
> > > >
> > > > G�bor,
> > > >
> > > > Great, thanks for providing a patch for this feature. One comment
though:
> > > >
> > > > >      public String encodeURL(String url) {
> > > > > -        // XXX
> > > > > -        // we don't support url rewriting yet!
> > > > > -        return url;
> > > > > +      Request request=response.getRequest();
> > > > > +      // if I have a session
> > > > > +      if (request.isRequestedSessionIdValid()){
> > > > > +       // if session not from cookie
> > > > > +       if (!request.isRequestedSessionIdFromCookie()) {
> > > > > +         StringBuffer sb=new StringBuffer(url);
> > > > > +         sb.append(";");
> > > > > +
> > sb.append(org.apache.tomcat.core.Constants.SESSION_COOKIE_NAME);
> > > > > +         sb.append("=");
> > > > > +         sb.append(request.getRequestedSessionId());
> > > > > +         url=sb.toString();
> > > > > +       }
> > > > > +      }
> > > > > +      return url;
> > > > >      }
> > > >
> > > > I don't think it's enough to check the information about the
"requested
> > > > session",
> > > > since URL rewriting should work even if the session is brand new. So
I
> > > > suggest adding a "session.isNew()" test as well, and if it is, skip
all 
> > > > tests on the "requested session".
> > 
> > What this code provides is both a cookie and URL rewrite the first time a
> > session is created, than URL rewrite only happens if the cookie was
rejected.
> 
> But doesn't request.isRequestedSessionIdValid() return false if the
> session is new (since there's no requested session at all)? If so, this
> method will not rewrite URLs in a new session.
> 
> > If there is a better way to verify that the user accepts cookies from the
> > current host, than that call could replace
> > 
> > if (!request.isRequestedSessionIdFromCookie())
> 
> This should work fine for a request that includes a session ID, but that's
> not what I commented on; my concern was that new sessions don't seem to
taken
> care of.

??? Again as per my testing, the code provides is both a cookie and URL
rewrite the first time a session is created, than URL rewrite only happens if
the cookie was rejected. You are welcome to modify the code if different
interaction is desired.

> > > Actually, I noticed one more thing that's missing. The URL may contain
> > > a query string. If so, the session ID needs to be inserted before the
> > > query string.
> > 
> > the url sent to browser generated as follows:
> > 
> > http://host/path?value=name&value1=name1;JSESSIONID=cookie
> 
> This is not the correct format for a URL with both a session ID and a query
> string, AFAIK. It's hard to find one place where the "path params" of a URL
> is defined in the latest HTTP spec, but in an earlier version (RFC 2068)
> it was described like this:
> 
>    3.2.1 General Syntax
> 
>    URIs in HTTP can be represented in absolute form or relative to some
>    known base URI, depending upon the context of their use. The two
>    forms are differentiated by the fact that absolute URIs always begin
>    with a scheme name followed by a colon.
> 
>           URI            = ( absoluteURI | relativeURI ) [ "#" fragment ]
> 
>           absoluteURI    = scheme ":" *( uchar | reserved )
> 
>           relativeURI    = net_path | abs_path | rel_path
> 
>           net_path       = "//" net_loc [ abs_path ]
>           abs_path       = "/" rel_path
>           rel_path       = [ path ] [ ";" params ] [ "?" query ]
> 
> As you can see, the param should be before the query string. The most
recent
> specs (RFC 2616, RFC 2396) have this as well, but it's not described as
clearly
> in one place.

Hmmm ... Thanks for pointing the spec out. To accomodate seems to a fairly
easy change, and I will make the modifications right after these previous
changes show up in the Tomcat CVS tree.
 
> Hans
> -- 
> Hans Bergsten		hans@gefionsoftware.com
> Gefion Software		http://www.gefionsoftware.com


____________________________________________________________________
Get free email and a permanent address at http://www.amexmail.com/?A=1