struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Niall Pemberton" <niall.pember...@gmail.com>
Subject Re: svn commit: r553240 - /struts/struts1/trunk/core/src/main/java/org/apache/struts/upload/CommonsMultipartRequestHandler.java
Date Fri, 06 Jul 2007 05:11:50 GMT
On 7/6/07, Paul Benedict <pbenedict@apache.org> wrote:
> In the ticket, you will see my concerns were equal to his. But then I
> changed my mind because the concerned is founded on the false principle
> that fileuploads are somehow special. There is no difference between a
> large file upload and a long text string to Struts. You could send a
> large 2GB string to the validator and that could be considered a DoS --
> however, no one believes so. The problem is not with the solution, but
> with one's server configuration. All modern servers can be configured
> for a maximum packet size, so if there is a DoS, it's not in any one
> particular data point (text or file) in Struts, but with the server
> setting itself.

I see no discussion on FILEUPLOAD-140 with Jochen about this and that
would seem a more logical place to fix than here in Struts. If it has
merit then you should be able to convince him - or at least try. I'm
no expert on file upload or DoS, but my gut feel is its a hack to fix
a problem that has nothing to do with Struts - which we've generally
resisted in the past.

Niall

> Paul
>
> Niall Pemberton wrote:
> > I assume this is related to FILEUPLOAD-140[1] - Jochen points out on
> > that ticket that this could be used for a DOS attack - so this change
> > doesn't look like a good idea.
> >
> > Niall
> >
> > [1] https://issues.apache.org/jira/browse/FILEUPLOAD-140
> >
> > On 7/4/07, pbenedict@apache.org <pbenedict@apache.org> wrote:
> >> Author: pbenedict
> >> Date: Wed Jul  4 08:27:07 2007
> >> New Revision: 553240
> >>
> >> URL: http://svn.apache.org/viewvc?view=rev&rev=553240
> >> Log:
> >> STR-2700: Clear input stream on aborted upload
> >>
> >> Modified:
> >>
> >> struts/struts1/trunk/core/src/main/java/org/apache/struts/upload/CommonsMultipartRequestHandler.java
> >>
> >>
> >> Modified:
> >> struts/struts1/trunk/core/src/main/java/org/apache/struts/upload/CommonsMultipartRequestHandler.java
> >>
> >> URL:
> >> http://svn.apache.org/viewvc/struts/struts1/trunk/core/src/main/java/org/apache/struts/upload/CommonsMultipartRequestHandler.java?view=diff&rev=553240&r1=553239&r2=553240
> >>
> >> ==============================================================================
> >>
> >> ---
> >> struts/struts1/trunk/core/src/main/java/org/apache/struts/upload/CommonsMultipartRequestHandler.java
> >> (original)
> >> +++
> >> struts/struts1/trunk/core/src/main/java/org/apache/struts/upload/CommonsMultipartRequestHandler.java
> >> Wed Jul  4 08:27:07 2007
> >> @@ -33,6 +33,7 @@
> >>
> >>  import javax.servlet.ServletContext;
> >>  import javax.servlet.ServletException;
> >> +import javax.servlet.ServletInputStream;
> >>  import javax.servlet.http.HttpServletRequest;
> >>
> >>  import java.io.File;
> >> @@ -188,10 +189,11 @@
> >>              // Special handling for uploads that are too big.
> >>
> >> request.setAttribute(MultipartRequestHandler.ATTRIBUTE_MAX_LENGTH_EXCEEDED,
> >>
> >>                  Boolean.TRUE);
> >> -
> >> +            clearInputStream(request);
> >>              return;
> >>          } catch (FileUploadException e) {
> >>              log.error("Failed to parse multipart request", e);
> >> +            clearInputStream(request);
> >>              throw new ServletException(e);
> >>          }
> >>
> >> @@ -266,6 +268,23 @@
> >>      }
> >>
> >>      // --------------------------------------------------------
> >> Support Methods
> >> +
> >> +    /**
> >> +     * Finishes reading the input stream from an aborted upload. Fix
> >> for
> >> +     * STR-2700 to prevent Window machines from hanging.
> >> +     */
> >> +    protected void clearInputStream(HttpServletRequest request) {
> >> +        try {
> >> +            ServletInputStream is = request.getInputStream();
> >> +            byte[] data = new byte[DEFAULT_SIZE_THRESHOLD];
> >> +            int bytesRead = 0;
> >> +            do {
> >> +                bytesRead = is.read(data);
> >> +            } while (bytesRead > -1);
> >> +        } catch (Exception e) {
> >> +            log.error(e.getMessage(), e);
> >> +        }
> >> +    }
> >>
> >>      /**
> >>       * <p> Returns the maximum allowable size, in bytes, of an
> >> uploaded file.
> >>
> >>
> >>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> > For additional commands, e-mail: dev-help@struts.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


Mime
View raw message