You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by lu...@apache.org on 2004/11/01 23:38:44 UTC
cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector RequestFacade.java LocalStrings.properties
luehe 2004/11/01 14:38:44
Modified: catalina/src/share/org/apache/catalina/connector
RequestFacade.java LocalStrings.properties
Log:
Throw more meaningful exception (instead of NPE) if underlying request has been
recycled and attempt is made to access it via its facade
Revision Changes Path
1.7 +335 -9 jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/RequestFacade.java
Index: RequestFacade.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/RequestFacade.java,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- RequestFacade.java 1 Nov 2004 22:22:37 -0000 1.6
+++ RequestFacade.java 1 Nov 2004 22:38:44 -0000 1.7
@@ -31,6 +31,8 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;
+import org.apache.catalina.util.StringManager;
+
/**
* Facade class that wraps a Coyote request object.
@@ -42,9 +44,7 @@
* @version $Revision$ $Date$
*/
-
-public class RequestFacade
- implements HttpServletRequest {
+public class RequestFacade implements HttpServletRequest {
// ----------------------------------------------------------- DoPrivileged
@@ -218,6 +218,13 @@
protected Request request = null;
+ /**
+ * The string manager for this package.
+ */
+ protected static StringManager sm =
+ StringManager.getManager(Constants.Package);
+
+
// --------------------------------------------------------- Public Methods
@@ -233,11 +240,23 @@
public Object getAttribute(String name) {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getAttribute(name);
}
public Enumeration getAttributeNames() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
if (System.getSecurityManager() != null){
return (Enumeration)AccessController.doPrivileged(
new GetAttributePrivilegedAction());
@@ -248,6 +267,12 @@
public String getCharacterEncoding() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
if (System.getSecurityManager() != null){
return (String)AccessController.doPrivileged(
new GetCharacterEncodingPrivilegedAction());
@@ -258,28 +283,57 @@
public void setCharacterEncoding(String env)
- throws java.io.UnsupportedEncodingException {
+ throws java.io.UnsupportedEncodingException {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
request.setCharacterEncoding(env);
}
public int getContentLength() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getContentLength();
}
public String getContentType() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getContentType();
}
- public ServletInputStream getInputStream()
- throws IOException {
+ public ServletInputStream getInputStream() throws IOException {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getInputStream();
}
public String getParameter(String name) {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
if (System.getSecurityManager() != null){
return (String)AccessController.doPrivileged(
new GetParameterPrivilegedAction(name));
@@ -290,6 +344,12 @@
public Enumeration getParameterNames() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
if (System.getSecurityManager() != null){
return (Enumeration)AccessController.doPrivileged(
new GetParameterNamesPrivilegedAction());
@@ -301,6 +361,11 @@
public String[] getParameterValues(String name) {
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
String[] ret = null;
/*
@@ -322,6 +387,12 @@
public Map getParameterMap() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
if (System.getSecurityManager() != null){
return (Map)AccessController.doPrivileged(
new GetParameterMapPrivilegedAction());
@@ -332,52 +403,111 @@
public String getProtocol() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getProtocol();
}
public String getScheme() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getScheme();
}
public String getServerName() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getServerName();
}
public int getServerPort() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getServerPort();
}
- public BufferedReader getReader()
- throws IOException {
+ public BufferedReader getReader() throws IOException {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getReader();
}
public String getRemoteAddr() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getRemoteAddr();
}
public String getRemoteHost() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getRemoteHost();
}
public void setAttribute(String name, Object o) {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
request.setAttribute(name, o);
}
public void removeAttribute(String name) {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
request.removeAttribute(name);
}
public Locale getLocale() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
if (System.getSecurityManager() != null){
return (Locale)AccessController.doPrivileged(
new GetLocalePrivilegedAction());
@@ -388,6 +518,12 @@
public Enumeration getLocales() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
if (System.getSecurityManager() != null){
return (Enumeration)AccessController.doPrivileged(
new GetLocalesPrivilegedAction());
@@ -398,11 +534,23 @@
public boolean isSecure() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.isSecure();
}
public RequestDispatcher getRequestDispatcher(String path) {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
if (System.getSecurityManager() != null){
return (RequestDispatcher)AccessController.doPrivileged(
new GetRequestDispatcherPrivilegedAction(path));
@@ -413,17 +561,34 @@
public String getRealPath(String path) {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getRealPath(path);
}
public String getAuthType() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getAuthType();
}
public Cookie[] getCookies() {
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
Cookie[] ret = null;
/*
@@ -445,16 +610,34 @@
public long getDateHeader(String name) {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getDateHeader(name);
}
public String getHeader(String name) {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getHeader(name);
}
public Enumeration getHeaders(String name) {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
if (System.getSecurityManager() != null){
return (Enumeration)AccessController.doPrivileged(
new GetHeadersPrivilegedAction(name));
@@ -465,6 +648,12 @@
public Enumeration getHeaderNames() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
if (System.getSecurityManager() != null){
return (Enumeration)AccessController.doPrivileged(
new GetHeaderNamesPrivilegedAction());
@@ -475,72 +664,155 @@
public int getIntHeader(String name) {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getIntHeader(name);
}
public String getMethod() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getMethod();
}
public String getPathInfo() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getPathInfo();
}
public String getPathTranslated() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getPathTranslated();
}
public String getContextPath() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getContextPath();
}
public String getQueryString() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getQueryString();
}
public String getRemoteUser() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getRemoteUser();
}
public boolean isUserInRole(String role) {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.isUserInRole(role);
}
public java.security.Principal getUserPrincipal() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getUserPrincipal();
}
public String getRequestedSessionId() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getRequestedSessionId();
}
public String getRequestURI() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getRequestURI();
}
public StringBuffer getRequestURL() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getRequestURL();
}
public String getServletPath() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getServletPath();
}
public HttpSession getSession(boolean create) {
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
if (System.getSecurityManager() != null){
return (HttpSession)AccessController.
doPrivileged(new GetSessionPrivilegedAction(create));
@@ -550,46 +822,100 @@
}
public HttpSession getSession() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return getSession(true);
}
public boolean isRequestedSessionIdValid() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.isRequestedSessionIdValid();
}
public boolean isRequestedSessionIdFromCookie() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.isRequestedSessionIdFromCookie();
}
public boolean isRequestedSessionIdFromURL() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.isRequestedSessionIdFromURL();
}
public boolean isRequestedSessionIdFromUrl() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.isRequestedSessionIdFromURL();
}
public String getLocalAddr() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getLocalAddr();
}
public String getLocalName() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getLocalName();
}
public int getLocalPort() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getLocalPort();
}
public int getRemotePort() {
+
+ if (request == null) {
+ throw new IllegalStateException(
+ sm.getString("requestFacade.nullRequest"));
+ }
+
return request.getRemotePort();
}
1.4 +1 -1 jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/LocalStrings.properties
Index: LocalStrings.properties
===================================================================
RCS file: /home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/LocalStrings.properties,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- LocalStrings.properties 23 Jun 2004 08:24:57 -0000 1.3
+++ LocalStrings.properties 1 Nov 2004 22:38:44 -0000 1.4
@@ -44,7 +44,7 @@
coyoteRequest.listenerStop=Exception sending context destroyed event to listener instance of class {0}
coyoteRequest.attributeEvent=Exception thrown by attributes event listener
coyoteRequest.postTooLarge=Parameters were not parsed because the size of the posted data was too big. Use the maxPostSize attribute of the connector to resolve this if the application should accept large POSTs.
-
+requestFacade.nullRequest=Null request object
#
# MapperListener
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector
RequestFacade.java LocalStrings.properties
Posted by Costin Manolache <cm...@yahoo.com>.
Remy Maucherat wrote:
> Jan Luehe wrote:
>
>> Bill Barker wrote:
>>
>>
>> In the case I was referring to, some project was storing a
>> servlet request (facade) in a ThreadLocal and, due to a bug in their
>> code, was hanging on to it beyond the request's lifetime. This was
>> happening only under rare circumstances.
>>
>>
> Great use case. And the talented programmer who coded that "thing"
> couldn't figure out what the problem was quickly enough without a proper
> error message, I assume ;)
>
>> So in this case, the Request behind the facade was indeed null.
>>
>>
> So now he'll get an error message saying "null request". Actually, it
> sounds like one of these useless M$ error messages.
>
> Lol and +1 for Bill's suggestion to change text to "if you're reading
> this, you're a moron" :D
As long as the exception doesn't get into the web browser of some user :-)
Costin
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector
RequestFacade.java LocalStrings.properties
Posted by Remy Maucherat <re...@apache.org>.
Jan Luehe wrote:
>Bill Barker wrote:
>
>
>In the case I was referring to, some project was storing a
>servlet request (facade) in a ThreadLocal and, due to a bug in their
>code, was hanging on to it beyond the request's lifetime. This was
>happening only under rare circumstances.
>
>
Great use case. And the talented programmer who coded that "thing"
couldn't figure out what the problem was quickly enough without a proper
error message, I assume ;)
>So in this case, the Request behind the facade was indeed null.
>
>
So now he'll get an error message saying "null request". Actually, it
sounds like one of these useless M$ error messages.
Lol and +1 for Bill's suggestion to change text to "if you're reading
this, you're a moron" :D
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit:
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector
RequestFacade.java LocalStrings.properties
Posted by Jan Luehe <Ja...@Sun.COM>.
Bill Barker wrote:
> ----- Original Message -----
> From: "Jan Luehe" <Ja...@Sun.COM>
> To: "Tomcat Developers List" <to...@jakarta.apache.org>
> Sent: Monday, November 01, 2004 3:41 PM
> Subject: Re: cvs commit:
> jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector
> RequestFacade.java LocalStrings.properties
>
>
>
>>Remy Maucherat wrote:
>>
>>>luehe@apache.org wrote:
>>>
>>>
>>>
>>>>luehe 2004/11/01 14:38:44
>>>>
>>>>Modified: catalina/src/share/org/apache/catalina/connector
>>>> RequestFacade.java LocalStrings.properties
>>>>Log:
>>>>Throw more meaningful exception (instead of NPE) if underlying request
>
> has been
>
>>>>recycled and attempt is made to access it via its facade
>>>>
>>>
>>>I think I always consistently refused this change (no use: if people who
>>>hack can't be bothered to look that up in the code, then I don't think
>>>they'll understand what your exception really means either), but I'll
>>>give up on that one.
>>
>>In this case, it's useful because rather than instinctively filing a bug
>>against Tomcat when seeing a NPE, people will be reminded to check their
>>code first, because they're obviously using Tomcat in a way it was not
>>intended to be used.
>>
>
>
> I agree with Remy: It's totally unnecessary, and gives somebody reading the
> code that the request can be null. The javadocs should probably be updated
> with something like:
> * @exception IllegalStateException If you are a total moron without a clue
> ;-)
In the case I was referring to, some project was storing a
servlet request (facade) in a ThreadLocal and, due to a bug in their
code, was hanging on to it beyond the request's lifetime. This was
happening only under rare circumstances.
So in this case, the Request behind the facade was indeed null.
Jan
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector RequestFacade.java LocalStrings.properties
Posted by Bill Barker <wb...@wilshire.com>.
----- Original Message -----
From: "Jan Luehe" <Ja...@Sun.COM>
To: "Tomcat Developers List" <to...@jakarta.apache.org>
Sent: Monday, November 01, 2004 3:41 PM
Subject: Re: cvs commit:
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector
RequestFacade.java LocalStrings.properties
> Remy Maucherat wrote:
> > luehe@apache.org wrote:
> >
> >
> >>luehe 2004/11/01 14:38:44
> >>
> >> Modified: catalina/src/share/org/apache/catalina/connector
> >> RequestFacade.java LocalStrings.properties
> >> Log:
> >> Throw more meaningful exception (instead of NPE) if underlying request
has been
> >> recycled and attempt is made to access it via its facade
> >>
> >
> > I think I always consistently refused this change (no use: if people who
> > hack can't be bothered to look that up in the code, then I don't think
> > they'll understand what your exception really means either), but I'll
> > give up on that one.
>
> In this case, it's useful because rather than instinctively filing a bug
> against Tomcat when seeing a NPE, people will be reminded to check their
> code first, because they're obviously using Tomcat in a way it was not
> intended to be used.
>
I agree with Remy: It's totally unnecessary, and gives somebody reading the
code that the request can be null. The javadocs should probably be updated
with something like:
* @exception IllegalStateException If you are a total moron without a clue
;-)
> We just ran into this internally.
>
>
> Jan
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.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.
Re: cvs commit:
jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector
RequestFacade.java LocalStrings.properties
Posted by Jan Luehe <Ja...@Sun.COM>.
Remy Maucherat wrote:
> luehe@apache.org wrote:
>
>
>>luehe 2004/11/01 14:38:44
>>
>> Modified: catalina/src/share/org/apache/catalina/connector
>> RequestFacade.java LocalStrings.properties
>> Log:
>> Throw more meaningful exception (instead of NPE) if underlying request has been
>> recycled and attempt is made to access it via its facade
>>
>
> I think I always consistently refused this change (no use: if people who
> hack can't be bothered to look that up in the code, then I don't think
> they'll understand what your exception really means either), but I'll
> give up on that one.
In this case, it's useful because rather than instinctively filing a bug
against Tomcat when seeing a NPE, people will be reminded to check their
code first, because they're obviously using Tomcat in a way it was not
intended to be used.
We just ran into this internally.
Jan
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector
RequestFacade.java LocalStrings.properties
Posted by Remy Maucherat <re...@apache.org>.
luehe@apache.org wrote:
>luehe 2004/11/01 14:38:44
>
> Modified: catalina/src/share/org/apache/catalina/connector
> RequestFacade.java LocalStrings.properties
> Log:
> Throw more meaningful exception (instead of NPE) if underlying request has been
> recycled and attempt is made to access it via its facade
>
I think I always consistently refused this change (no use: if people who
hack can't be bothered to look that up in the code, then I don't think
they'll understand what your exception really means either), but I'll
give up on that one.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org