You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Steve Hay <st...@uk.radan.com> on 2003/08/18 17:58:18 UTC

[PATCH mp1.28] Clean up file uploaded by test suite

Hi,

After running the test suite with mp1.28 on Windows I find that I have a 
temporary file left behind in my temp dir (in my case, C:\Temp).

This happens because the filehandle is left open.  The attached patch 
closes it, and the file gets deleted automatically later.

- Steve

[PATCH] Clean up temp files [Was: Re: [PATCH mp1.28] Clean up file uploaded by test suite]

Posted by Steve Hay <st...@uk.radan.com>.
Hi Stas / Lincoln,

Lincoln: We've been having a discussion on the mod_perl dev list 
(http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=106122218228508&w=2) 
about a problem with temporary files from file uploads being left behind 
by CGI.pm on Windows.  The problem occurs because the unlink is 
attempted before the file is closed, which doesn't work on Windows.  
Initially I sent a patch for the affected test script in mod_perl to 
simply close the file itself which remedies the problem, but Stas 
insisted that it was really a bug in CGI.pm.  I think he's right, and 
hopefully the attached patch (against 3.00) fixes it.

Stas Bekman wrote:

> Steve Hay wrote:
>
>> If I run this test script:
>>
>> ==========
>> use CGI;
>> my $cgi = CGI->new();
>> print    $cgi->header(),
>>    $cgi->start_html(),
>>    $cgi->start_multipart_form();
>> my $fh = $cgi->upload('upload');
>> if (defined $fh) {
>>    1 while <$fh>;
>>    print $cgi->p(sprintf("Read %d bytes", tell $fh));
>> }
>> else {
>>    print    $cgi->filefield({-name => 'upload'}),
>>        $cgi->submit();
>> }
>> print    $cgi->end_form(),
>>    $cgi->end_html();
>> ==========
>>
>> then the filehandle $fh gets left open, so when the DESTROY in 
>> CGI.pm's CGITempFile package runs and tries to unlink the temporary 
>> file it fails because you can't unlink a file while it is open on 
>> Windows.
>>
>> In fact, adding some debug into that DESTROY subroutine:
>>
>>    unlink $safe               # get rid of the file
>>        or print STDERR "Can't unlink '$safe': $!\n";
>>
>> yields this error message in my Apache error.log:
>>
>>    Can't unlink 'C:\temp\CGItemp34790': Permission denied
>>
>> If I add
>>
>>    close $fh;
>>
>> to the end of the "if" block in my test script above then the error 
>> goes away and the file is deleted.
>
>
> So, $fh is getting destroyed after $cgi, which leads to the problem. 
> If $fh could be made to destroy before $cgi, the problem could be 
> removed.
>
> I think, the proper solution is for CGI.pm to register a cleanup 
> handler, which will do the cleanup, after $cgi has gone. This will 
> work for mod_perl, for plain cgi scripts perhaps an END block will do. 
> Finally, making the returned filehandle an object, with its own 
> destroy method may do the trick as well.
>
> Another solution I can think of is to use Devel::Peek to reduce the 
> reference count of all filehandles opened by CGI.pm to 0, from 
> CGI::DESTROY, but that's probably too intrusive and it's also not the 
> best idea to rely on dev-module ;) 

I think the problem is this: the CGI object stores a Fh object and a 
CGITempFile object: they're both stashed away at the end of 
read_multipart() - the Fh object ($filehandle) in $self->{$param}, and 
the CGITempFile object ($tmpfile) in $self->{'.tmpfiles'}.  When the CGI 
object is destroyed by Perl there is no guarantee which of its contained 
objects gets destroyed first.

It would appear (on Windows, at least) that CGITempFile->DESTROY gets 
run before Fh->DESTROY.  On Windows, that doesn't work.

The attached patch (against 3.00) fixes the problem for me by filling in 
the CGI class' currently empty DESTROY method with something useful: 
Namely, make it have each Fh destroyed before the corresponding 
CGITempFile.  To make this a little easier, I've stashed the Fh object 
in $self->{'.tmpfiles'} at the end of read_multipart() as well.

