Skip Menu |
 

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the CGI CPAN distribution.

Report information
The Basics
Id: 89747
Status: resolved
Priority: 0/
Queue: CGI

People
Owner: Nobody in particular
Requestors: cpan [...] c-dot.co.uk
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: Subprocess created using safe pipes wipes out temp files
Download (untitled) / with headers
text/plain 1.6k
perl 5.14.2, CGI 3.52 If a short-lived subprocess is created using safe pipes after a CGI instance has already been created, the cleanup of the subprocess will demolish the tempfiles used by the parent. Here's a short script (arghno.pl) that demonstrates the problem: #!/usr/bin/perl use strict; use CGI; use CGI::Carp ('fatalsToBrowser'); my $q = new CGI; print CGI::header(); my $fn = $q->param('filepath'); my $f = $q->tmpFileName($fn); my $handle; my $pid = open($handle, "-|"); if ($pid) { print "This is the parent"; die "Parent: Tmpfile has gone before reading!" unless -e $f; my $data = <$handle>; die "Parent: Tmpfile has gone after reading!" unless -e $f; } else { # This is the child print "This is the child"; } Invoke with the following HTML: <html> <body> <form enctype="multipart/form-data" action="arghno.pl" method="post"> <input type="file" name="filepath" value="" /> <input type="submit" value='Upload file' /> </form> </body> </html> The temp files are removed in the CGITempFile::DESTROY method, when it is invoked during subprocess cleanup. Curiously enough the problem is not seen when using fork() to create the subprocess. I was able to work around this problem by monkey-patching CGITempFile::DESTROY so that it was only invoked when called from the parent process, thus: our $CONSTRUCTOR_PID = $$; our $SAVE_DESTROY = \&CGITempFile::DESTROY; if ( defined $SAVE_DESTROY ) { no warnings 'redefine'; *CGITempFile::DESTROY = sub { if ( $$ == $CONSTRUCTOR_PID ) { # Parent process, unlink the temp file &$SAVE_DESTROY(@_); } }; } my $query - new CGI(); etc.
Download (untitled) / with headers
text/plain 241b
This issue has been copied to: https://github.com/leejo/CGI.pm/issues/125 please take all future correspondence there. This ticket will remain open but please do not reply here. This ticket will be closed when the github issue is dealt with.
Download (untitled) / with headers
text/plain 1.4k
commit 1aa4cb0262103d7d0f657099bb54a557ce1b237b Author: Lee Johnson <lee@givengain.ch> Date: Sun Sep 28 20:46:46 2014 +0100 resolve #125, #131 - refactor temp file handling the CGITempFile and Fh packages are gone and have been replaced by CGI::File::Temp, which is a compatibility wrapper around the File::Temp package. File::Temp behaves as a filehandle, and isa IO::Handle and isa IO::Seekable so behaves as we want it to bump version to 4.04_02 for cpantesters dev release testing Squashed commit of the following: commit 3af477dacef09d7d01086a6fb3984471e6892c9e Author: Lee Johnson <lee@givengain.ch> Date: Sun Sep 28 20:45:32 2014 +0100 temp files done commit 982fc37665a8754d09e7f5952bc04a736ebcfc26 Author: Lee Johnson <lee@givengain.ch> Date: Thu Sep 25 10:55:33 2014 +0100 WIP - refactor CGITempFile and Fh to File::Temp Changes | 8 ++-- MANIFEST | 1 - Makefile.PL | 19 ++++----- lib/CGI.pm | 328 ++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------- lib/CGI/Carp.pm | 2 +- lib/CGI/Cookie.pm | 2 +- lib/CGI/Pretty.pm | 2 +- lib/CGI/Push.pm | 2 +- lib/CGI/Util.pm | 2 +- t/rt-31107.t | 12 +++++- t/tmpdir.t | 51 ------------------------ t/upload.t | 2 +- 12 files changed, 103 insertions(+), 328 deletions(-)
Download (untitled) / with headers
text/plain 1.4k
commit 1aa4cb0262103d7d0f657099bb54a557ce1b237b Author: Lee Johnson <lee@givengain.ch> Date: Sun Sep 28 20:46:46 2014 +0100 resolve #125, #131 - refactor temp file handling the CGITempFile and Fh packages are gone and have been replaced by CGI::File::Temp, which is a compatibility wrapper around the File::Temp package. File::Temp behaves as a filehandle, and isa IO::Handle and isa IO::Seekable so behaves as we want it to bump version to 4.04_02 for cpantesters dev release testing Squashed commit of the following: commit 3af477dacef09d7d01086a6fb3984471e6892c9e Author: Lee Johnson <lee@givengain.ch> Date: Sun Sep 28 20:45:32 2014 +0100 temp files done commit 982fc37665a8754d09e7f5952bc04a736ebcfc26 Author: Lee Johnson <lee@givengain.ch> Date: Thu Sep 25 10:55:33 2014 +0100 WIP - refactor CGITempFile and Fh to File::Temp Changes | 8 ++-- MANIFEST | 1 - Makefile.PL | 19 ++++----- lib/CGI.pm | 328 ++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------- lib/CGI/Carp.pm | 2 +- lib/CGI/Cookie.pm | 2 +- lib/CGI/Pretty.pm | 2 +- lib/CGI/Push.pm | 2 +- lib/CGI/Util.pm | 2 +- t/rt-31107.t | 12 +++++- t/tmpdir.t | 51 ------------------------ t/upload.t | 2 +- 12 files changed, 103 insertions(+), 328 deletions(-)
Subject: Re: [rt.cpan.org #89747] Subprocess created using safe pipes wipes out temp files
Date: Sun, 28 Sep 2014 14:50:05 -0700
To: Lee Johnson via RT <bug-CGI [...] rt.cpan.org>
From: Mark Stosberg <mark [...] stosberg.com>
Download (untitled) / with headers
text/plain 1.8k
I recommend keeping Fh as an empty subclass of the new package. I have seen code that explicitly checks for the Fh class. Mark On Sun, Sep 28, 2014, at 12:55 PM, Lee Johnson via RT wrote: Show quoted text
> Queue: CGI > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=89747 > > > commit 1aa4cb0262103d7d0f657099bb54a557ce1b237b > Author: Lee Johnson <lee@givengain.ch> > Date: Sun Sep 28 20:46:46 2014 +0100 > > resolve #125, #131 - refactor temp file handling > > the CGITempFile and Fh packages are gone and have been replaced by > CGI::File::Temp, which is a compatibility wrapper around the > File::Temp > package. File::Temp behaves as a filehandle, and isa IO::Handle and > isa IO::Seekable so behaves as we want it to > > bump version to 4.04_02 for cpantesters dev release testing > > Squashed commit of the following: > > commit 3af477dacef09d7d01086a6fb3984471e6892c9e > Author: Lee Johnson <lee@givengain.ch> > Date: Sun Sep 28 20:45:32 2014 +0100 > > temp files done > > commit 982fc37665a8754d09e7f5952bc04a736ebcfc26 > Author: Lee Johnson <lee@givengain.ch> > Date: Thu Sep 25 10:55:33 2014 +0100 > > WIP - refactor CGITempFile and Fh to File::Temp > > Changes | 8 ++-- > MANIFEST | 1 - > Makefile.PL | 19 ++++----- > lib/CGI.pm | 328 > ++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------- > lib/CGI/Carp.pm | 2 +- > lib/CGI/Cookie.pm | 2 +- > lib/CGI/Pretty.pm | 2 +- > lib/CGI/Push.pm | 2 +- > lib/CGI/Util.pm | 2 +- > t/rt-31107.t | 12 +++++- > t/tmpdir.t | 51 ------------------------ > t/upload.t | 2 +- > 12 files changed, 103 insertions(+), 328 deletions(-)
Download (untitled) / with headers
text/plain 212b
On Sun Sep 28 17:50:21 2014, mark@stosberg.com wrote: Show quoted text
> I recommend keeping Fh as an empty subclass of the new package. I have > seen code that explicitly checks for the Fh class. > > Mark
Thanks Mark, will do.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.