Mailing-List: contact cocoon-dev-help@xml.apache.org; run by ezmlm
Precedence: bulk
Reply-To: cocoon-dev@xml.apache.org
Message-ID: <3E8D9159.8090007@apache.org>
Date: Fri, 04 Apr 2003 16:06:17 +0200
From: Stefano Mazzocchi <stefano@apache.org>
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US;
 rv:1.3) Gecko/20030312
MIME-Version: 1.0
To: Apache Cocoon <cocoon-dev@xml.apache.org>
Subject: [heads-up] Updated upload system and fixed a bunch of security issues
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

First of all, the servlet api decided not to deal with the payload of 
the servlet request in case of file-upload. So, either you parse the 
inputstream yourself, or you wrap the request with something different.

Cocoon uses the second approach: the incoming servlet request is wrapped 
  and this wrap provides a new method

  Object get (String name)

that can be called to access the uploaded files.

This approach is not optimal because:

  1) it creates an additional contract around the servlet API
  2) it is a weakly typed contract

In the future, we have to think at a better way to handle this, but for 
now, let's stick with this.

The thing is even worse because cocoon allowed people to plug in their 
own request factories. Thus making this contract 'shaky'.

I've just committed a patch that removes the ability to plug different 
request factories. Now cocoon has its own. Hardwired. And this creates a 
more solid contract that can be used thruout the framework and that you 
can rely upon.

Note that the Cocoon Environment Request object transparently wraps 
around the Object get(String name) method, so, you don't have to do any 
type casting since this is transparently done for you.

The only difference is that since the Cocoon Environment doesn't specify 
the *input part* object (this is something we'll have to do in the 
future), the type casting is required to acquire the "input part" that 
wraps around the uploaded file.

This is what I call "weakly typed" java contract. It looks like a 
contract, but it's not.

Anyway, I also fixed a number of security issues. Most notably:

  1) uploaded files are saved on disk by default (and web.xml has been 
changed accordingly) as a temporary storage.

  2) uploaded files saved on disk are removed right at the end of the 
request. This assumes that you will handle the uploaded files yourself 
and the upload-dir is only used as a temporary media. [This might break 
back-compatibility on behavior, but I think it's a very sane thing to 
cleanup after your own mess]

  3) I added a new servlet configuration parameter that disables 
uploading completely. And defaults to off for security reasons.

  4) I also changed 'allow-reload' to false as default.

                                           - o -

Note that the above refactoring is partially back-incompatible because 
the Object that wraps around the incoming request has been changed.

So, if you used an action for handling file upload, everything is safe 
and totally back compatible.

If you wrote your own code into a component, xsp or flow, this has to 
change.

So, here is the way you should handle the upload, for example, in flow:

Suppose you have the following HTML:

   <html>
    <form action="/upload" method="POST" enctype="multipart/form-data">
     <input type="file" name="blah"/>
     <input type="submit" name="action" value="Upload"/>