You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by co...@locus.apache.org on 2000/01/12 20:54:04 UTC
cvs commit: jakarta-tomcat/src/share/org/apache/tomcat/util LocalStrings.properties
costin 00/01/12 11:54:03
Modified: src/share/org/apache/tomcat/core Context.java
ContextManager.java IncludedResponse.java
LocalStrings.properties Request.java
RequestDispatcherImpl.java
ServletContextFacade.java
src/share/org/apache/tomcat/server LocalStrings.properties
src/share/org/apache/tomcat/servlets InvokerServlet.java
src/share/org/apache/tomcat/util LocalStrings.properties
Log:
Cleaned-up RequestDispatcher !
I don't know if the previous model was better or not, but it was _very_ hard to
follow it.
When you call getRequestDispatcher a new Request ( used to be LookupResult, but it behaved the
same way ) is created. Now this request is used when invoking the included servlet,
instead of setting the old request with some of the parameters in the new request.
While the old way was probably faster, it required a number of hacks ( the included
request had special "magic" in it in form of special attributes).
Feel free to find a better way to implement include().
This will also clean up most of InvokerServlet ( which can move to a normal interceptor)
It seems all test are passing ( even with the incomplete implementation we have - that means
we need more tests, since the code is obviously broken !)
Revision Changes Path
1.28 +23 -23 jakarta-tomcat/src/share/org/apache/tomcat/core/Context.java
Index: Context.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/core/Context.java,v
retrieving revision 1.27
retrieving revision 1.28
diff -u -r1.27 -r1.28
--- Context.java 2000/01/12 06:35:20 1.27
+++ Context.java 2000/01/12 19:54:01 1.28
@@ -259,16 +259,29 @@
! path.startsWith("/")) {
return null; // spec say "return null if we can't return a dispather
}
- // Request subReq=context.getContextManager().createRequest( path );
- // RequestDispatcherImpl requestDispatcher = new RequestDispatcherImpl(subReq);
-
- RequestDispatcherImpl requestDispatcher = new RequestDispatcherImpl(this);
- requestDispatcher.setPath( path ) ;
+
+ Request subReq=contextM.createRequest( this, path );
+ contextM.processRequest(subReq);
- return requestDispatcher;
+ return new RequestDispatcherImpl(subReq);
}
-
+ public RequestDispatcher getNamedDispatcher(String name) {
+ if (name == null)
+ return null;
+
+ ServletWrapper wrapper = getServletByName( name );
+ if (wrapper == null)
+ return null;
+
+ // creates a new subrequest, and set the wrapper.
+ Request subR = new Request();
+ subR.setWrapper( wrapper );
+ subR.setPathInfo("");
+ subR.setContext( this );
+
+ return new RequestDispatcherImpl(subR);
+ }
/** Implements getResource() - use a sub-request to let interceptors do the job.
*/
@@ -296,8 +309,9 @@
Context getContext(String path) {
if (! path.startsWith("/")) {
- String msg = sm.getString("sfcacade.context.iae", path);
- throw new IllegalArgumentException(msg);
+ return null; // according to spec, null is returned
+ // if we can't return a servlet, so it's more probable
+ // servlets will check for null than IllegalArgument
}
return contextM.getContextByPath(path);
}
@@ -852,20 +866,6 @@
prefixMappedServlets.remove(mapping);
extensionMappedServlets.remove(mapping);
pathMappedServlets.remove(mapping);
- }
-
- Request lookupServletByName(String servletName) {
- Request lookupResult = null;
-
- ServletWrapper wrapper = (ServletWrapper)servlets.get(servletName);
-
- if (wrapper != null) {
- lookupResult = new Request();
- lookupResult.setWrapper( wrapper );
- lookupResult.setPathInfo("");
- }
-
- return lookupResult;
}
public ServletWrapper getServletByName(String servletName) {
1.15 +3 -1 jakarta-tomcat/src/share/org/apache/tomcat/core/ContextManager.java
Index: ContextManager.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/core/ContextManager.java,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -r1.14 -r1.15
--- ContextManager.java 2000/01/12 06:35:20 1.14
+++ ContextManager.java 2000/01/12 19:54:01 1.15
@@ -346,8 +346,10 @@
lr.setRequestAdapter( reqA);
lr.setLookupPath( urlPath );
lr.setQueryString( queryString );
- lr.setContext( ctx );
+ lr.processQueryString();
+ lr.setContext( ctx );
+
// XXX set query string too
return lr;
}
1.3 +4 -3 jakarta-tomcat/src/share/org/apache/tomcat/core/IncludedResponse.java
Index: IncludedResponse.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/core/IncludedResponse.java,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -r1.2 -r1.3
--- IncludedResponse.java 2000/01/11 02:06:53 1.2
+++ IncludedResponse.java 2000/01/12 19:54:01 1.3
@@ -1,7 +1,7 @@
/*
- * $Header: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/core/IncludedResponse.java,v 1.2 2000/01/11 02:06:53 costin Exp $
- * $Revision: 1.2 $
- * $Date: 2000/01/11 02:06:53 $
+ * $Header: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/core/IncludedResponse.java,v 1.3 2000/01/12 19:54:01 costin Exp $
+ * $Revision: 1.3 $
+ * $Date: 2000/01/12 19:54:01 $
*
* ====================================================================
*
@@ -90,6 +90,7 @@
}
public void sendError(int sc, String msg) throws IOException {
+ // /*XXX*/ try {throw new Exception(); } catch(Exception ex) {ex.printStackTrace();}
getRealResponse().sendBodyText("Included servlet error: " + sc +
": " + msg + "\r\n");
}
1.7 +7 -1 jakarta-tomcat/src/share/org/apache/tomcat/core/LocalStrings.properties
Index: LocalStrings.properties
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/core/LocalStrings.properties,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- LocalStrings.properties 2000/01/11 03:02:14 1.6
+++ LocalStrings.properties 2000/01/12 19:54:01 1.7
@@ -1,4 +1,4 @@
-# $Id: LocalStrings.properties,v 1.6 2000/01/11 03:02:14 costin Exp $
+# $Id: LocalStrings.properties,v 1.7 2000/01/12 19:54:01 costin Exp $
#
# Localized strings for package org.apache.tomcat.core
@@ -92,3 +92,9 @@
sc.503=Service Unavailable
sc.504=Gateway Timeout
sc.505=HTTP Version Not Supported
+
+servletOutputStreamImpl.write.iae=invalid write length: {0}
+servletOutputStreamImpl.fmt.not_iso8859_1=Not an ISO 8859_1 character:{0}
+servletOutputStreamImpl.reset.ise=can't reset buffer after writing to client
+servletOutputStreamImpl.setbuffer.ise=setting buffer after writing to the writer
+
1.19 +4 -3 jakarta-tomcat/src/share/org/apache/tomcat/core/Request.java
Index: Request.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/core/Request.java,v
retrieving revision 1.18
retrieving revision 1.19
diff -u -r1.18 -r1.19
--- Request.java 2000/01/12 06:35:20 1.18
+++ Request.java 2000/01/12 19:54:02 1.19
@@ -1,7 +1,7 @@
/*
- * $Header: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/core/Request.java,v 1.18 2000/01/12 06:35:20 costin Exp $
- * $Revision: 1.18 $
- * $Date: 2000/01/12 06:35:20 $
+ * $Header: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/core/Request.java,v 1.19 2000/01/12 19:54:02 costin Exp $
+ * $Revision: 1.19 $
+ * $Date: 2000/01/12 19:54:02 $
*
* ====================================================================
*
@@ -590,6 +590,7 @@
public void setPathInfo(String pathInfo) {
+ ///*DEBUG*/ try {throw new Exception(); } catch(Exception ex) {ex.printStackTrace();}
this.pathInfo = pathInfo;
}
1.10 +15 -126 jakarta-tomcat/src/share/org/apache/tomcat/core/RequestDispatcherImpl.java
Index: RequestDispatcherImpl.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/core/RequestDispatcherImpl.java,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -r1.9 -r1.10
--- RequestDispatcherImpl.java 2000/01/12 00:57:31 1.9
+++ RequestDispatcherImpl.java 2000/01/12 19:54:02 1.10
@@ -80,9 +80,6 @@
private Context context;
private Request subRequest = null;
- private String name = null;
- private String urlPath;
- private String queryString;
RequestDispatcherImpl(Context context) {
this.context = context;
@@ -98,6 +95,9 @@
Request realRequest = ((HttpServletRequestFacade)request).getRealRequest();
Response realResponse = ((HttpServletResponseFacade)response).getRealResponse();
+ String urlPath=realRequest.getLookupPath();
+ String queryString=realRequest.getQueryString();
+
// according to specs
if (realResponse.isStarted()) {
String msg = sm.getString("rdi.forward.ise");
@@ -118,137 +118,26 @@
}
public void include(ServletRequest request, ServletResponse response)
- throws ServletException, IOException {
+ throws ServletException, IOException
+ {
HttpServletRequest req = (HttpServletRequest)request;
-
- // XXX instead of setting the new parameters and back to original,
- // it should just use a sub-request ( by cloning the original )
- // That will simplify everything and make the code clear ( costin )
-
- //
- // XXX
- // while this appears to work i believe the code
- // could be streamlined/normalized a bit.
-
- // if we are in a chained include, then we'll store the attributes
- // from the last round so that we've got them for the next round
-
- String request_uri =
- (String)req.getAttribute(Constants.Attribute.RequestURI);
- String servlet_path =
- (String)req.getAttribute(Constants.Attribute.ServletPath);
- String path_info =
- (String)req.getAttribute(Constants.Attribute.PathInfo);
- String query_string =
- (String)req.getAttribute(Constants.Attribute.QueryString);
-
- HttpServletRequestFacade reqFacade =
- (HttpServletRequestFacade)request;
- HttpServletResponseFacade resFacade =
- (HttpServletResponseFacade)response;
- Request realRequest = reqFacade.getRealRequest();
- Response realResponse = resFacade.getRealResponse();
- String originalQueryString = realRequest.getQueryString();
- Hashtable originalParameters = realRequest.getParametersCopy();
-
-
- // XXX
- // not sure why we're pre-pending context.getPath() here
- //req.setAttribute(Constants.Attribute.RequestURI,
- // context.getPath() + urlPath);
-
- // XXX
- // added the "check for null" to get the named dispatcher
- // stuff working ... this might break something else
-
- if (urlPath != null) {
- req.setAttribute(Constants.Attribute.RequestURI,
- urlPath);
- }
-
- if (subRequest.getServletPath() != null) {
- req.setAttribute(Constants.Attribute.ServletPath,
- subRequest.getServletPath());
- }
-
- if (subRequest.getPathInfo() != null) {
- req.setAttribute(Constants.Attribute.PathInfo,
- subRequest.getPathInfo());
- }
+ Request realRequest = ((HttpServletRequestFacade)request).getRealRequest();
+ Response realResponse = ((HttpServletResponseFacade)response).getRealResponse();
// add new query string parameters to request
// if names are duplicates, new values will be prepended to arrays
- addQueryString( reqFacade.getRealRequest(), this.queryString );
-
- if (reqFacade.getRealRequest().getQueryString() != null) {
- req.setAttribute(Constants.Attribute.QueryString,
- reqFacade.getRealRequest().getQueryString());
- }
+ //XXX TODO addQueryString( reqFacade.getRealRequest(), this.queryString );
IncludedResponse iResponse = new IncludedResponse(realResponse);
-
- subRequest.getWrapper().handleRequest(reqFacade, iResponse);
-
- // revert the parameters and query string to its original value
- reqFacade.getRealRequest().setParameters(originalParameters);
- reqFacade.getRealRequest().setQueryString(originalQueryString);
- // XXX revert to the old parameters too - it was broken in the previous
- // model - it is still bad
- reqFacade.getRealRequest().processQueryString();
+ // XXX Make sure we "clone" all usefull informations
+ subRequest.setResponse( realResponse );
+ subRequest.setServerName( realRequest.getServerName() );
-
- if (request_uri != null) {
- req.setAttribute(Constants.Attribute.RequestURI, request_uri);
- } else {
- reqFacade.removeAttribute(Constants.Attribute.RequestURI);
+ try {
+ subRequest.getWrapper().handleRequest(subRequest.getFacade() , iResponse);
+ } catch( Exception ex) {
+ ex.printStackTrace();
}
-
- if (servlet_path != null) {
- req.setAttribute(Constants.Attribute.ServletPath,
- servlet_path);
- } else {
- reqFacade.removeAttribute(Constants.Attribute.ServletPath);
- }
-
- if (path_info != null) {
- req.setAttribute(Constants.Attribute.PathInfo, path_info);
- } else {
- reqFacade.removeAttribute(Constants.Attribute.PathInfo);
- }
-
- if (query_string != null) {
- req.setAttribute(Constants.Attribute.QueryString,
- query_string);
- } else {
- reqFacade.removeAttribute(Constants.Attribute.QueryString);
- }
- }
-
- void setName(String name) {
- this.name = name;
- this.subRequest =
- context.lookupServletByName(this.name);
- }
-
- void setPath(String urlPath) {
- // assert urlPath!=null
- int len=urlPath.length();
- int i = urlPath.indexOf("?");
-
- if (i>-1) {
- if(i<len)
- this.queryString =urlPath.substring(i + 1, urlPath.length());
- urlPath = urlPath.substring(0, i);
- }
-
- this.urlPath = urlPath;
-
- this.subRequest = context.getContextManager().createRequest(context, urlPath);
- context.getContextManager().processRequest(subRequest);
- }
-
- boolean isValid() {
- return (this.subRequest != null);
}
/**
1.15 +1 -11 jakarta-tomcat/src/share/org/apache/tomcat/core/ServletContextFacade.java
Index: ServletContextFacade.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/core/ServletContextFacade.java,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -r1.14 -r1.15
--- ServletContextFacade.java 2000/01/12 06:35:20 1.14
+++ ServletContextFacade.java 2000/01/12 19:54:02 1.15
@@ -154,17 +154,7 @@
}
public RequestDispatcher getNamedDispatcher(String name) {
- if (name == null) {
- String msg = sm.getString("scfacade.dispatcher.iae2", name);
- throw new IllegalArgumentException(msg);
- }
-
- RequestDispatcherImpl requestDispatcher =
- new RequestDispatcherImpl(context);
-
- requestDispatcher.setName(name);
-
- return (requestDispatcher.isValid()) ? requestDispatcher : null;
+ return context.getNamedDispatcher( name );
}
public String getServerInfo() {
1.4 +1 -6 jakarta-tomcat/src/share/org/apache/tomcat/server/LocalStrings.properties
Index: LocalStrings.properties
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/server/LocalStrings.properties,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- LocalStrings.properties 2000/01/08 14:50:40 1.3
+++ LocalStrings.properties 2000/01/12 19:54:03 1.4
@@ -1,4 +1,4 @@
-# $Id: LocalStrings.properties,v 1.3 2000/01/08 14:50:40 costin Exp $
+# $Id: LocalStrings.properties,v 1.4 2000/01/12 19:54:03 costin Exp $
#
# Localized strings for package org.apache.tomcat.server
@@ -16,11 +16,6 @@
server.defaultContext.docBase.npe=invalid context docBase
server.workDir.npe=work dir is null
-
-servletOutputStreamImpl.write.iae=invalid write length: {0}
-servletOutputStreamImpl.fmt.not_iso8859_1=Not an ISO 8859_1 character:{0}
-servletOutputStreamImpl.reset.ise=can't reset buffer after writing to client
-servletOutputStreamImpl.setbuffer.ise=setting buffer after writing to the writer
BufferedPrintWriter.setbuffer.ise=setting buffer after writing to the out
BufferedPrintWriter.reset.ise=can't reset buffer after writing to client
1.2 +22 -9 jakarta-tomcat/src/share/org/apache/tomcat/servlets/InvokerServlet.java
Index: InvokerServlet.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/servlets/InvokerServlet.java,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- InvokerServlet.java 2000/01/11 03:02:15 1.1
+++ InvokerServlet.java 2000/01/12 19:54:03 1.2
@@ -87,9 +87,9 @@
context = facade.getRealContext();
}
- public void service(HttpServletRequest request,
- HttpServletResponse response)
- throws ServletException, IOException {
+ public void service(HttpServletRequest request,HttpServletResponse response)
+ throws ServletException, IOException
+ {
String requestPath = request.getRequestURI();
String pathInfo = (String)request.getAttribute(
Constants.Attribute.PathInfo);
@@ -102,6 +102,7 @@
Constants.Attribute.RequestURI);
boolean inInclude = false;
+ // XXX XXX XXX in the new model we are _never_ inInclude
if (includedRequestURI != null) {
inInclude = true;
} else {
@@ -199,12 +200,21 @@
// request dispatcher forwards through the invoker. This is
// some seriously sick code here that needs to be done
// better, but this will do the trick for now.
-
- String savedServletPath = (String)realRequest.getAttribute(
- Constants.Attribute.ServletPath);
- String savedPathInfo = (String)realRequest.getAttribute(
- Constants.Attribute.PathInfo);
+ String savedServletPath=null;
+ String savedPathInfo =null;
+
+ // XXX XXX XXX need to be removed after the include hacks are out
+ if( ! inInclude ) {
+ savedPathInfo=realRequest.getPathInfo();
+ savedServletPath=realRequest.getServletPath();
+ } else {
+ savedServletPath = (String)realRequest.getAttribute(
+ Constants.Attribute.ServletPath);
+ savedPathInfo = (String)realRequest.getAttribute(
+ Constants.Attribute.PathInfo);
+ }
+
if (! inInclude) {
realRequest.setServletPath(newServletPath);
realRequest.setPathInfo(newPathInfo);
@@ -229,7 +239,10 @@
wrapper.handleRequest(requestfacade, responsefacade);
- if (inInclude) {
+ if (!inInclude) {
+ realRequest.setServletPath( savedServletPath);
+ realRequest.setPathInfo(savedPathInfo);
+ } else {
if (savedServletPath != null) {
realRequest.setAttribute(
Constants.Attribute.ServletPath, savedServletPath);
1.3 +2 -2 jakarta-tomcat/src/share/org/apache/tomcat/util/LocalStrings.properties
Index: LocalStrings.properties
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/util/LocalStrings.properties,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -r1.2 -r1.3
--- LocalStrings.properties 1999/10/24 00:08:03 1.2
+++ LocalStrings.properties 2000/01/12 19:54:03 1.3
@@ -1,11 +1,11 @@
-# $Id: LocalStrings.properties,v 1.2 1999/10/24 00:08:03 craigmcc Exp $
+# $Id: LocalStrings.properties,v 1.3 2000/01/12 19:54:03 costin Exp $
#
# Copyright Statement
# Localized strings for package org.apache.tomcat.util
# This is the default locale and is en_US
-ascii.parseInt.nfe=number formatting error {0}
+ascii.parseInit.nfe=number formatting error {0}
hexUtil.bad=Bad hexadecimal digit
hexUtil.odd=Odd number of hexadecimal digits
httpDate.pe=invalid date format: {0}
Re: cvs commit: jakarta-tomcat/src/share/org/apache/tomcat/util
LocalStrings.properties
Posted by Hans Bergsten <ha...@gefionsoftware.com>.
costin@locus.apache.org wrote:
>
> costin 00/01/12 11:54:03
>
> Modified: src/share/org/apache/tomcat/core Context.java
> ContextManager.java IncludedResponse.java
> LocalStrings.properties Request.java
> RequestDispatcherImpl.java
> ServletContextFacade.java
> src/share/org/apache/tomcat/server LocalStrings.properties
> src/share/org/apache/tomcat/servlets InvokerServlet.java
> src/share/org/apache/tomcat/util LocalStrings.properties
> Log:
> Cleaned-up RequestDispatcher !
>
> I don't know if the previous model was better or not, but it was _very_ hard to
> follow it.
I agree, thanks for cleaning it up. But does the new version handle merging
of parameters (both body and query string) in the new Request used by the RD?
I have not tested, but from the diffs it doesn't look like that's the case.
Hans
--
Hans Bergsten hans@gefionsoftware.com
Gefion Software http://www.gefionsoftware.com