You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Geoff Howard <co...@leverageweb.com> on 2003/01/29 23:28:19 UTC
New Temporary upload feature
I've been working on a quick patch to provide a third autosave-uploads
option. Currently "true" results in a permanent file (FilePartFile) being
created in the upload-dir. "false" results in a temporary byte[] stored in
memory (FilePartArray).
when doing work on file uploads before it occurred to me that it might be
useful to have an option to have a temporary physical file which gets
deleted after request processing is done. I had originally planned on using
FilePartArray for this but was concerned about situations where large files
are uploaded with respect to memory use. Using FilePart instead I consider
a potential problem since an abusive user aware of cocoon's system could
upload arbitrary files to any server running cocoon limited only by the
default 10 meg limit per request.
i've got it working locally (though not bugzilla ready yet) but before I
spend more time on it wanted to see if
1) does this seem like a worthwhile feature, and
2) make sure my implementation was acceptable.
Currently I have added a boolean tempUploads to CocoonServlet which will be
triggered by a "temp" value for autosave in web.xml [1]
the relevant section comes right after request processing:
if (this.cocoon.process(env)) {
contentType = env.getContentType();
// processing is done & successful - Clean File Parts up
// TODO: could this be done regardless of success of process?
if (tempUploads) {
log.debug("AutoSave uploads set to temporary - examining request
parameters");
Enumeration enum=request.getParameterNames();
while(enum.hasMoreElements()) {
// use requestFactory directly so we don't have to go back through
// the objectModel. We already know we're in the servlet environment
Object obj=this.requestFactory.get(request,(String)enum.nextElement());
// should FilePartArrays be handled too? (nulled?)
if (obj instanceof FilePartFile) {
File tmpFile=((FilePartFile)obj).getFile();
if (tmpFile.exists() && tmpFile.delete()) {
log.debug("Removed " + tmpFile.getName());
} else {
log.debug("Could not remove file: " + tmpFile.getName());
}
}
}
}
This should probably be done whether requests are successful or not which
would mean doing clean up in the condition this.cocoon == null at line 995
as well as in finally blocks in at least two other places unless I'm
thinking of this wrong.
Any suggestions?
Geoff Howard
[1] autoSaveUploads will still need to be set to true to get FilePartFile -
I considered but rejected subclassing FilePartFile to FilePartTempFile
because I'd also have to modify at least the MultipartParser and the method
signature for requestFactory.getServletRequest and wasn't sure how that
would affect MaybeUpload
---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org
RE: New Temporary upload feature
Posted by Geoff Howard <co...@leverageweb.com>.
Sorry I dropped out there:
> -----Original Message-----
> From: Vadim Gritsenko [mailto:vadim.gritsenko@verizon.net]
> Sent: Wednesday, January 29, 2003 9:05 PM
> To: cocoon-dev@xml.apache.org
> Subject: Re: New Temporary upload feature
>
> Geoff Howard wrote:
<snip/>
>
> >if (this.cocoon.process(env)) {
> > contentType = env.getContentType();
> >
> >// processing is done & successful - Clean File Parts up
> >// TODO: could this be done regardless of success of process?
> >
> >
>
> It's not necessary: if you want logic more complex then "save all" or
> "save and then remove all", then you should code own action / xsp page /
> whatever to clean up things (move files where needed / delete / etc).
> Here it is enough to have basic functionality preventing abuse.
The brief comment must not have made it very clear. I meant that if for
whatever reason this.cocoon.process(env) returns false, or this.cocoon =
getCocoon(request.getPathInfo(),
request.getParameter(Constants.RELOAD_PARAM));
returns a null value, they will not be cleaned up. The uploaded files will
already have been saved to disk because it has already happened at the call
to requestFactory.getServletRequest() in line 985. So by placing the code
only where
I have so far, files are only cleaned up when a request is served
successfully. The
implication is for instance multipart requests to cocoon/fakeURI would
defeat the
cleanup process. I suppose the implications for when this.cocoon is null
are not as
serious - it would only be exploitable on a server where cocoon failed to
start up and
was left that way - right?
>
>
> >if (tempUploads) {
> > log.debug("AutoSave uploads set to temporary - examining request
> >parameters");
> > Enumeration enum=request.getParameterNames();
> > while(enum.hasMoreElements()) {
> > // use requestFactory directly so we don't have to go back through
> > // the objectModel. We already know we're in the servlet environment
> > Object
> obj=this.requestFactory.get(request,(String)enum.nextElement());
> > // should FilePartArrays be handled too? (nulled?)
> > if (obj instanceof FilePartFile) {
> > File tmpFile=((FilePartFile)obj).getFile();
> >
>
> Correction (if added):
Did I read the correction correctly this is logically identical but more
readable?
>
> > if (tmpFile.exists()) {
> >
> > if (tmpFile.delete()) {
> > log.debug("Removed " + tmpFile.getName());
> > } else {
> > log.debug("Could not remove file: " + tmpFile.getName());
> > }
> > }
> >
> > }
> > }
> >}
> >
> >This should probably be done whether requests are successful or not which
> >would mean doing clean up in the condition this.cocoon == null
> at line 995
> >as well as in finally blocks in at least two other places unless I'm
> >thinking of this wrong.
> >
> >
>
> Somewhere in the finally block should be ok (haven't looked into
> the code).
Ok, you must have understood correctly then.
Thanks for the feedback. I'll send a patch soon.
Geoff
>
> Vadim
>
>
> >Any suggestions?
> >
> >Geoff Howard
> >
> >[1] autoSaveUploads will still need to be set to true to get
> FilePartFile -
> >I considered but rejected subclassing FilePartFile to FilePartTempFile
> >because I'd also have to modify at least the MultipartParser and
> the method
> >signature for requestFactory.getServletRequest and wasn't sure how that
> >would affect MaybeUpload
> >
> >
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
> For additional commands, email: cocoon-dev-help@xml.apache.org
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org
Re: New Temporary upload feature
Posted by Vadim Gritsenko <va...@verizon.net>.
Geoff Howard wrote:
>I've been working on a quick patch to provide a third autosave-uploads
>option. Currently "true" results in a permanent file (FilePartFile) being
>created in the upload-dir. "false" results in a temporary byte[] stored in
>memory (FilePartArray).
>
>when doing work on file uploads before it occurred to me that it might be
>useful to have an option to have a temporary physical file which gets
>deleted after request processing is done. I had originally planned on using
>FilePartArray for this but was concerned about situations where large files
>are uploaded with respect to memory use. Using FilePart instead I consider
>a potential problem since an abusive user aware of cocoon's system could
>upload arbitrary files to any server running cocoon limited only by the
>default 10 meg limit per request.
>
>i've got it working locally (though not bugzilla ready yet) but before I
>spend more time on it wanted to see if
>
>1) does this seem like a worthwhile feature, and
>
>
I think it makes sense to add feature like this.
>2) make sure my implementation was acceptable.
>
>Currently I have added a boolean tempUploads to CocoonServlet which will be
>triggered by a "temp" value for autosave in web.xml [1]
>
>the relevant section comes right after request processing:
>
>if (this.cocoon.process(env)) {
> contentType = env.getContentType();
>
>// processing is done & successful - Clean File Parts up
>// TODO: could this be done regardless of success of process?
>
>
It's not necessary: if you want logic more complex then "save all" or
"save and then remove all", then you should code own action / xsp page /
whatever to clean up things (move files where needed / delete / etc).
Here it is enough to have basic functionality preventing abuse.
>if (tempUploads) {
> log.debug("AutoSave uploads set to temporary - examining request
>parameters");
> Enumeration enum=request.getParameterNames();
> while(enum.hasMoreElements()) {
> // use requestFactory directly so we don't have to go back through
> // the objectModel. We already know we're in the servlet environment
> Object obj=this.requestFactory.get(request,(String)enum.nextElement());
> // should FilePartArrays be handled too? (nulled?)
> if (obj instanceof FilePartFile) {
> File tmpFile=((FilePartFile)obj).getFile();
>
Correction (if added):
> if (tmpFile.exists()) {
>
> if (tmpFile.delete()) {
> log.debug("Removed " + tmpFile.getName());
> } else {
> log.debug("Could not remove file: " + tmpFile.getName());
> }
> }
>
> }
> }
>}
>
>This should probably be done whether requests are successful or not which
>would mean doing clean up in the condition this.cocoon == null at line 995
>as well as in finally blocks in at least two other places unless I'm
>thinking of this wrong.
>
>
Somewhere in the finally block should be ok (haven't looked into the code).
Vadim
>Any suggestions?
>
>Geoff Howard
>
>[1] autoSaveUploads will still need to be set to true to get FilePartFile -
>I considered but rejected subclassing FilePartFile to FilePartTempFile
>because I'd also have to modify at least the MultipartParser and the method
>signature for requestFactory.getServletRequest and wasn't sure how that
>would affect MaybeUpload
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org