I think this is easier than mucking about with cleanup handlers and/or 
END blocks.  One doesn't normally explicitly call DESTROY methods, but 
it is OK according to Perl's perlobj manpage (in the section on 
"Destructors").

(That same manpage also comments "[contained] objects will be freed and 
destroyed automatically when the current object is freed, provided no 
other references to them exist elsewhere," but in this case there are 
two problems with that - firstly, other references to them _do_ exist 
(in the caller's script), and secondly, we need to control the _order_ 
in which the contained objects are destroyed - hence I think we're 
justified in doing this.)

What do you think?

- Steve

[DOC PATCH] Re: [PATCH libapreq-1.2] Re: [PATCH mp1.28] Clean up file uploaded by test suite

Posted by Steve Hay <st...@uk.radan.com>.
Joe Schaefer wrote:

>Steve Hay <st...@uk.radan.com> writes:
>  
>
>>How does tempnam() behave on other OS's?  My K&R makes no mention of
>>environment variables coming into play.
>>    
>>
>
>glibc's documented behavior for tempnam() is similar to MS's 
>(except they use "TMPDIR" instead of "TMP"):
>
>  % man 3 tempnam
>    ...
>    Attempts to find an  appropriate  directory  go through the
>    following steps: (i) In case the environment variable TMPDIR exists
>    and  contains  the  name  of  an appropriate  directory, that is used.  
>    ...
>
>  
>
>>If it's only Windows that behaves this way then I'm happy to provide a DOC
>>PATCH to describe this behaviour.
>>    
>>
>
>+1.
>
OK, then here's patch for Request.pm's POD.

It also cleans up a couple of whitespace-only lines which podchecker 
doesn't like, and removes the =over 4 ... =back commands that the 
=head2's were enclosed in because podchecker didn't like them either.

- Steve

Re: [PATCH libapreq-1.2] Re: [PATCH mp1.28] Clean up file uploaded by test suite

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Steve Hay <st...@uk.radan.com> writes:


[...]

> In my case, the default temporary directory used was C:\WINDOWS\Temp,
> but when I tried changing it to C:\Temp via that TEMP_DIR setting, I
> found that it made no difference - the uploaded file still got spooled
> into C:\WINDOWS\Temp. A look at the MS documentation for their
> implementation of the standard C library routine tempnam(), which
> ApacheRequest_tmpfile() uses, says of the first argument, "Target
> directory to be used if TMP not defined".  The "TMP" there is a
> reference to the TMP environment variable.

FWIW, here's what POSIX (v3) says about tempnam():

  char *tempnam(const char *dir, const char *pfx);

  DESCRIPTION

  The tempnam() function shall generate a pathname that may be used for
  a temporary file. 

  The tempnam() function allows the user to control the choice of a
  directory. The dir argument points to the name of the directory in which
  the file is to be created. If dir is a null pointer or points to a
  string which is not a name for an appropriate directory, the path prefix
  defined as P_tmpdir in the <stdio.h> header shall be used. If that
  directory is not accessible, an implementation-defined directory may be
  used.

It conveniently omits any mention of what should happen if dir
*is* a valid directory name.

> How does tempnam() behave on other OS's?  My K&R makes no mention of
> environment variables coming into play.

glibc's documented behavior for tempnam() is similar to MS's 
(except they use "TMPDIR" instead of "TMP"):

  % man 3 tempnam
    ...
    Attempts to find an  appropriate  directory  go through the
    following steps: (i) In case the environment variable TMPDIR exists
    and  contains  the  name  of  an appropriate  directory, that is used.  
    ...

> If it's only Windows that behaves this way then I'm happy to provide a DOC
> PATCH to describe this behaviour.

+1.

-- 
Joe Schaefer


[PATCH libapreq-1.2] Re: [PATCH mp1.28] Clean up file uploaded by test suite

Posted by Steve Hay <st...@uk.radan.com>.
Stas Bekman wrote:

> Steve Hay wrote:
>
>> Stas Bekman wrote:
>>
>>> Do we have the same problem with Apache::Request?
>>
>>
>>
>> I don't think so, but I'm not sure where the uploaded data gets 
>> spooled to by Apache::Request.  (CGI.pm created "CGItemp<number>" 
>> files in my C:\Temp directory.  Where does Apache::Request put this 
>> data?)
>
>
> You can specify the temp dir in new(), with TEMP_DIR. it uses the 
> apreq prefix for temp files. see ApacheRequest_tmpfile() in 
> standard/c/apache_request.c. 

Thanks.  We definitely don't have a problem in that regard then: 
ApacheRequest_tmpfile() calls ap_register_cleanup(..., remove_tmpfile, 
...), and remove_tmpfile() very sensibly calls ap_pfclose() before remove().

In fact, I hadn't spotted it yesterday, but Apache::Upload has a 
tempname() method that tells me exactly where the file went!  It is 
indeed cleaned up OK.

That's the good news.

The bad news is that the TEMP_DIR setting that you've just drawn my 
attention to doesn't work as documented, at least on Windows.

In my case, the default temporary directory used was C:\WINDOWS\Temp, 
but when I tried changing it to C:\Temp via that TEMP_DIR setting, I 
found that it made no difference - the uploaded file still got spooled 
into C:\WINDOWS\Temp.

A look at the MS documentation for their implementation of the standard 
C library routine tempnam(), which ApacheRequest_tmpfile() uses, says of 
the first argument, "Target directory to be used if TMP not defined".  
The "TMP" there is a reference to the TMP environment variable.

I had the Apache service running as the LocalSystem account, for which 
TMP is evidently defined as C:\WINDOWS\Temp by the system.  Sure enough, 
if I reconfigure Apache to run as the user "steveh", whose TMP 
environment variable is defined as "C:\DOCUME~1\steveh\LOCALS~1\Temp" 
(again by the system), then the uploaded file now gets spooled into 
there, again ignoring my TEMP_DIR setting.

How does tempnam() behave on other OS's?  My K&R makes no mention of 
environment variables coming into play.

If it's only Windows that behaves this way then I'm happy to provide a 
DOC PATCH to describe this behaviour.

Alternatively, the attached patch fixes it for me.  It works by simply 
clearing the TMP environment variable for the duration of the tempnam() 
call.  Note that it only does that if TEMP_DIR has been set, otherwise 
tempnam() has no directory specified to work with, which makes it 
default to the value of P_tmpdir in <stdio.h>, which is "\\" -- not very 
nice!

- Steve

[PATCH libapreq-1.2] Re: [PATCH mp1.28] Clean up file uploaded by test suite

Posted by Steve Hay <st...@uk.radan.com>.
Stas Bekman wrote:

> Steve Hay wrote:
>
>> Stas Bekman wrote:
>>
>>> Do we have the same problem with Apache::Request?
>>
>>
>>
>> I don't think so, but I'm not sure where the uploaded data gets 
>> spooled to by Apache::Request.  (CGI.pm created "CGItemp<number>" 
>> files in my C:\Temp directory.  Where does Apache::Request put this 
>> data?)
>
>
> You can specify the temp dir in new(), with TEMP_DIR. it uses the 
> apreq prefix for temp files. see ApacheRequest_tmpfile() in 
> standard/c/apache_request.c. 

Thanks.  We definitely don't have a problem in that regard then: 
ApacheRequest_tmpfile() calls ap_register_cleanup(..., remove_tmpfile, 
...), and remove_tmpfile() very sensibly calls ap_pfclose() before remove().

In fact, I hadn't spotted it yesterday, but Apache::Upload has a 
tempname() method that tells me exactly where the file went!  It is 
indeed cleaned up OK.

That's the good news.

The bad news is that the TEMP_DIR setting that you've just drawn my 
attention to doesn't work as documented, at least on Windows.

In my case, the default temporary directory used was C:\WINDOWS\Temp, 
but when I tried changing it to C:\Temp via that TEMP_DIR setting, I 
found that it made no difference - the uploaded file still got spooled 
into C:\WINDOWS\Temp.

A look at the MS documentation for their implementation of the standard 
C library routine tempnam(), which ApacheRequest_tmpfile() uses, says of 
the first argument, "Target directory to be used if TMP not defined".  
The "TMP" there is a reference to the TMP environment variable.

I had the Apache service running as the LocalSystem account, for which 
TMP is evidently defined as C:\WINDOWS\Temp by the system.  Sure enough, 
if I reconfigure Apache to run as the user "steveh", whose TMP 
environment variable is defined as "C:\DOCUME~1\steveh\LOCALS~1\Temp" 
(again by the system), then the uploaded file now gets spooled into 
there, again ignoring my TEMP_DIR setting.

How does tempnam() behave on other OS's?  My K&R makes no mention of 
environment variables coming into play.

If it's only Windows that behaves this way then I'm happy to provide a 
DOC PATCH to describe this behaviour.

Alternatively, the attached patch fixes it for me.  It works by simply 
clearing the TMP environment variable for the duration of the tempnam() 
call.  Note that it only does that if TEMP_DIR has been set, otherwise 
tempnam() has no directory specified to work with, which makes it 
default to the value of P_tmpdir in <stdio.h>, which is "\\" -- not very 
nice!

- Steve

Re: [PATCH mp1.28] Clean up file uploaded by test suite

Posted by Stas Bekman <st...@stason.org>.
Steve Hay wrote:
> Stas Bekman wrote:
> 
>> Do we have the same problem with Apache::Request?
> 
> 
> I don't think so, but I'm not sure where the uploaded data gets spooled 
> to by Apache::Request.  (CGI.pm created "CGItemp<number>" files in my 
> C:\Temp directory.  Where does Apache::Request put this data?)

You can specify the temp dir in new(), with TEMP_DIR. it uses the apreq prefix 
for temp files. see ApacheRequest_tmpfile() in standard/c/apache_request.c.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


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


Re: [PATCH mp1.28] Clean up file uploaded by test suite

Posted by Steve Hay <st...@uk.radan.com>.
Stas Bekman wrote:

> Do we have the same problem with Apache::Request?

I don't think so, but I'm not sure where the uploaded data gets spooled 
to by Apache::Request.  (CGI.pm created "CGItemp<number>" files in my 
C:\Temp directory.  Where does Apache::Request put this data?)

The script below, adapted from the one I posted previously that showed 
up the bug in CGI.pm, doesn't leave any files in any of my obvious 
"temp" directories.

===
use Apache;
use Apache::Request;
use CGI qw(); # just for HTML output
my $r = Apache->request();
my $apr = Apache::Request->new($r);
my $cgi = CGI->new('');
print    $cgi->header(),
    $cgi->start_html(),
    $cgi->start_multipart_form();
my $upl = $apr->upload('upload');
my $fh;
if (defined $upl) {
    $fh = $upl->fh();
    1 while <$fh>;
    print $cgi->p(sprintf("Read %d bytes", tell $fh));
}
else {
    print    $cgi->filefield({-name => 'upload'}),
        $cgi->submit();
}
print    $cgi->end_form(),
    $cgi->end_html();
===



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


Re: [PATCH mp1.28] Clean up file uploaded by test suite

Posted by Stas Bekman <st...@stason.org>.
Steve Hay wrote:
> Stas Bekman wrote:
> 
>> Steve Hay wrote:
>> [...]
>>
>>>> I can't find in the CGI.pm manpage
>>>
>>>
>>>
>>>
>>> It's right after the bit that you quoted :-)  Lines 4439-4442 in the 
>>> CGI.pm of 2.97.
>>>
>>>> , where it says that the filehandle should be closed, even on 
>>>> windows. Perhaps ask Lincoln?
>>>
>>>
>>
>> Sorry, I don't understand. I said:
>>
>>   I can't find in the CGI.pm manpage,  where it says that the filehandle
>>   should be closed, even on windows. Perhaps ask Lincoln?
>>
>> I don't see where it says that on the lines that you've quoted. 
> 
> 
> My apologies, I only half-read what you said.  I thought you were having 
> trouble finding the bit that I'd quoted ;-)

;)

