[issue9968] cgi.FieldStorage: Give control about the directory used for uploads

[phep]

phep <patrice.pillot at teletopie.net> added the comment:

Le 30/07/2011 15:45, Éric Araujo a écrit :
>
> Éric Araujo<merwok at netwok.org>  added the comment:
> I’ve read in the tempfile module docstring that in order to control the
> directory, you have to set tempfile.tempdir before calling any tempfile
> function.  I suspect this will not be okay for some applications.  I
> wonder if the same limitation applies when one sets os.environ['TMP'].

You were right in pointing into this direction I forgot to mention in the 
doc part of the patch. I've had a look at tempfile implementation and made 
some tests and this led me to notice a documentation problem that might be 
worth considering (cf. infra).

But first things first: Actually, after reading tempfile.py, I cannot see 
the reason of the tempfile docstring about tempdir, except maybe for 
performance reasons. Setting tempfile.tempdir manually should not be a 
problem in any case: direct access to the member or call to 
tempfile.gettempdir() - except maybe in the very improbable case where 
tempdir would first be set to some not None value then re-set to None and a 
nasty trick I overlooked is lurking inside of the gettempdir() 
concurrent-access lock...

But, on the other hand, trying to tweak the system by using os.environ on 
TMP and such may well fail in a number of occasions since those variables 
won't be checked after the first call to tempfile.gettempdir() (except if 
tempfile.tempdir is reset to None). I wonder if the file docstring does not 
relate to this problem, as a matter of fact.

> In the light of that, do you think the tempfile solution is enough, or do
> you want to get back to the earlier idea of adding an argument to
> FieldStorage?

I can't think it of any reasonable use case that would make necessary to 
overload the interface right now provided the need to (maybe) set 
tempfile.tempdir is documented.

At last, as a side note, during the tests I ran, I noticed one should really 
not use os.rename() since this raises an OSError (file not found) upon 
script termination (since the NamedTemporaryFile has been ... renamed, hey).

As this problem does not concern the present issue, I did not deemed 
necessary to edit the documentation accordingly. Yet this might be debatable 
since the doc never explicitly says FieldStorage uses a (Named)TemporaryFile 
under the hood. Do you think one should state explicitly the coupling of 
those modules ?

> (One advantage of doc changes is that they can get committed to 2.7, 3.2
> and 3.3 and get published immediately.  We can anyway make a code change
> in 3.3 and a doc change for older versions.)

On this topic, I was wondering if the changes I propose have any chance of 
landing some day in 2.7 land - dunno Python workflow precisely enough.

>> I fiddled last night with setting an environment to deploy and test a
>> patched Python
> Are you aware of the developers’ guide?
> http://docs.python.org/devguide/

Yep, it helped me. It just took me some time to be sure to get it right, run 
the tests, ... (by the way, patchcheck.py seems to be broken).

>> (I had some problem to understand what happened when I encountered
>> 6755).
> What is 6755?

Sorry, I meant issue 6755.

> ---------- title: Let cgi.FieldStorage have named uploaded file ->
> cgi.FieldStorage: Give control about the directory used for uploads

Well, actually, the useful feature in the patch is the possibility to have a 
named-hence-linkable uploaded file. Giving control on the upload directory 
is but a useful-while-quite-necessary sister-feature, isn't it ?

>
> _______________________________________ Python
> tracker<report at bugs.python.org> <http://bugs.python.org/issue9968>
> _______________________________________
>

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue9968>
_______________________________________