Sorry I dropped out there:
> -----Original Message-----
> From: Vadim Gritsenko [mailto:[EMAIL PROTECTED]]
> Sent: Wednesday, January 29, 2003 9:05 PM
> To: [EMAIL PROTECTED]
> 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: [EMAIL PROTECTED]
> For additional commands, email: [EMAIL PROTECTED]
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, email: [EMAIL PROTECTED]
