You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@struts.apache.org by "Stephan Schroeder (JIRA)" <ji...@apache.org> on 2008/03/18 17:11:58 UTC

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

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.1.0
         Environment: Windows Vista
            Reporter: Stephan Schroeder


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.


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

Posted by "Stephan Schroeder (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2557?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Stephan Schroeder updated WW-2557:
----------------------------------

    Environment: Windows Vista, Java 1.6.0_05  (was: Windows Vista)

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


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

Posted by "Rainer Hermanns (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2557?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Rainer Hermanns resolved WW-2557.
---------------------------------

    Resolution: Fixed
      Assignee: Rainer Hermanns

patch slightly modified and applied, thanks!

> 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
>            Assignee: Rainer Hermanns
>             Fix For: 2.1.3
>
>
> 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.


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

Posted by "Stephan Schroeder (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2557?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43581#action_43581 ] 

simon.void edited comment on WW-2557 at 4/4/08 7:30 AM:
---------------------------------------------------------------

Sorry, the above test doesn't work correctly (apart from the syntax error because of the missing ')' at the end of the req.addFile-lines ).
ServletFileUpload.isMultipartContent( req ) resolves to "false" even after adding
req.addHeader("Content-type", "multipart/form-data");
req.setMethod("POST");

I don't know more, yet.

      was (Author: simon.void):
    Sorry, the above test doesn't work correctly (apart from the syntax error because of the missing ')' at the end of the req.addFile-lines ).
ServletFileUpload.isMultipartContent( req ) resolves to "false" but i don't know why yet.
  
> 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.


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

Posted by "Stephan Schroeder (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2557?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Stephan Schroeder updated WW-2557:
----------------------------------

          Description: 
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>

  was:
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>

    Affects Version/s:     (was: 2.1.0)
                       2.0.11

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


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

Posted by "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.


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

Posted by "Stephan Schroeder (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2557?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43575#action_43575 ] 

Stephan Schroeder commented on WW-2557:
---------------------------------------

