[WICKET-6250] FileUploadField does not deteach models and fails to null the reference to the transient fileUploads field if forceCloseStreamsOnDetach is false

• 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: file_upload
• Assignee: Sven Meier
• Reporter: Torsten Krah
• Created: 29/Sep/16 16:02
• Updated: 04/Jan/17 09:43
• Resolved: 01/Oct/16 21:45

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().

Issue Links

• is broken by: WICKET-3370 Extend multiple file handling to IMultipartWebRequest