> It doesn't actually say anywhere in the CGI.pm manpage that you must 
> close the filehandle, even on Windows.  That's just something that I've 
> found from experience.
> 
> The bit that I quoted hints at a possible explanation:  It is actually 
> talking about a "private_tempfiles" option that the mod_perl test script 
> doesn't use, but it mentions that on _Unix_ a file can be unlinked while 
> it is open.  The implication is that this perhaps can't be done on other 
> OS's, and in practice I find that that is indeed the case on Windows, 
> and probably explains what is going on here.

It does that mainly for security reasons as the manpage explains, so one won't 
be able to snoop easily on your files. I still maintain that this is either a 
bug in CGI.pm, or this side-effect should be *at least* documented.

> If I run this test script:
> 
> ==========
> use CGI;
> my $cgi = CGI->new();
> print    $cgi->header(),
>    $cgi->start_html(),
>    $cgi->start_multipart_form();
> my $fh = $cgi->upload('upload');
> if (defined $fh) {
>    1 while <$fh>;
>    print $cgi->p(sprintf("Read %d bytes", tell $fh));
> }
> else {
>    print    $cgi->filefield({-name => 'upload'}),
>        $cgi->submit();
> }
> print    $cgi->end_form(),
>    $cgi->end_html();
> ==========
> 
> then the filehandle $fh gets left open, so when the DESTROY in CGI.pm's 
> CGITempFile package runs and tries to unlink the temporary file it fails 
> because you can't unlink a file while it is open on Windows.
> 
> In fact, adding some debug into that DESTROY subroutine:
> 
>    unlink $safe               # get rid of the file
>        or print STDERR "Can't unlink '$safe': $!\n";
> 
> yields this error message in my Apache error.log:
> 
>    Can't unlink 'C:\temp\CGItemp34790': Permission denied
> 
> If I add
> 
>    close $fh;
> 
> to the end of the "if" block in my test script above then the error goes 
> away and the file is deleted.

