Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.1-incubating-core-SNAPSHOT
    • Fix Version/s: 1.0.8-core, 1.2.8-core
    • Component/s: None
    • Labels:
      None
    • Environment:
      All, commons-upload

      Description

      Trinidad/commons-upload assume some memory and space limits for uploads which can be configured in /WEB-INF/web.xml:
      -----------
      <context-param>
      <param-name>org.apache.myfaces.trinidad.UPLOAD_MAX_MEMORY</param-name>
      <param-value>512000</param-value>
      </context-param>

      <context-param>
      <param-name>org.apache.myfaces.trinidad.UPLOAD_MAX_DISK_SPACE</param-name>
      <param-value>5120000</param-value>
      </context-param>
      -----------

      When the larger of these two values is exceeded, trinidad just trows an EOFException. What the user sees in the browser is this:
      ----------
      HTTP ERROR: 500

      Per-request disk space limits exceeded.

      RequestURI=/apache_trinidad/upload.faces
      Caused by:

      java.io.EOFException: Per-request disk space limits exceeded.
      at org.apache.myfaces.trinidadinternal.webapp.UploadedFileImpl.loadFile(UploadedFileImpl.java:236)
      at org.apache.myfaces.trinidadinternal.webapp.UploadedFileProcessorImpl.processFile(UploadedFileProcessorImpl.java:106)
      at org.apache.myfaces.trinidadinternal.webapp.TrinidadFilterImpl._doUploadFile(TrinidadFilterImpl.java:342)
      at org.apache.myfaces.trinidadinternal.webapp.TrinidadFilterImpl.doFilter(TrinidadFilterImpl.java:254)
      at org.apache.myfaces.trinidad.webapp.TrinidadFilter.doFilter(TrinidadFilter.java:90)
      at org.mortbay.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1040)
      at org.mortbay.jetty.servlet.ServletHandler.handle(ServletHandler.java:352)
      at org.mortbay.jetty.servlet.SessionHandler.handle(SessionHandler.java:230)
      at org.mortbay.jetty.handler.ContextHandler.handle(ContextHandler.java:627)
      at org.mortbay.jetty.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:149)
      at org.mortbay.jetty.handler.HandlerCollection.handle(HandlerCollection.java:123)
      at org.mortbay.jetty.handler.HandlerWrapper.handle(HandlerWrapper.java:141)
      at org.mortbay.jetty.Server.handle(Server.java:286)
      at org.mortbay.jetty.HttpConnection.handleRequest(HttpConnection.java:444)
      at org.mortbay.jetty.HttpConnection$RequestHandler.content(HttpConnection.java:715)
      at org.mortbay.jetty.HttpParser.parseNext(HttpParser.java:627)
      at org.mortbay.jetty.HttpParser.parseAvailable(HttpParser.java:203)
      at org.mortbay.jetty.HttpConnection.handle(HttpConnection.java:340)
      at org.mortbay.jetty.nio.HttpChannelEndPoint.run(HttpChannelEndPoint.java:270)
      at org.mortbay.thread.BoundedThreadPool$PoolThread.run(BoundedThreadPool.java:475)

      Powered by Jetty://
      ----------

      It is possible to add a "validator" attribute to a tr:inputFile component. However, that validator is only invoked when the upload size is smaller than the limits and the upload succeeds. I believe these limits really do make sense but a 500 plus stacktrace is not very cool. I propose that trinidad writes a FacesMessage to the FacesContext for the inputFile component and re-renders the same page, so the user sees the normal page again with a meaningful error message. The error message should be configurable/localizable.

      I don't know much about uploading things. Maybe it's also possible to determine the upload size before having received any data and then invoke the validator before receiving the data, so the developer could for example apply different size limits based on the mime-type of the upload. The inputFile component had to be extended to contain the same information as the valueChangeListener of the inputFile component received on its invocation.

      regards
      Patrick

        Activity

        Hide
        Adam Winer added a comment -

        It's a bit trickier than that... the parsing is needed not only to find the files, but also to extract ordinary form parameters, like those used for page state. So, we'd really have to continue parsing the entire page (just dropping the file content on the ground). That means that we're parsing the mega-file, if not actually storing. So, there's still a bit of DoS involved.

        There's no real way to do per-mime-type size validation (and not clear you'd want to, since mime types can be lied about). You can't do validation until after file upload has completed, because of how the JSF lifecycle and component models work.

        it would be good, however, to at least support as an option showing a FacesMessage for the component where file upload failed.

        Show
        Adam Winer added a comment - It's a bit trickier than that... the parsing is needed not only to find the files, but also to extract ordinary form parameters, like those used for page state. So, we'd really have to continue parsing the entire page (just dropping the file content on the ground). That means that we're parsing the mega-file, if not actually storing. So, there's still a bit of DoS involved. There's no real way to do per-mime-type size validation (and not clear you'd want to, since mime types can be lied about). You can't do validation until after file upload has completed, because of how the JSF lifecycle and component models work. it would be good, however, to at least support as an option showing a FacesMessage for the component where file upload failed.
        Hide
        Matthias Weßendorf added a comment -

        keeping the upload mechanism almost as is, but returning an ErrorFile when EOF has been thrown.
        In Phase3 of JSF lifecycle we throw Converter Exception and place a FacesMessage to signal that
        something was wrong with the upload

        Show
        Matthias Weßendorf added a comment - keeping the upload mechanism almost as is, but returning an ErrorFile when EOF has been thrown. In Phase3 of JSF lifecycle we throw Converter Exception and place a FacesMessage to signal that something was wrong with the upload

          People

          • Assignee:
            Matthias Weßendorf
            Reporter:
            Patrick Huber
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development