You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Martin Cooper <ma...@apache.org> on 2005/11/28 05:25:41 UTC

Re: DO NOT REPLY [Bug 32785] - [fileupload] FileItem implements Serializable incorrectly

On 11/27/05, Niall Pemberton <ni...@blueyonder.co.uk> wrote:
>
> Sorry Martin, I'm probably driving you mad. I just attached the 3rd patch
> before I saw you'd accepted the second :-(
>
> I'll go away now.


;-)

Actually, the second is more robust than the third in any case, since the
default location for these files is the container's temp directory, which is
very likely to be cleaned out on a server restart. And if the default isn't
used, all bets are off anyway.

I'm still apprehensive about the performance implications, if people don't
think about what they're doing, but at least now DiskFileItem adheres to its
contract.

--
Martin Cooper


Niall
>
>
> ----- Original Message -----
> From: <bu...@apache.org>
> To: <co...@jakarta.apache.org>
> Sent: Monday, November 28, 2005 4:07 AM
> Subject: DO NOT REPLY [Bug 32785] - [fileupload] FileItem implements
> Serializable incorrectly
>
>
> > DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUGĀ·
> > RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
> > <http://issues.apache.org/bugzilla/show_bug.cgi?id=32785>.
> > ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED ANDĀ·
> > INSERTED IN THE BUG DATABASE.
> >
> > http://issues.apache.org/bugzilla/show_bug.cgi?id=32785
> >
> >
> > niallp@apache.org changed:
> >
> >            What    |Removed                     |Added
> >
> --------------------------------------------------------------------------
> --
> >   Attachment #17059|0                           |1
> >         is obsolete|                            |
> >
> >
> >
> >
> > ------- Additional Comments From niallp@apache.org  2005-11-28
> 05:07 -------
> > Created an attachment (id=17060)
> >  -->
> (http://issues.apache.org/bugzilla/attachment.cgi?id=17060&action=view)
> > 3rd Patch for DiskFileItem
> >
> > I guess the 2nd patch isn't a great idea either since its not
> v.efficient
> > copying a ton of data.
> >
> > Attaching a 3rd patch - if its not "in memory" this one caches the File
> that
> > DeferredFileOutputStream has written to when serializing. Everywhere
> that
> > referenced the DeferredFileOutputStream now checks to see if that File
> is
> there
> > first. If it is "in memory" it re-creates the DeferredFileOutputStream.
> >
> > --
> > Configure bugmail:
> http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
> > ------- You are receiving this mail because: -------
> > You are the assignee for the bug, or are watching the assignee.
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> > For additional commands, e-mail: commons-dev-help@jakarta.apache.org
> >
> >
> >
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>

Re: DO NOT REPLY [Bug 32785] - [fileupload] FileItem implements Serializable incorrectly

Posted by Martin Cooper <ma...@apache.org>.
On 11/27/05, Niall Pemberton <ni...@gmail.com> wrote:
>
> On 11/28/05, Martin Cooper <ma...@apache.org> wrote:
> > On 11/27/05, Niall Pemberton <ni...@blueyonder.co.uk> wrote:
> > >
> > > Sorry Martin, I'm probably driving you mad. I just attached the 3rd
> patch
> > > before I saw you'd accepted the second :-(
> > >
> > > I'll go away now.
> >
> >
> > ;-)
> >
> > Actually, the second is more robust than the third in any case, since
> the
> > default location for these files is the container's temp directory,
> which is
> > very likely to be cleaned out on a server restart. And if the default
> isn't
> > used, all bets are off anyway.
>
> If the default is used and the  container's temp directory is cleared
> out - then the 2nd patch is stuffed as well - it'll try and copy a
> file that doesn't exist.


Ah, right. Oh well.

> I'm still apprehensive about the performance implications, if people don't
> > think about what they're doing, but at least now DiskFileItem adheres to
> its
> > contract.
>
> I agree, and I think they'll be alot not so large files that'll now
> work well. Although the best solution was your original comment -
> don't stick the FileItem in the Session :-)
>
> You look like you have FileUpload heading towards a release?


That's the general idea, yes. I've temporarily abandoned the enhancements I
had in the wings, on the basis that what's in SVN now has been there so
long, it's stable, and people need a released version, especially the folks
who need portlet support. The bugs are fixed, so it's pretty close, I think.

--
Martin Cooper


> Martin Cooper
> >
> >
> > Niall
> > >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>

Re: DO NOT REPLY [Bug 32785] - [fileupload] FileItem implements Serializable incorrectly

Posted by Niall Pemberton <ni...@gmail.com>.
On 11/28/05, Martin Cooper <ma...@apache.org> wrote:
> On 11/27/05, Niall Pemberton <ni...@blueyonder.co.uk> wrote:
> >
> > Sorry Martin, I'm probably driving you mad. I just attached the 3rd patch
> > before I saw you'd accepted the second :-(
> >
> > I'll go away now.
>
>
> ;-)
>
> Actually, the second is more robust than the third in any case, since the
> default location for these files is the container's temp directory, which is
> very likely to be cleaned out on a server restart. And if the default isn't
> used, all bets are off anyway.

If the default is used and the  container's temp directory is cleared
out - then the 2nd patch is stuffed as well - it'll try and copy a
file that doesn't exist.

> I'm still apprehensive about the performance implications, if people don't
> think about what they're doing, but at least now DiskFileItem adheres to its
> contract.

I agree, and I think they'll be alot not so large files that'll now
work well. Although the best solution was your original comment -
don't stick the FileItem in the Session :-)

You look like you have FileUpload heading towards a release?

> Martin Cooper
>
>
> Niall
> >

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org