So, $fh is getting destroyed after $cgi, which leads to the problem. If $fh 
could be made to destroy before $cgi, the problem could be removed.

I think, the proper solution is for CGI.pm to register a cleanup handler, 
which will do the cleanup, after $cgi has gone. This will work for mod_perl, 
for plain cgi scripts perhaps an END block will do. Finally, making the 
returned filehandle an object, with its own destroy method may do the trick as 
well.

Another solution I can think of is to use Devel::Peek to reduce the reference 
count of all filehandles opened by CGI.pm to 0, from CGI::DESTROY, but that's 
probably too intrusive and it's also not the best idea to rely on dev-module ;)

Do we have the same problem with Apache::Request?

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


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


Re: [PATCH mp1.28] Clean up file uploaded by test suite

Posted by Steve Hay <st...@uk.radan.com>.
Stas Bekman wrote:

> Steve Hay wrote:
> [...]
>
>>> I can't find in the CGI.pm manpage
>>
>>
>>
>> It's right after the bit that you quoted :-)  Lines 4439-4442 in the 
>> CGI.pm of 2.97.
>>
>>> , where it says that the filehandle should be closed, even on 
>>> windows. Perhaps ask Lincoln?
>>
>
> Sorry, I don't understand. I said:
>
>   I can't find in the CGI.pm manpage,  where it says that the filehandle
>   should be closed, even on windows. Perhaps ask Lincoln?
>
> I don't see where it says that on the lines that you've quoted. 

