[jira] Commented: (WW-2557) FileUploadInterceptor allows forbidden files when passed with allowed files

["Jeromy Evans (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/struts/browse/WW-2557?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43574#action_43574 ] 

Jeromy Evans commented on WW-2557:
----------------------------------

The bugfix and improvement look good.

The impediment for a developer committing this is the need to create a suitable unit test.  If you have time to create a unit test that shows the fix then that will help a lot.  There may be an existing test that just needs to be modified.

If the other developers are anything like me, they look at the code above, think yeah that seems good, but decide to come back to it later because it needs testing (and then forget to come back)

> FileUploadInterceptor allows forbidden files when passed with allowed files
> ---------------------------------------------------------------------------
>
>                 Key: WW-2557
>                 URL: https://issues.apache.org/struts/browse/WW-2557
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>    Affects Versions: 2.0.11
>         Environment: Windows Vista, Java 1.6.0_05
>            Reporter: Stephan Schroeder
>
> Summary: If you set the "allowedTypes" parameter of FileUploadInterceptor for example to "image/jpeg" and upload a jpg file and a gif file whit the same form name 
> (e.g.:
> <@s.form action="photoupload" method="post" enctype="multipart/form-data">
>         <@s.file name="photos" label="Pictured 1"/>
>         <@s.file name="photos" label="Pictured 2"/>
> 	<@s.submit/>
> </...@s.form>)
> than the gif file will be accepted too.
> this is some code from the uptodate SVN repository of FileUploadInterceptor
> (http://svn.apache.org/viewvc/struts/struts2/trunk/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java?revision=615436&view=markup)
> <code>
> 1  File[] files = multiWrapper.getFiles(inputName);
> 2  if (files != null) {
> 3    for (int index = 0; index < files.length; index++) {
> 4      if (acceptFile(files[index], contentType[index], inputName, validation, ac.getLocale())){
> 5        parameters.put(inputName, files);
> 6        parameters.put(inputName + "ContentType", contentType);
> 7        parameters.put(inputName + "FileName", fileName);
> 8      }
> 9    }
> 10}
> </code>
> Bug 1) as you can see in line 4 and 5 as soon as one file is accepted the whole array is added to parameters which of course means even the files which haven't been accepted themselfs.
> Improvement 1) in line 6 and 7 static string concatenations are done within a loop. This should move out of the loop.
> Here is my proposal for a fix for both issues:
> <code>
> File[] files = multiWrapper.getFiles(inputName);
> if (files != null) {
>   ArrayList acceptedFiles = new ArrayList( files.length() );
>   ArrayList acceptedContentTypes = new ArrayList( files.length() );
>   ArrayList acceptedFileNames = new ArrayList( files.length() );
>   String contentTypeName = inputName + "ContentType";
>   String fileNameName    = inputName + "FileName";
>   for (int index = 0; index < files.length; index++) {
>     if (acceptFile(files[index], contentType[index], inputName, validation, ac.getLocale())){
>       acceptedFiles.add( files[index] );
>       acceptedContentTypes.add( contentType[index] );
>       acceptedFileNames.add( fileName[index] );
>     }
>   }
>   if( acceptedFiles.size()!=0 ) {
>     parameters.put(inputName, acceptedFiles.toArray());
>     parameters.put(contentTypeName, acceptedContentTypes.toArray());
>     parameters.put(fileNameName, acceptedFileNames.toArray());
>   }
> }
> </code>

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.