You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Mark Thomas <ma...@apache.org> on 2005/11/24 20:34:23 UTC

getContext() - spec interpretation

All,

I have been looking at bug 13040 and reviewing the current 
getContext() implementation. I saw Remy's comment from some time ago 
when fixing some related bugs 
(http://marc.theaimsgroup.com/?l=tomcat-dev&m=106008981803343&w=2) 
that this would be better if the spec mandated that the parameter 
passed to getContext() must be an exact match for a context path.

Having read the 2.4 spec several times I am pretty sure that is does 
say this, albeit not as directly as it might. I assume (perhaps 
wrongly) that any changes in this area will generate a lot of debate 
so I wanted to do the debate and then change the code.

The key parts of the spec are:
SRV.14.2.8 ServletContext
<snip>
public ServletContext getContext(java.lang.String uripath)
<snip>
uripath - a String specifying the context path of another web 
application in the container.
<snip>

My interpretation is:
SRV.14.2.8 says the parameter is a context path
SRV.4.4 is very clear about what is context path is

Therefore, getContext() must look for an exact match of uripath 
against the context paths for the currently deployed web-apps.

My proposal, therefore is to change the getContext() implementation to 
look for an exact match. This is stricter than the current 
implementation (and may cause problems for some users) but will fix a 
number of odd behaviours including the one described in bug 13040.

In the unlikely event that no-one disagrees with my interpretation, 
I'll commit a fix over the weekend to TC4 and TC5. (The 2.3 spec has 
the exact same wording.)

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: getContext() - spec interpretation

