Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/coyote/tomcat5 CoyoteConnector.java CoyoteRequest.java LocalStrings.properties

2003-11-16 Thread Remy Maucherat
Bill Barker wrote:
remm2003/11/15 01:45:02

 Modified:catalina/src/share/org/apache/coyote/tomcat5
   CoyoteConnector.java CoyoteRequest.java
   LocalStrings.properties
 Log:
 - Add a limit to the size of a POST which will be processed using
getParameter

   (which does allocate a significant amount of objects).

  int len = getContentLength();

  if (len  0) {
 +if (len  ((CoyoteConnector) connector).getMaxPostSize()) {
 +log(sm.getString(coyoteRequest.postTooLarge));
 +return;
 +}
  try {
  byte[] formData = null;
  if (len  CACHED_POST_LEN) {


I'm -1 on this.  It gives the Servlet no indication that anything is wrong,
and leaves junk in the input stream for the next keep-alive request.  I
think that it would be better to handle this in the Adaptor's
postParseRequest method.
This is a security issues, so I'll have to ignore your -1, since you 
give no alternative solution.

Remy



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/coyote/tomcat5 CoyoteConnector.java CoyoteRequest.java LocalStrings.properties

2003-11-16 Thread Remy Maucherat
Bill Barker wrote:
I'm -1 on this.  It gives the Servlet no indication that anything is wrong,
and leaves junk in the input stream for the next keep-alive request.  I
think that it would be better to handle this in the Adaptor's
postParseRequest method.
I disagree: the problem with this is that we allocate one array right 
away based on content-length, not that there's a big upload (which is fine).

Obviously, keep-alive is not an option in that case (and Tomcat will 
attempt to swallow any remaining output).

Remy



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/coyote/tomcat5 CoyoteConnector.java CoyoteRequest.java LocalStrings.properties

2003-11-16 Thread Remy Maucherat
Bill Barker wrote:
I'm -1 on this.  It gives the Servlet no indication that anything is wrong,
and leaves junk in the input stream for the next keep-alive request.  I
think that it would be better to handle this in the Adaptor's
postParseRequest method.
One last thing I forgot: you can't do it beforehand, because you don't 
know if the servlet is going to use getParameterXXX.
So I don't agree on putting this check in postParseRequest.

Remy



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/coyote/tomcat5 CoyoteConnector.java CoyoteRequest.java LocalStrings.properties

2003-11-16 Thread Bill Barker

- Original Message - 
From: Remy Maucherat [EMAIL PROTECTED]
To: Tomcat Developers List [EMAIL PROTECTED]
Sent: Sunday, November 16, 2003 2:34 AM
Subject: Re: cvs commit:
jakarta-tomcat-catalina/catalina/src/share/org/apache/coyote/tomcat5
CoyoteConnector.java CoyoteRequest.java LocalStrings.properties


 Bill Barker wrote:
  I'm -1 on this.  It gives the Servlet no indication that anything is
wrong,
  and leaves junk in the input stream for the next keep-alive request.  I
  think that it would be better to handle this in the Adaptor's
  postParseRequest method.

 I disagree: the problem with this is that we allocate one array right
 away based on content-length, not that there's a big upload (which is
fine).


The Adaptor can check for this:
  if(POST.equalsIgnoreCase(request.getMethod()) 
 application/x-www-form-urlencoded.equals(contentType) 
 request.getContentLength()  connector.getMaxPostSize()) {
  response.setStatus(413);
  response.setMessage(POST Body exceeds maximum);
  return false;
   }


 Obviously, keep-alive is not an option in that case (and Tomcat will
 attempt to swallow any remaining output).

 Remy



 -
 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/coyote/tomcat5 CoyoteConnector.java CoyoteRequest.java LocalStrings.properties

2003-11-16 Thread Remy Maucherat
Bill Barker wrote:
Bill Barker wrote:

I'm -1 on this.  It gives the Servlet no indication that anything is
wrong,

and leaves junk in the input stream for the next keep-alive request.  I
think that it would be better to handle this in the Adaptor's
postParseRequest method.
I disagree: the problem with this is that we allocate one array right
away based on content-length, not that there's a big upload (which is
fine).

The Adaptor can check for this:
  if(POST.equalsIgnoreCase(request.getMethod()) 
 application/x-www-form-urlencoded.equals(contentType) 
 request.getContentLength()  connector.getMaxPostSize()) {
  response.setStatus(413);
  response.setMessage(POST Body exceeds maximum);
  return false;
   }
This is not very efficient (doing checks there as well as in parse 
parameters), and what if the servlet intended to stream the body by 
itself ? (in this case, there's no particular efficiency problem)
So that's why I did do the check lazily (like the parsing itself, which 
must be lazy).

Remy



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/coyote/tomcat5 CoyoteConnector.java CoyoteRequest.java LocalStrings.properties

2003-11-15 Thread Tim Funk
I like this idea. But if the client sends too much data, it appears there 
will be no post data for the Servlet/JSP to process causing strange user 
errors. (User: Why is getParameter(...) null?)

Should an error of SC_REQUEST_ENTITY_TOO_LARGE be sent instead?  Of course, 
checking this too early before the user has chance to use a reader or input 
stream could also cause problems.

I guess I don't know what I want, but I wonder what to say in the future in 
case the assumption above is correct and I see a tomcat-user post about this.

Would it also be reasonable to allow a value of 0 (or -1) to disable this 
feature?

-Tim

[EMAIL PROTECTED] wrote:

remm2003/11/15 01:45:02

  Modified:catalina/src/share/org/apache/coyote/tomcat5
CoyoteConnector.java CoyoteRequest.java
LocalStrings.properties
  Log:
  - Add a limit to the size of a POST which will be processed using getParameter
(which does allocate a significant amount of objects).
  


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/coyote/tomcat5 CoyoteConnector.java CoyoteRequest.java LocalStrings.properties

2003-11-15 Thread Remy Maucherat
Tim Funk wrote:
I like this idea. But if the client sends too much data, it appears 
there will be no post data for the Servlet/JSP to process causing 
strange user errors. (User: Why is getParameter(...) null?)

Should an error of SC_REQUEST_ENTITY_TOO_LARGE be sent instead?  Of 
course, checking this too early before the user has chance to use a 
reader or input stream could also cause problems.
For now, the error is logged, and that's it.

I guess I don't know what I want, but I wonder what to say in the future 
in case the assumption above is correct and I see a tomcat-user post 
about this.

Would it also be reasonable to allow a value of 0 (or -1) to disable 
this feature?
-1 for unlimited sounds good to me. I should have tought about that. 
Good catch.

Remy

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


Re: cvs commit: jakarta-tomcat-catalina/catalina/src/share/org/apache/coyote/tomcat5 CoyoteConnector.java CoyoteRequest.java LocalStrings.properties

2003-11-15 Thread Bill Barker

- Original Message - 
From: [EMAIL PROTECTED]
To: [EMAIL PROTECTED]
Sent: Saturday, November 15, 2003 1:45 AM
Subject: cvs commit:
jakarta-tomcat-catalina/catalina/src/share/org/apache/coyote/tomcat5
CoyoteConnector.java CoyoteRequest.java LocalStrings.properties


 remm2003/11/15 01:45:02

   Modified:catalina/src/share/org/apache/coyote/tomcat5
 CoyoteConnector.java CoyoteRequest.java
 LocalStrings.properties
   Log:
   - Add a limit to the size of a POST which will be processed using
getParameter
 (which does allocate a significant amount of objects).

int len = getContentLength();

if (len  0) {
   +if (len  ((CoyoteConnector) connector).getMaxPostSize()) {
   +log(sm.getString(coyoteRequest.postTooLarge));
   +return;
   +}
try {
byte[] formData = null;
if (len  CACHED_POST_LEN) {


I'm -1 on this.  It gives the Servlet no indication that anything is wrong,
and leaves junk in the input stream for the next keep-alive request.  I
think that it would be better to handle this in the Adaptor's
postParseRequest method.


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]