Re: [Zope-dev] Patch: let non-seekable streams be input for ZPublisher (updated)
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
Ames Andreas (MPA/DF) wrote at 2004-8-19 18:04 +0200:
... 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.
Maybe, I have a much simpler solution: Something in ZServer makes all file fields seekable by delivering them through some temporary (either a StringIO or a temporary file). Maybe, you could do the same for your requests? -- Dieter
participants (2)
-
Ames Andreas (MPA/DF) -
Dieter Maurer