Posted by Remy Maucherat <re...@apache.org>.
Mark Thomas wrote:
> Bill Barker wrote:
> 
>> I can't really dispute the interpretation of the spec, but the change 
>> is likely to break many more webapps then it fixes :(.
> 
> 
> I thought this too. However, I think we should implement the spec as is. 
> If we start relaxing it here, then where else do we relax it? If other 
> Servlet containers start relaxing the specs in their own ways it rapidly 
> defeats the object of having a spec in the first place.
> 
> As it happens, the SSI functionality depends on the incorrect 
> getContext() implementation and I intend to use the current getContext() 
> implementation as the basis for a fix to the SSI code. This fix should 
> be transferable to other web-apps that have similar dependencies on 
> getContext().
> 
> My preference is to post a 'heads-up' to the users list that this change 
> is coming in the next release along with a link to the SSI changes so 
> users can start to look at their apps ahead of the next release.

Personally, I don't see any way right now other than keep the current 
behavior (at least that will be the case in my tree) :(

Rémy

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: getContext() - spec interpretation

Posted by Cédrik LIME <cl...@icare-france.com>.
At 18:20 27/11/2005 +0000, you wrote:
>Bill Barker wrote:
>>I can't really dispute the interpretation of 
>>the spec, but the change is likely to break many more webapps then it fixes :(.
>
>I thought this too. However, I think we should 
>implement the spec as is. If we start relaxing 
>it here, then where else do we relax it? If 
>other Servlet containers start relaxing the 
>specs in their own ways it rapidly defeats the 
>object of having a spec in the first place.
[snip]
>My preference is to post a 'heads-up' to the 
>users list that this change is coming in the 
>next release along with a link to the SSI 
>changes so users can start to look at their apps ahead of the next release.

Don't forget that not all tc users are on the 
users mailing list. If this change can 
potentially break user applications, there should 
be IMHO a big sign saying so in the release note web page as well.

As for the question, I agree with Mark (+1 for implementing the spec tightly).

         Cédrik 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: getContext() - spec interpretation

Posted by Mark Thomas <ma...@apache.org>.
Mark Thomas wrote:

The complications I referred to in the SSI commit are:

1. The definition of uripath says it is a context path which for the 
root context would be "". In the method description it says uripath 
must always start with "/". This is not consistent.

2. The definition of uripath says "uripath - a String specifying the 
context path of another web application in the container.". The spec 
is silent as to what the behaviour should be if uripath is the context 
path for the current context, ie not "another web application".

I think there is room for interpretation in point 2 and we can safely 
return the current context if that is what the caller has asked for.

My current thinking for point 1 is to treat the root context as a 
special case and, crossContext permitting,  to return the root context 
for getContext("/")

This doesn't change any of the points I made below.

I am going to think about this for a few more days before I make any 
changes.

> Bill Barker wrote:
> 
>> I can't really dispute the interpretation of the spec, but the change 
>> is likely to break many more webapps then it fixes :(.
> 
> 
> I thought this too. However, I think we should implement the spec as is. 
> If we start relaxing it here, then where else do we relax it? If other 
> Servlet containers start relaxing the specs in their own ways it rapidly 
> defeats the object of having a spec in the first place.
> 
> As it happens, the SSI functionality depends on the incorrect 
> getContext() implementation and I intend to use the current getContext() 
> implementation as the basis for a fix to the SSI code. This fix should 
> be transferable to other web-apps that have similar dependencies on 
> getContext().
> 
> My preference is to post a 'heads-up' to the users list that this change 
> is coming in the next release along with a link to the SSI changes so 
> users can start to look at their apps ahead of the next release.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: getContext() - spec interpretation

Posted by Mark Thomas <ma...@apache.org>.
Bill Barker wrote:
> I can't really dispute the interpretation of the spec, but the change is 
> likely to break many more webapps then it fixes :(.

I thought this too. However, I think we should implement the spec as 
is. If we start relaxing it here, then where else do we relax it? If 
other Servlet containers start relaxing the specs in their own ways it 
rapidly defeats the object of having a spec in the first place.

As it happens, the SSI functionality depends on the incorrect 
getContext() implementation and I intend to use the current 
getContext() implementation as the basis for a fix to the SSI code. 
This fix should be transferable to other web-apps that have similar 
dependencies on getContext().

My preference is to post a 'heads-up' to the users list that this 
change is coming in the next release along with a link to the SSI 
changes so users can start to look at their apps ahead of the next 
release.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: getContext() - spec interpretation

Posted by Bill Barker <wb...@wilshire.com>.
----- Original Message ----- 
From: "Mark Thomas" <ma...@apache.org>
To: <de...@tomcat.apache.org>
Sent: Thursday, November 24, 2005 11:34 AM
Subject: getContext() - spec interpretation


> All,
>
> I have been looking at bug 13040 and reviewing the current getContext() 
> implementation. I saw Remy's comment from some time ago when fixing some 
> related bugs 
> (http://marc.theaimsgroup.com/?l=tomcat-dev&m=106008981803343&w=2) that 
> this would be better if the spec mandated that the parameter passed to 
> getContext() must be an exact match for a context path.
>
> Having read the 2.4 spec several times I am pretty sure that is does say 
> this, albeit not as directly as it might. I assume (perhaps wrongly) that 
> any changes in this area will generate a lot of debate so I wanted to do 
> the debate and then change the code.
>

I can't really dispute the interpretation of the spec, but the change is 
likely to break many more webapps then it fixes :(.

For TC 5, it would be nice to keep some backwards compatibility with 
something like:

     MessageBytes uriMB = new MessageBytes();
     uriMB.setString(uri);
     MappingData md = new MappingData();
     MessageBytes serverName = request.getCoyoteRequest().serverName();
     request.getConnector().getMapper().map(serverName, uriMB, md);
     if(md.context == null) {
        return null;
     }
     return ((Context)md.context).getServletContext();



> The key parts of the spec are:
> SRV.14.2.8 ServletContext
> <snip>
> public ServletContext getContext(java.lang.String uripath)
> <snip>
> uripath - a String specifying the context path of another web application 
> in the container.
> <snip>
>
> My interpretation is:
> SRV.14.2.8 says the parameter is a context path
> SRV.4.4 is very clear about what is context path is
>
> Therefore, getContext() must look for an exact match of uripath against 
> the context paths for the currently deployed web-apps.
>
> My proposal, therefore is to change the getContext() implementation to 
> look for an exact match. This is stricter than the current implementation 
> (and may cause problems for some users) but will fix a number of odd 
> behaviours including the one described in bug 13040.
>
> In the unlikely event that no-one disagrees with my interpretation, I'll 
> commit a fix over the weekend to TC4 and TC5. (The 2.3 spec has the exact 
> same wording.)
>
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
> 



This message is intended only for the use of the person(s) listed above as the intended recipient(s), and may contain information that is PRIVILEGED and CONFIDENTIAL.  If you are not an intended recipient, you may not read, copy, or distribute this message or any attachment. If you received this communication in error, please notify us immediately by e-mail and then delete all copies of this message and any attachments.

In addition you should be aware that ordinary (unencrypted) e-mail sent through the Internet is not secure. Do not send confidential or sensitive information, such as social security numbers, account numbers, personal identification numbers and passwords, to us via ordinary (unencrypted) e-mail.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: getContext() - spec interpretation

Posted by Yoav Shapira <yo...@apache.org>.
Oddly enough, I was thinking about the same thing a couple of weeks
ago and tend to agree with your interpretation...

Yoav

On 11/24/05, Mark Thomas <ma...@apache.org> wrote:
> All,
>
> I have been looking at bug 13040 and reviewing the current
> getContext() implementation. I saw Remy's comment from some time ago
> when fixing some related bugs
> (http://marc.theaimsgroup.com/?l=tomcat-dev&m=106008981803343&w=2)
> that this would be better if the spec mandated that the parameter
> passed to getContext() must be an exact match for a context path.
>
> Having read the 2.4 spec several times I am pretty sure that is does
> say this, albeit not as directly as it might. I assume (perhaps
> wrongly) that any changes in this area will generate a lot of debate
> so I wanted to do the debate and then change the code.
>
> The key parts of the spec are:
> SRV.14.2.8 ServletContext
> <snip>
> public ServletContext getContext(java.lang.String uripath)
> <snip>
> uripath - a String specifying the context path of another web
> application in the container.
> <snip>
>
> My interpretation is:
> SRV.14.2.8 says the parameter is a context path
> SRV.4.4 is very clear about what is context path is
>
> Therefore, getContext() must look for an exact match of uripath
> against the context paths for the currently deployed web-apps.
>
> My proposal, therefore is to change the getContext() implementation to
> look for an exact match. This is stricter than the current
> implementation (and may cause problems for some users) but will fix a
> number of odd behaviours including the one described in bug 13040.
>
> In the unlikely event that no-one disagrees with my interpretation,
> I'll commit a fix over the weekend to TC4 and TC5. (The 2.3 spec has
> the exact same wording.)
>
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>


--
Yoav Shapira
System Design and Management Fellow
MIT Sloan School of Management
Cambridge, MA, USA
yoavs@computer.org / www.yoavshapira.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: getContext() - spec interpretation

Posted by Jan Luehe <Ja...@Sun.COM>.

Mark Thomas wrote On 11/30/05 13:42,:
> Jan,
> 
> This looks good to me.
> 
> I have patches ready to go for TC4 and TC5. They are based on the 
> existing code but implement the spirit of Bill's proposed solution and 
> add handling for crossContext.
> 
> I'll give people a little longer in case they want to add to this 
> discussion and then I'll commit them.

Sounds good!

Thanks, Mark!


Jan


> 
> Mark
> 
> Jan Luehe wrote:
> 
>>I've rewritten the javadocs of getContext(), as follows:
>>
>>    /**
>>     * Returns the <code>ServletContext</code> object to which the
>>     * specified URI has been mapped.
>>     *
>>     * <p>This method allows servlets to gain access to foreign servlet
>>     * contexts deployed on the same container, from which
>>     * {@link RequestDispatcher} objects may be obtained.
>>     * A servlet context is considered foreign to a servlet if it is
>>     * different from the servlet's own servlet context.
>>     *
>>     * <p>The given URI must be begin with "/".
>>     *
>>     * <p>A security conscious servlet container may return
>>     * <code>null</code> if the specified URI maps to a foreign context.
>>     *
>>     * @param uripath The URI to map
>>     *
>>     * @return The <code>ServletContext</code> to which the given URI
>>     * has been mapped, or null if the given URI does not map to any of
>>     * the servlet contexts deployed on the container, or if the URI
>>     * maps to a foreign context and access to foreign contexts has been
>>     * disabled by the container
>>     *
>>     * @see RequestDispatcher
>>     *
>>     */
>>    public ServletContext getContext(String uripath);
>>
>>
>>If we all agree, I will send this proposal to the Servlet EG.
>>
>>
>>Thanks,
>>
>>
>>Jan
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: getContext() - spec interpretation

Posted by Mark Thomas <ma...@apache.org>.
Jan,

This looks good to me.

I have patches ready to go for TC4 and TC5. They are based on the 
existing code but implement the spirit of Bill's proposed solution and 
add handling for crossContext.

I'll give people a little longer in case they want to add to this 
discussion and then I'll commit them.

Mark

Jan Luehe wrote:
> I've rewritten the javadocs of getContext(), as follows:
> 
>     /**
>      * Returns the <code>ServletContext</code> object to which the
>      * specified URI has been mapped.
>      *
>      * <p>This method allows servlets to gain access to foreign servlet
>      * contexts deployed on the same container, from which
>      * {@link RequestDispatcher} objects may be obtained.
>      * A servlet context is considered foreign to a servlet if it is
>      * different from the servlet's own servlet context.
>      *
>      * <p>The given URI must be begin with "/".
>      *
>      * <p>A security conscious servlet container may return
>      * <code>null</code> if the specified URI maps to a foreign context.
>      *
>      * @param uripath The URI to map
>      *
>      * @return The <code>ServletContext</code> to which the given URI
>      * has been mapped, or null if the given URI does not map to any of
>      * the servlet contexts deployed on the container, or if the URI
>      * maps to a foreign context and access to foreign contexts has been
>      * disabled by the container
>      *
>      * @see RequestDispatcher
>      *
>      */
>     public ServletContext getContext(String uripath);
> 
> 
> If we all agree, I will send this proposal to the Servlet EG.
> 
> 
> Thanks,
> 
> 
> Jan


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: getContext() - spec interpretation

Posted by Jan Luehe <Ja...@Sun.COM>.
Mark,

Mark Thomas wrote On 11/29/05 12:46,:
> Jan,
> 
> Jan Luehe wrote:
> 
>>Hi Mark,
> 
> <snip>
> 
>>I agree the current javadocs of ServletContext.getContext() are
>>ambiguous, and I agree we need to present this case to the
>>Servlet EG. Maybe there is a small chance to have this resolved
>>in the Servlet 2.5 timeframe ...
>>
>>I think it would be more useful to have the "uripath" argument be
>>interpreted as a full URI rather than a context root, and have
>>ServletContext.getContext() determine the longest match of the given
>>"uripath" with the context roots of any deployed contexts.
> 
> <snip>
>   I agree the current behaviour, which "hides" any foreign contexts
> 
>>whose context roots match a subdir of the current context, is broken
>>(in part because of the spec ambiguity), because it returns different
>>results depending on the context from which the getContext() call is
>>made.
>>
>>I don't see why we need to restrict "uripath" to be a context root in
>>order to fix the current behaviour, though. As I said, returning
>>the context whose context root has the longest match with the given
>>"uripath" may make more sense.
>>
>>Would you agree?
>>
>>
>>Jan
> 
> 
> I have just re-read the spec keeping score of number of times the 
> wording suggests an exact match or a longest match. The result was a 
> draw so I am happy to interpret it either way. Given that the longest 
> match makes the most sense to me and won't break any existing 
> functionality I will take this approach until the EG issues some 
> clarification. Hopefully they will clarify it the "right" way.

Makes sense.

So it seems we could apply the patch proposed by Bill to achieve the new
semantics of getContext():

     MessageBytes uriMB = new MessageBytes();
     uriMB.setString(uri);
     MappingData md = new MappingData();
     MessageBytes serverName = request.getCoyoteRequest().serverName();
     request.getConnector().getMapper().map(serverName, uriMB, md);
     if(md.context == null) {
        return null;
     }
     return ((Context)md.context).getServletContext();

I've rewritten the javadocs of getContext(), as follows:

    /**
     * Returns the <code>ServletContext</code> object to which the
     * specified URI has been mapped.
     *
     * <p>This method allows servlets to gain access to foreign servlet
     * contexts deployed on the same container, from which
     * {@link RequestDispatcher} objects may be obtained.
     * A servlet context is considered foreign to a servlet if it is
     * different from the servlet's own servlet context.
     *
     * <p>The given URI must be begin with "/".
     *
     * <p>A security conscious servlet container may return
     * <code>null</code> if the specified URI maps to a foreign context.
     *
     * @param uripath The URI to map
     *
     * @return The <code>ServletContext</code> to which the given URI
     * has been mapped, or null if the given URI does not map to any of
     * the servlet contexts deployed on the container, or if the URI
     * maps to a foreign context and access to foreign contexts has been
     * disabled by the container
     *
     * @see RequestDispatcher
     *
     */
    public ServletContext getContext(String uripath);


If we all agree, I will send this proposal to the Servlet EG.


Thanks,


Jan


> 
> Mark
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: getContext() - spec interpretation

Posted by Mark Thomas <ma...@apache.org>.
Jan,

Jan Luehe wrote:
> Hi Mark,
<snip>
> I agree the current javadocs of ServletContext.getContext() are
> ambiguous, and I agree we need to present this case to the
> Servlet EG. Maybe there is a small chance to have this resolved
> in the Servlet 2.5 timeframe ...
> 
> I think it would be more useful to have the "uripath" argument be
> interpreted as a full URI rather than a context root, and have
> ServletContext.getContext() determine the longest match of the given
> "uripath" with the context roots of any deployed contexts.
<snip>
  I agree the current behaviour, which "hides" any foreign contexts
> whose context roots match a subdir of the current context, is broken
> (in part because of the spec ambiguity), because it returns different
> results depending on the context from which the getContext() call is
> made.
> 
> I don't see why we need to restrict "uripath" to be a context root in
> order to fix the current behaviour, though. As I said, returning
> the context whose context root has the longest match with the given
> "uripath" may make more sense.
> 
> Would you agree?
> 
> 
> Jan

I have just re-read the spec keeping score of number of times the 
wording suggests an exact match or a longest match. The result was a 
draw so I am happy to interpret it either way. Given that the longest 
match makes the most sense to me and won't break any existing 
functionality I will take this approach until the EG issues some 
clarification. Hopefully they will clarify it the "right" way.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: getContext() - spec interpretation

Posted by Jan Luehe <Ja...@Sun.COM>.
Hi Mark,


Mark Thomas wrote On 11/24/05 11:34,:
> All,
> 
> I have been looking at bug 13040 and reviewing the current 
> getContext() implementation. I saw Remy's comment from some time ago 
> when fixing some related bugs 
> (http://marc.theaimsgroup.com/?l=tomcat-dev&m=106008981803343&w=2) 
> that this would be better if the spec mandated that the parameter 
> passed to getContext() must be an exact match for a context path.
> 
> Having read the 2.4 spec several times I am pretty sure that is does 
> say this, albeit not as directly as it might. I assume (perhaps 
> wrongly) that any changes in this area will generate a lot of debate 
> so I wanted to do the debate and then change the code.
> 
> The key parts of the spec are:
> SRV.14.2.8 ServletContext
> <snip>
> public ServletContext getContext(java.lang.String uripath)
> <snip>
> uripath - a String specifying the context path of another web 
> application in the container.
> <snip>
> 
> My interpretation is:
> SRV.14.2.8 says the parameter is a context path
> SRV.4.4 is very clear about what is context path is
> 
> Therefore, getContext() must look for an exact match of uripath 
> against the context paths for the currently deployed web-apps.
> 
> My proposal, therefore is to change the getContext() implementation to 
> look for an exact match. This is stricter than the current 
> implementation (and may cause problems for some users) but will fix a 
> number of odd behaviours including the one described in bug 13040.
> 
> In the unlikely event that no-one disagrees with my interpretation, 
> I'll commit a fix over the weekend to TC4 and TC5. (The 2.3 spec has 
> the exact same wording.)


I agree the current javadocs of ServletContext.getContext() are
ambiguous, and I agree we need to present this case to the
Servlet EG. Maybe there is a small chance to have this resolved
in the Servlet 2.5 timeframe ...

I think it would be more useful to have the "uripath" argument be
interpreted as a full URI rather than a context root, and have
ServletContext.getContext() determine the longest match of the given
"uripath" with the context roots of any deployed contexts.

Applied to the scenario described at:

  http://marc.theaimsgroup.com/?l=tomcat-dev&m=108109687130165&w=2

which assumes the following configured contexts:

  A: /
  B: /foo/
  C: /foo/bar/

the above interpretation would give the following consistent results:

  Context uripath         result
  ------------------------------
  A       "/"             A
  A       "/foo/"         B
  A       "/foo/bar/"     C
  A       "/foo/bar/xxx/" C

  B       "/"             A
  B       "/foo/"         B
  B       "/foo/bar/"     C
  B       "/foo/bar/xxx/" C

  C       "/"             A
  C       "/foo/"         B
  C       "/foo/bar/"     C
  C       "/foo/bar/xxx/" C

I agree the current behaviour, which "hides" any foreign contexts
whose context roots match a subdir of the current context, is broken
(in part because of the spec ambiguity), because it returns different
results depending on the context from which the getContext() call is
made.

I don't see why we need to restrict "uripath" to be a context root in
order to fix the current behaviour, though. As I said, returning
the context whose context root has the longest match with the given
"uripath" may make more sense.

Would you agree?


Jan

> 
> Mark
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org