Item13172: CGI has removed CGITempFile package.
Priority: Normal
Current State: Closed
Released In: n/a
Target Release: minor
Subject says it all.
CGITempFile package does not exist.
- Unit tests fail.
UploadScriptTests
directly accesses CGITempFile->new()
-
Foswiki::Engine::CGI
fixed an upload temp file issue by overriding the now missing package.
See
http://github.com/foswiki/distro/commit/b8a8c318 and
Item12084. (
Item12084 patched
CGITempFile DESTROY routine to make sure forks to RCS made by Sandbox syscommand didn't remove the temp file before the upload was processed.)
See also
CPAN:CGI We probably need to fix this. CGI 4.05 has the multi_param security fixes referenced in a CVE, so distributions will most likely pick up this version.
Changes in temporary file handling (v4.05+)
CGI.pm had its temporary file handling significantly refactored. this logic is now all deferred to File::Temp (which is wrapped in a compatibility object, CGI::File::Temp - DO NOT USE THIS PACKAGE DIRECTLY). As a consequence the PRIVATE_TEMPFILES variable has been removed along with deprecation of the private_tempfiles routine and complete removal of the CGITempFile package. The $CGITempFile::TMPDIRECTORY is no longer used to set the temp directory, refer to the perldoc for File::Temp is you want to override the default settings in that package (the TMPDIR env variable is still available on some platforms). For Windows platforms the temporary directory order remains as before: TEMP > TMP > WINDIR ( > TMPDIR ) so if you have any of these in use in existing scripts they should still work.
The Fh package still exists but does nothing, the CGI::File::Temp class is a subclass of both File::Temp and the empty Fh package, so if you have any code that checks that the filehandle isa Fh this should still work.
When you get the internal file handle you will receive a File::Temp object, this should be transparent as File::Temp isa IO::Handle and isa IO::Seekable meaning it behaves as previously. if you are doing anything out of the ordinary with regards to temp files you should test your code before deploying this update and refer to the File::Temp documentation for more information.
--
GeorgeClark - 20 Dec 2014
(16:07:24) gac410: Trying to run unit tests on my perlbrew versions, found a possible reversion of your fix to http://foswiki.org/Tasks/Item12084 New versions of CGI have elimiated the CGITempFile
(16:08:24) gac410: I opened http://foswiki.org/Tasks/Item13172
(16:10:01) gac410: No idea. I don't really understand your fix for the internals. hooking CGITempFile::DESTROY won't work any more. So users of RCS stores will have issues uploading attachments if I understand the original issue
(16:10:43) CDot: the subprocesses close files that the parent process expects to keep open
(16:10:52) gac410: Right. So now that that package no longer exists, does that mean we have to hook into File::Temp somehow?
(16:11:01) CDot: I imagine the same will happen with File::Temp (though haven't tested it)
(16:12:51) CDot: try the arghno.pm code I wrote to test the issue on CGITemp
(16:15:01) CDot: still a problem on CGI 3.63
(16:15:28) gac410: CGITempFile was disabled in CGI 4.03 I think
(16:16:51) CDot: upstream seem to think it's fixed
(16:16:53) CDot: https://rt.cpan.org/Public/Bug/Display.html?id=89747
(16:17:04) CDot: do you have 4.03+ to test on?
(16:18:03) CDot: chances are it just needs a if($CGI::VERSION < 4.03) sort of test to ignore the monkey-patch of CGI::TempFile::DESTROY
(16:18:06) gac410: Yes. Any perl version where I installed CGI::Session has ended up with latest CGI as well due to wonders of cpanm
(17:32:01) gac410: Yeah it works fine if I give it a real file to upload.
(17:32:07) gac410: Even without the KEEP_ALL
(17:33:40) gac410: Excellent, So just disabling the monkey-patch if CGI >= 4.05
Looks like
Engine::CGI
already does an eval, and conditional monkey patch only when needed. However the unit test uses it to build the test files for upload.
--
GeorgeClark - 20 Dec 2014
Downgrading this to normal. Since Engine::CGI handles the new code, this is only a unit test failure.
--
GeorgeClark - 09 Jan 2015
This was fixed under the catch-all
Item12888. Closing.
--
GeorgeClark - 18 Jul 2015