My apologies, I only half-read what you said.  I thought you were having 
trouble finding the bit that I'd quoted ;-)

It doesn't actually say anywhere in the CGI.pm manpage that you must 
close the filehandle, even on Windows.  That's just something that I've 
found from experience.

The bit that I quoted hints at a possible explanation:  It is actually 
talking about a "private_tempfiles" option that the mod_perl test script 
doesn't use, but it mentions that on _Unix_ a file can be unlinked while 
it is open.  The implication is that this perhaps can't be done on other 
OS's, and in practice I find that that is indeed the case on Windows, 
and probably explains what is going on here.

If I run this test script:

==========
use CGI;
my $cgi = CGI->new();
print    $cgi->header(),
    $cgi->start_html(),
    $cgi->start_multipart_form();
my $fh = $cgi->upload('upload');
if (defined $fh) {
    1 while <$fh>;
    print $cgi->p(sprintf("Read %d bytes", tell $fh));
}
else {
    print    $cgi->filefield({-name => 'upload'}),
        $cgi->submit();
}
print    $cgi->end_form(),
    $cgi->end_html();
==========

then the filehandle $fh gets left open, so when the DESTROY in CGI.pm's 
CGITempFile package runs and tries to unlink the temporary file it fails 
because you can't unlink a file while it is open on Windows.

In fact, adding some debug into that DESTROY subroutine:

    unlink $safe               # get rid of the file
        or print STDERR "Can't unlink '$safe': $!\n";

yields this error message in my Apache error.log:

    Can't unlink 'C:\temp\CGItemp34790': Permission denied

If I add

    close $fh;

to the end of the "if" block in my test script above then the error goes 
away and the file is deleted.

So I think the patch was well worth applying.

Sorry for any confusion.

- Steve


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


Re: [PATCH mp1.28] Clean up file uploaded by test suite

Posted by Stas Bekman <st...@stason.org>.
Steve Hay wrote:
[...]
>> I can't find in the CGI.pm manpage
> 
> 
> It's right after the bit that you quoted :-)  Lines 4439-4442 in the 
> CGI.pm of 2.97.
> 
>> , where it says that the filehandle should be closed, even on windows. 
>> Perhaps ask Lincoln?

Sorry, I don't understand. I said:

   I can't find in the CGI.pm manpage,  where it says that the filehandle
   should be closed, even on windows. Perhaps ask Lincoln?

I don't see where it says that on the lines that you've quoted.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


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


Re: [PATCH mp1.28] Clean up file uploaded by test suite

Posted by Steve Hay <st...@uk.radan.com>.
Stas Bekman wrote:

> Steve Hay wrote:
>
>> Stas Bekman wrote:
>>
>>> Steve Hay wrote:
>>>
>>>> Hi,
>>>>
>>>> After running the test suite with mp1.28 on Windows I find that I 
>>>> have a temporary file left behind in my temp dir (in my case, 
>>>> C:\Temp).
>>>>
>>>> This happens because the filehandle is left open.  The attached 
>>>> patch closes it, and the file gets deleted automatically later.
>>>
>>>
>>>
>>>
>>> I suppose we can/should do that, but isn't it a responsibility of 
>>> CGI.pm to do that?
>>>
>>>       -private_tempfiles
>>>            CGI.pm can process uploaded file. Ordinarily it spools 
>>> the uploaded
>>>            file to a temporary directory, then deletes the file when 
>>> done.
>>>                                           
>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>
>>> I remember this was a bug in the older CGI.pm versions, which was 
>>> fixed long time ago. I can't reproduce it with CGI 2.97. 
>>
>>
>>
>> I'm running CGI.pm 2.97 too.  I've had similar trouble with other 
>> software that I've written.  It may be a Windows-specific thing:
>>
>> "On Unix systems, the -private_tempfiles pragma will cause the 
>> temporary file to be unlinked as soon as it is opened and before any 
>> data is written into it ..."
>>    ^^^^
>
>
> Did you mean to underline the UNIX word?
>
> > "On Unix systems,
>       ^^^^^ 

Yes.  (That line wasn't wrapped when I wrote the mail, just afterwards 
when I sent it.)

>
>
> I can't find in the CGI.pm manpage

It's right after the bit that you quoted :-)  Lines 4439-4442 in the 
CGI.pm of 2.97.

> , where it says that the filehandle should be closed, even on windows. 
> Perhaps ask Lincoln?
>
> In any case, I have committed your patch, Steve. 

Thanks.


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


Re: [PATCH mp1.28] Clean up file uploaded by test suite

Posted by Stas Bekman <st...@stason.org>.
Steve Hay wrote:
> Stas Bekman wrote:
> 
>> Steve Hay wrote:
>>
>>> Hi,
>>>
>>> After running the test suite with mp1.28 on Windows I find that I 
>>> have a temporary file left behind in my temp dir (in my case, C:\Temp).
>>>
>>> This happens because the filehandle is left open.  The attached patch 
>>> closes it, and the file gets deleted automatically later.
>>
>>
>>
>> I suppose we can/should do that, but isn't it a responsibility of 
>> CGI.pm to do that?
>>
>>       -private_tempfiles
>>            CGI.pm can process uploaded file. Ordinarily it spools the 
>> uploaded
>>            file to a temporary directory, then deletes the file when 
>> done.
>>                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> I remember this was a bug in the older CGI.pm versions, which was 
>> fixed long time ago. I can't reproduce it with CGI 2.97. 
> 
> 
> I'm running CGI.pm 2.97 too.  I've had similar trouble with other 
> software that I've written.  It may be a Windows-specific thing:
> 
> "On Unix systems, the -private_tempfiles pragma will cause the temporary 
> file to be unlinked as soon as it is opened and before any data is 
> written into it ..."
>    ^^^^

Did you mean to underline the UNIX word?

 > "On Unix systems,
       ^^^^^

I can't find in the CGI.pm manpage, where it says that the filehandle should 
be closed, even on windows. Perhaps ask Lincoln?

In any case, I have committed your patch, Steve.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


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


Re: [PATCH mp1.28] Clean up file uploaded by test suite

Posted by Steve Hay <st...@uk.radan.com>.
Stas Bekman wrote:

> Steve Hay wrote:
>
>> Hi,
>>
>> After running the test suite with mp1.28 on Windows I find that I 
>> have a temporary file left behind in my temp dir (in my case, C:\Temp).
>>
>> This happens because the filehandle is left open.  The attached patch 
>> closes it, and the file gets deleted automatically later.
>
>
> I suppose we can/should do that, but isn't it a responsibility of 
> CGI.pm to do that?
>
>       -private_tempfiles
>            CGI.pm can process uploaded file. Ordinarily it spools the 
> uploaded
>            file to a temporary directory, then deletes the file when 
> done.
>                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> I remember this was a bug in the older CGI.pm versions, which was 
> fixed long time ago. I can't reproduce it with CGI 2.97. 

I'm running CGI.pm 2.97 too.  I've had similar trouble with other 
software that I've written.  It may be a Windows-specific thing:

"On Unix systems, the -private_tempfiles pragma will cause the temporary 
file to be unlinked as soon as it is opened and before any data is 
written into it ..."
    ^^^^

- Steve


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


Re: [PATCH mp1.28] Clean up file uploaded by test suite

Posted by Stas Bekman <st...@stason.org>.
Steve Hay wrote:
> Hi,
> 
> After running the test suite with mp1.28 on Windows I find that I have a 
> temporary file left behind in my temp dir (in my case, C:\Temp).
> 
> This happens because the filehandle is left open.  The attached patch 
> closes it, and the file gets deleted automatically later.

I suppose we can/should do that, but isn't it a responsibility of CGI.pm to do 
that?

       -private_tempfiles
            CGI.pm can process uploaded file. Ordinarily it spools the uploaded
            file to a temporary directory, then deletes the file when done.
                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I remember this was a bug in the older CGI.pm versions, which was fixed long 
time ago. I can't reproduce it with CGI 2.97.

> ------------------------------------------------------------------------
> 
> diff -ruN mod_perl-1.28.orig/t/net/perl/cgi.pl.PL mod_perl-1.28/t/net/perl/cgi.pl.PL
> --- mod_perl-1.28.orig/t/net/perl/cgi.pl.PL	2003-06-06 12:31:12.000000000 +0100
> +++ mod_perl-1.28/t/net/perl/cgi.pl.PL	2003-08-18 16:46:57.000000000 +0100
> @@ -19,4 +19,5 @@
>      local $/;
>      $content = <$httpupload>;
>      $r->print( "ok $content\n" );
> +    close $httpupload;
>  }
> 
> 
> 
> ------------------------------------------------------------------------
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
> For additional commands, e-mail: dev-help@perl.apache.org


-- 


__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


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