here is a unit test:
(attention:
-it uses a private method of FileUploadInterceptorTest, therefore it should be included there
-import org.springframework.mock.web.MockMultipartFile;
-import org.springframework.mock.web.MockMultipartHttpServletRequest;
)
<code>
    /**
     * tests whether with multiple files sent with the same name, the ones with forbiddenTypes 
     * (see FileUploadInterceptor.setAllowedTypes(...) ) are sorted out.
     * @throws Exception
     */
    public void testMultipleAccept() throws Exception {
      MockMultipartHttpServletRequest req = new MockMultipartHttpServletRequest();
      
      String htmlContent  = "<html><head></head><body>html content</body></html>";
      String plainContent = "plain content";
      
      req.addFile( new MockMultipartFile("file","test1.html","text/html", htmlContent.getBytes( "US-ASCII" ) );
      req.addFile( new MockMultipartFile("file","test2.html","text/html", htmlContent.getBytes( "US-ASCII" ) );
      req.addFile( new MockMultipartFile("file","test.txt", "text/plain",plainContent.getBytes( "US-ASCII" ) );
      
      MyFileupAction action = new MyFileupAction();

      MockActionInvocation mai = new MockActionInvocation();
      mai.setAction(action);
      mai.setResultCode("success");
      mai.setInvocationContext(ActionContext.getContext());
      Map param = new HashMap();
      ActionContext.getContext().setParameters(param);
      ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest( req, 2000));

      interceptor.setAllowedTypes( "text/html" );
      interceptor.intercept(mai);

      assertTrue(! action.hasErrors());

      assertTrue(param.size() == 3);
      File[] files = (File[]) param.get("file");
      String[] fileContentTypes = (String[]) param.get("fileContentType");
      String[] fileRealFilenames = (String[]) param.get("fileFileName");

      assertNotNull(files);
      assertNotNull(fileContentTypes);
      assertNotNull(fileRealFilenames);
      assertTrue(files.length == 2);
      assertTrue(fileContentTypes.length == 2);
      assertTrue(fileRealFilenames.length == 2);
      assertEquals("text/html", fileContentTypes[0]);
      assertNotNull("test1.html", fileRealFilenames[0]);
  }
</code>
I have to admit that i wasn't able to test this test because my spring.jar is lacking the org.springframework.web.multipart package on which MockMultipartHttpServletRequest and MockMultipartFile depend.
I asked the spring people about this error (http://forum.springframework.org/showthread.php?t=52071) but they haven't answerd yet but i figured you might have an own spring build (the package is present in the spring source) and therefore the test should work for you.

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


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

Posted by "Stephan Schroeder (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2557?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Stephan Schroeder updated WW-2557:
----------------------------------

    Flags: [Patch]

added a patch-flag because there is a patch in the description

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


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

Posted by "Stephan Schroeder (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2557?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43581#action_43581 ] 

Stephan Schroeder commented on WW-2557:
---------------------------------------

Sorry, the above test doesn't work correctly (apart from the syntax error because of the missing ')' at the end of the req.addFile-lines ).
ServletFileUpload.isMultipartContent( req ) resolves to "false" but i don't know why yet.

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


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

Posted by "Stephan Schroeder (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/struts/browse/WW-2557?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=43589#action_43589 ] 

Stephan Schroeder commented on WW-2557:
---------------------------------------

ok, here is the unit test you asked for. It's tested and turns red on the current implementation of FileUploaderInterceptor:

<code>
    /**
     * tests whether with multiple files sent with the same name, the ones with forbiddenTypes (see
     * FileUploadInterceptor.setAllowedTypes(...) ) are sorted out.
     * 
     * @throws Exception
     */
  public void testMultipleAccept() throws Exception
  {
    final String htmlContent = "<html><head></head><body>html content</body></html>";
    final String plainContent = "plain content";    
    final String bondary = "simple boundary";
    final String endline = "\r\n";
    
    MockHttpServletRequest req = new MockHttpServletRequest();
    req.setCharacterEncoding("text/html");
    req.setMethod( "POST" );
    req.setContentType( "multipart/form-data; boundary="+bondary );
    req.addHeader( "Content-type", "multipart/form-data" );
    StringBuilder content = new StringBuilder( 128 );
    content.append( encodeTextFile( bondary,endline,"file","test.html","text/plain",plainContent ) );
    content.append( encodeTextFile( bondary,endline,"file","test1.html","text/html",htmlContent ) );
    content.append( encodeTextFile( bondary,endline,"file","test2.html","text/html",htmlContent ) );
    content.append( endline );
    content.append( endline );
    content.append( endline );
    content.append( "--" );
    content.append( bondary );
    content.append( "--" );
    content.append( endline );
    req.setContent( content.toString().getBytes() );

    assertTrue( ServletFileUpload.isMultipartContent( req ) );

    MyFileupAction action = new MyFileupAction();
    MockActionInvocation mai = new MockActionInvocation();
    mai.setAction( action );
    mai.setResultCode( "success" );
    mai.setInvocationContext( ActionContext.getContext() );
    Map param = new HashMap();
    ActionContext.getContext().setParameters( param );
    ActionContext.getContext().put( ServletActionContext.HTTP_REQUEST,createMultipartRequest( req,2000 ) );

    interceptor.setAllowedTypes( "text/html" );
    interceptor.intercept( mai );

    assertEquals( 3,param.size() );
    File[] files = (File[])param.get( "file" );
    String[] fileContentTypes = (String[])param.get( "fileContentType" );
    String[] fileRealFilenames = (String[])param.get( "fileFileName" );

    assertNotNull( files );
    assertNotNull( fileContentTypes );
    assertNotNull( fileRealFilenames );
    assertEquals( "files accepted ",2,files.length );
    assertEquals( 2,fileContentTypes.length );
    assertEquals( 2,fileRealFilenames.length );
    assertEquals( "text/html",fileContentTypes[0] );
    assertNotNull( "test1.html",fileRealFilenames[0] );
  }

  private String encodeTextFile( String bondary,String endline,String name,String filename,String contentType,String content )
  {
    final StringBuilder sb = new StringBuilder( 64 );
    sb.append( endline );
    sb.append( "--" );
    sb.append( bondary );
    sb.append( endline );
    sb.append( "Content-Disposition: form-data; name=\"" );
    sb.append( name );
    sb.append( "\"; filename=\"" );
    sb.append( filename );
    sb.append( endline );
    sb.append( "Content-Type: " );
    sb.append( contentType );
    sb.append( endline );
    sb.append( endline );
    sb.append( content );

    return sb.toString();
  }
</code>

Note that it uses a private method of FileUploadInterceptorTest, therefore it should be included there.

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


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

Posted by "Don Brown (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2557?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Don Brown updated WW-2557:
--------------------------

    Fix Version/s: 2.1.3

> 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
>             Fix For: 2.1.3
>
>
> 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.


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

Posted by "Stephan Schroeder (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/struts/browse/WW-2557?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Stephan Schroeder updated WW-2557:
----------------------------------

    Description: 
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>

  was:
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>


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