Bug 13040 - can't retrieve external context who's uri is a sub-dir of current context
can't retrieve external context who's uri is a sub-dir of current context
Status: RESOLVED FIXED
Product: Tomcat 4
Classification: Unclassified
Component: Servlet & JSP API
4.1.12
All All
: P3 normal with 15 votes (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
: 10544 11652 11865 14114 16258 (view as bug list)
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2002-09-26 17:54 UTC by Ryan Smith
Modified: 2005-12-04 10:36 UTC (History)
6 users (show)



Attachments
proposed fix (diff) (283 bytes, patch)
2002-09-26 18:24 UTC, Ryan Smith
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Smith 2002-09-26 17:54:07 UTC
org.apache.catalina.core.ApplicationContext.getContext(String uri) method was 
written to return the current context whenever the uri passed into it was 
a "sub-dir" of the current context's uri.

for example, calling getContext("/store/common") from within a context that 
had as it's uri: "/store", would always return the context of "/store", even 
if "/store/common" was a valid context
Comment 1 Ryan Smith 2002-09-26 18:21:04 UTC
*** Bug 11865 has been marked as a duplicate of this bug. ***
Comment 2 Ryan Smith 2002-09-26 18:21:51 UTC
*** Bug 11652 has been marked as a duplicate of this bug. ***
Comment 3 Ryan Smith 2002-09-26 18:24:50 UTC
Created attachment 3247 [details]
proposed fix (diff)
Comment 4 Martin Algesten 2002-10-22 16:57:58 UTC
The problem is much more trivial. We will always have a contextPath with a
length > 0 since we always append "/" if there isn't one already.

$ diff -u ApplicationContext.java-2002-10-21 ApplicationContext.java
--- ApplicationContext.java-2002-10-21	Mon Sep 23 11:23:16 2002
+++ ApplicationContext.java	Tue Oct 22 17:55:08 2002
@@ -442,7 +442,7 @@
         String contextPath = context.getPath();
         if (!contextPath.endsWith("/"))
             contextPath = contextPath + "/";
-        if ((contextPath.length() > 0) && (uri.startsWith(contextPath))) {
+        if ((contextPath.length() > 1) && (uri.startsWith(contextPath))) {
             return (this);
         }
 
Martin Algesten
Comment 5 Remy Maucherat 2002-10-23 06:53:04 UTC
In the general case, "/store/common" is supposed to return context "/store" if
it exists.
The current algorithm has a big problem with the root context, as pointed out by
Martin. However, it actually has the same problem for all contexts, so I think
there's a spec problem here.
Comment 6 Martin Algesten 2002-10-23 10:08:45 UTC
The bit of code we're talking about is there in order to always allow for 
getting the current context, even if crossContext="false" (god knows why 
anyone would like to get the current context like that, but allowing it surely 
doesn't hurt).

I don't understand why we are adding on a trailing "/" in case the contextPath 
doesn't have one. That seems wrong to me since it would require the caller to 
say "/store/" in order to trigger the optimisation. The incoming uri we need 
to handle could be on the forms (have I missed anyone?):
"/"
"/store"
"/store/"
"/store/common"
"/store/common/even/more"

The context paths can be on the forms (again, correct me if I'm wrong):
""
"/store"

It seems we would need to treat the ROOT context differently to the rest of 
them. A propose for the fix would then be:

----------------------------------------------------------------------
--- ApplicationContext.java-2002-10-21  Mon Sep 23 11:23:16 2002
+++ ApplicationContext.java     Wed Oct 23 10:54:51 2002
@@ -439,13 +439,16 @@
             return (null);

         // Return the current context if requested
-        String contextPath = context.getPath();
-        if (!contextPath.endsWith("/"))
-            contextPath = contextPath + "/";
-        if ((contextPath.length() > 0) && (uri.startsWith(contextPath))) {
-            return (this);
-        }
-
+       if ( context.getPath().equals( "" ) ) {
+         if ( uri.equals( "/" ) ) {
+           return (this);
+         }
+       } else {
+         if ( uri.startsWith( context.getPath() ) ) {
+           return (this);
+         }
+       }
+
         // Return other contexts only if allowed
         if (!context.getCrossContext())
             return (null);
----------------------------------------------------------------------


I've tested this with the following JSP code:
<%
  ServletContext ctx = getServletContext();
  System.out.println( "/: "+ctx.getContext( "/" ) );
  System.out.println( "/publisher: "+ctx.getContext( "/publisher" ) );
  System.out.println( "/publisher/: "+ctx.getContext( "/publisher/" ) );
  System.out.println( "/publisher/protected: "+ctx.getContext
( "/publisher/protected" ) );
  System.out.println( "/publisher/protected/: "+ctx.getContext
( "/publisher/protected/" ) );
%>

With crossContext="false".

Running the JSP in the ROOT context gives me:
/: org.apache.catalina.core.ApplicationContextFacade@a45a24
/publisher: null
/publisher/: null
/publisher/protected: null
/publisher/protected/: null

Running the JSP in the /publisher context gives me:
/: null
/publisher: org.apache.catalina.core.ApplicationContextFacade@15c6c8d
/publisher/: org.apache.catalina.core.ApplicationContextFacade@15c6c8d
/publisher/protected: org.apache.catalina.core.ApplicationContextFacade@15c6c8d
/publisher/protected/: 
org.apache.catalina.core.ApplicationContextFacade@15c6c8d

Martin Algesten
Comment 7 Remy Maucherat 2002-10-23 12:41:34 UTC
The "/" is added to avoid matching stuff like:
Context name: "/foo"
Uri: "/foobar"

I still think the spec is bad, and should modify to specify that the desired
context path should be matched exactly.
BTW, there's a Watchdog test about this (unfortunately). I wonder why, since the
API Javadocs are quite unclear.
Comment 8 Martin Algesten 2002-10-23 13:18:08 UTC
Agreed, we need to add "/" to the end, but that goes for the incoming URI as 
well. We also need to treat the ROOT context differently. I agree the specs 
are bad, but we need to solve this with what we got, since the getContext() 
method as it currently stands isn't useful.

-----------------------------------------------------------
--- ApplicationContext.java-2002-10-21  Mon Sep 23 11:23:16 2002
+++ ApplicationContext.java     Wed Oct 23 14:08:41 2002
@@ -439,12 +439,25 @@
             return (null);

         // Return the current context if requested
-        String contextPath = context.getPath();
-        if (!contextPath.endsWith("/"))
-            contextPath = contextPath + "/";
-        if ((contextPath.length() > 0) && (uri.startsWith(contextPath))) {
-            return (this);
-        }
+       String contextPath = context.getPath();
+       if ( contextPath.equals( "" ) ) {
+         if ( uri.equals( "/" ) ) {
+           return (this);
+         }
+       } else {
+         String compareUri = uri;
+         if ( !uri.endsWith( "/" ) ) {
+           compareUri = compareUri+"/";
+         }
+         if ( !contextPath.endsWith( "/" ) ) {
+           contextPath = contextPath+"/";
+         }
+         if ( compareUri.startsWith( contextPath ) ) {
+           return (this);
+         }
+       }
+
+       System.out.println( context.getCrossContext() );

         // Return other contexts only if allowed
         if (!context.getCrossContext())
-----------------------------------------------------------

Martin Algesten
Comment 9 Ryan Smith 2002-10-23 21:07:55 UTC
ok, from the servlet 2.3 spec:

-----------------------------------------
getContext(String)
  public ServletContext getContext(java.lang.String uripath) 

    Returns a ServletContext object that corresponds to a specified URL on the
    server.
    This method allows servlets to gain access to the context for various 
parts of
    the server, and as needed obtain RequestDispatcher objects from the 
context.
    The given path must be begin with “/”, is interpreted relative to the
    server’s document root and is matched against the context roots of other 
web
    applications hosted on this container.
    In a security conscious environment, the servlet container may return null
    for a given URL.

    Parameters:
    uripath - a String specifying the context path of another web application 
in
    the container.

    Returns: the ServletContext object that corresponds to the named URL, or
    null if either none exists or the container wishes to restrict this access.
-----------------------------------------


There doesn't appear to be anything in here at all suggesting that when you 
call getContext(String) with a parameter specifying a uri which is a sub-dir 
of the current context's uri, it should ignore your request and simply return 
the current context.

I also beleive that the following assumption is incorrect: "In the general 
case, '/store/common' is supposed to return context '/store' if
it exists.".
 
  The spec states that the given path is "matched" against the context roots 
of other web apps.  That implies to me that it must match exactly.

The solutions provided here do not address this bug: "can't retrieve external 
context who's uri is a sub-dir of current context".  Each of them actually 
still has the same problem (except for the initial postings attachment).


Here's a solution, with minimal changes, that fixes this bug:


-----------------------------------------
445c445
<         if ((contextPath.length() > 0) && (uri.startsWith(contextPath))) {
---
>         if ((contextPath.length() > 1) && (uri.equals(contextPath))) {
-----------------------------------------


Ryan Smith
Comment 10 Martin Algesten 2002-10-24 09:33:43 UTC
That doesn't fix it properly. To trigger the optimisation you would need to send
in the uri "/store/" to get a match. Agreed that if we can ditch handling the
"/store/common" uri, we could make this work much easier.

--- ApplicationContext.java-2002-10-21	Mon Sep 23 11:23:16 2002
+++ ApplicationContext.java	Thu Oct 24 10:30:56 2002
@@ -439,12 +439,13 @@
             return (null);
 
         // Return the current context if requested
-        String contextPath = context.getPath();
-        if (!contextPath.endsWith("/"))
-            contextPath = contextPath + "/";
-        if ((contextPath.length() > 0) && (uri.startsWith(contextPath))) {
-            return (this);
-        }
+	String contextPath = context.getPath();
+	if ( contextPath.equals( "" ) && uri.equals( "/" ) ||
+	     !contextPath.equals( "" ) && uri.equals( contextPath ) ) {
+	  return (this);
+	}
+
+	System.out.println( context.getCrossContext() );
 
         // Return other contexts only if allowed
         if (!context.getCrossContext())

Martin Algesten
Comment 11 Martin Algesten 2002-10-30 16:41:53 UTC
That System.out should of course not be there.

--- ApplicationContext.java-2002-10-21  Mon Sep 23 11:23:16 2002
+++ ApplicationContext.java     Wed Oct 30 16:40:08 2002
@@ -439,12 +439,11 @@
             return (null);

         // Return the current context if requested
-        String contextPath = context.getPath();
-        if (!contextPath.endsWith("/"))
-            contextPath = contextPath + "/";
-        if ((contextPath.length() > 0) && (uri.startsWith(contextPath))) {
-            return (this);
-        }
+       String contextPath = context.getPath();
+       if ( contextPath.equals( "" ) && uri.equals( "/" ) ||
+            !contextPath.equals( "" ) && uri.equals( contextPath ) ) {
+         return (this);
+       }

         // Return other contexts only if allowed
         if (!context.getCrossContext())
Comment 12 Remy Maucherat 2002-10-31 05:44:11 UTC
*** Bug 14114 has been marked as a duplicate of this bug. ***
Comment 13 Christer Grimsaeth 2002-12-04 12:29:41 UTC
This is a serious bug that needs to be fixed.
I agree that the last patch submited by Martin Algesten lookes nice.

Please add it yo the CVS tree.
Thanks!

Christer Grimsæth
Comment 14 Martin Algesten 2003-01-16 13:57:56 UTC
*** Bug 10544 has been marked as a duplicate of this bug. ***
Comment 15 Remy Maucherat 2003-01-20 15:05:02 UTC
*** Bug 16258 has been marked as a duplicate of this bug. ***
Comment 16 Chrstian Wicke 2003-04-08 08:32:07 UTC
Is there a chance that this bug gets fixed within 2003? For us it's really a 
show stopper. 
Thanks a lot,
Christian
Comment 17 maa 2004-05-24 12:26:15 UTC
 Is this bug solved?
Comment 18 Mark Thomas 2004-05-24 18:18:18 UTC
Not completely no. The most recent debate on this is here:
http://marc.theaimsgroup.com/?l=tomcat-dev&m=108109687130165&w=2
Comment 19 Martin Gritsch 2005-02-04 09:39:53 UTC
Can you give us a short status about this bug ? the last entry is April '04 ?
Comment 20 Mark Thomas 2005-02-04 21:30:10 UTC
No change.

What is needed to move this forward is input from the servlet spec team on the
proposal in http://marc.theaimsgroup.com/?l=tomcat-dev&m=108109687130165&w=2

This is still on my todo list - just not going anywhere at the moment. If you
want to try and get some spec team input that would be great.
Comment 21 Mark Thomas 2005-12-04 19:36:59 UTC
After some discussion on what the spec actually means on the dev list, we have
agreed a way forward for this bug and I have just commited a change that fixes
this issue for TC4. A similar change will be ported to TC5.

The dev discussion can be found on this thread
http://marc.theaimsgroup.com/?l=tomcat-dev&m=113286089307014&w=2