[Zope-dev] Patch: let non-seekable streams be input for ZPublisher (updated)

[Ames Andreas (MPA/DF)]

Hi Dieter, *,

Dieter Maurer wrote:

>   The "FieldStorage" must be rebuilt, because it can contain
>   file objects. These file objects must be reset as they may
>   have been (partially) read in the previous request.
>   This prevent reusing the previous "FieldStorage".

As you may have seen in my first patch proposal I actually tried to
address this problem, although halfbaken and buggy.  So here's the
next trial.  I hope I've got it straight now.

Why didn't I do something to reset the files, FiledStorage creates?

Because I thought it wouldn't be needed.  If you look at
cgi.FieldStorage you will see, that accessing FieldStorage's 'value'
field, will always reset the file before reading it and HTTPRequest's
'BODY' field does the same.

Cause of your hint I looked again and realised that I was wrong.
HTTPRquest's 'BODYFILE' field fails to reset the file before accessing
it (as in OFS/Image.py the File class does).  That could be easily
fixed.  What's worse is that FiledStorage can possibly create
subinstances of itsellf which can possibly contain other fileobjects.
This is the case for multipart-type requests.


How to solve this?

I can see three alternative aproaches:

1) In HTTPRequest.retry loop through self._fs.list (if it exists) and
   call seek(0) on every fileobject, you can find there.  This can
   only work if the maximum depth of the FiledStorage tree is
   predictable, which in turn depends on the possible input.  I don't
   know enough about the possible HTTP requests to determine such a
   limit or to even 'prove' its existance.

2) In HTTPRequest.retry recursively walk through the FieldStorage
   trees in self._fs.list (if it exists).  I wouldn't do that in
   Python without being able to predict the (worst case) number of
   function calls needed to do this recursion.  And without refreshing
   my knowledge of compiler techniques I wouldn't be able to roll the
   recursion out to a loop.

3) This is a somewhat drastic (and possibly conceived as hackish?)
   approach which OTOH is independent of the input and should perform
   quite well.  I overwrite cgi.FieldStorage, esp. its make_file
   method, such that it adds all fileobjects, that it creates, to a
   (possibly caller provided) list.  Thus the caller
   (i.e. HTTPRequest) has a list of all those fileobjects that need to
   be reset if the FieldStorage should be reused (as in ZPublisher's
   retries).

For the patch attached I chose the third approach.  Please let me
know, what you think about it.  If it is assessed to be ok I will
update the collctor item, I opened yesterday.


cheers,

andreas



-------------- next part --------------
A non-text attachment was scrubbed...
Name: HTTPRequest.patch
Type: text/x-patch
Size: 4827 bytes
Desc: HTTPRequest.patch (3)
Url : http://mail.zope.org/pipermail/zope-dev/attachments/20040819/f9b9712b/HTTPRequest.bin