Uploaded image for project: 'Wicket'
  1. Wicket
  2. WICKET-6250

FileUploadField does not deteach models and fails to null the reference to the transient fileUploads field if forceCloseStreamsOnDetach is false

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 6.24.0
    • Fix Version/s: 8.0.0-M2, 7.5.0
    • Component/s: wicket
    • Labels:

      Description

      FileUpload does not clear our references and its model when the forceCloseStreamsOnDetach is false - which does not match the expectation from the javadoc:

        /**                                                                 
         * The FileUploadField will close any input streams you have opened in its FileUpload by
         * default. If you wish to manage the stream yourself (e.g. you want to use it in another
         * thread) then you can override this method to prevent this behavior.
         *                                                                  
         * @return <code>true</code> if stream should be closed at the end of request
         */   
      

      So it just is about not closing the streams.
      However the fileupload component does not only not close the streams - it also fails (if you return false from the forceCloseStreamsOnDetach method) to reset the model and forget about the current file uploads cached in the transient fileUploads variable.

        protected void onDetach()                                           
        {                                                                   
          if ((fileUploads != null) && forceCloseStreamsOnDetach())         
          {                                                                 
            for (FileUpload fu : fileUploads)                               
            {                                                               
              fu.closeStreams();                                            
            }                                                               
            fileUploads = null;                                             
                                                                            
            if (getModel() != null)                                         
            {                                                               
              getModel().setObject(null);                                   
            }                                                               
          }                                                                 
          super.onDetach();                                                 
        }  
      

      Shouldn't that read more like this:

        protected void onDetach()                                           
        {                                                                   
          if ((fileUploads != null))         
          { 
            if(forceCloseStreamsOnDetach() {
      	 for (FileUpload fu : fileUploads)                               
      	 {                                                               
      	  fu.closeStreams();                                            
      	 }                                                               
            }
            fileUploads = null;                                             
                                                                            
            if (getModel() != null)                                         
            {                                                               
              getModel().setObject(null);                                   
            }                                                               
          }                                                                 
          super.onDetach();                                                 
        }  
      

      In this case my streams wouldn't be closed but you could provide new streams in the next request.

      As the variable is private and i don't want to close the stream i have to use reflection at the moment to reset the field to null in an overridden onDeteach().

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                svenmeier Sven Meier
                Reporter:
                tkrah Torsten Krah
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: