Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector RequestFacade.java LocalStrings.properties
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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector RequestFacade.java LocalStrings.properties
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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector RequestFacade.java LocalStrings.properties
Bill Barker wrote: > - Original Message - > From: "Jan Luehe" <[EMAIL PROTECTED]> > To: "Tomcat Developers List" <[EMAIL PROTECTED]> > 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: >> >>>[EMAIL PROTECTED] 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector RequestFacade.java LocalStrings.properties
- Original Message - From: "Jan Luehe" <[EMAIL PROTECTED]> To: "Tomcat Developers List" <[EMAIL PROTECTED]> 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: > > [EMAIL PROTECTED] 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: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector RequestFacade.java LocalStrings.properties
Remy Maucherat wrote: > [EMAIL PROTECTED] 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector RequestFacade.java LocalStrings.properties
[EMAIL PROTECTED] 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
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 ChangesPath
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.java1 Nov 2004 22:22:37 - 1.6
+++ RequestFacade.java1 Nov 2004 22:38:44 - 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
