struts-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stephan Schroeder (JIRA)" <j...@apache.org>
Subject [jira] Commented: (WW-2557) FileUploadInterceptor allows forbidden files when passed with allowed files
Date Mon, 07 Apr 2008 14:46:59 GMT

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


Mime
View raw message