You are viewing a plain text version of this content. The canonical link for it is here.
Posted to apreq-dev@httpd.apache.org by Randy Kobes <ra...@theoryx5.uwinnipeg.ca> on 2004/07/03 21:01:53 UTC
[apreq-2] $upload->tempname on Win32
The current cvs implementation of $upload->tempname on Win32
in the perl glue fails because, for some reason, although
the temp file gets created and populated OK in the
appropriate temp directory, it gets deleted prematurely. For
example, in src/apreq.c, if I remove the APR_DELONCLOSE flag
in the apr_file_mktemp() call in the apreq_file_mktemp()
sub, then the tempname tests in glue/perl/t/apreq/request.t
pass (save for a \r\n line-ending tweak). I couldn't see
though within apreq-2 at what point the deletion occurs;
apreq_xs_upload_tempname within Apache__Upload.h seems to
run to completion successfully.
I'll look at this more closely, but was just wondering if
this problem rang a bell with anyone? I know CGI.pm had the
opposite problem in leaving such uploaded temp files behind
on Win32 ...
--
best regards,
randy
Re: [apreq-2] $upload->tempname on Win32
Posted by Markus Wichitill <ma...@gmx.de>.
> I just looked over apr's Unix implementation of APR_DELONCLOSE,
> and it simply registers a pool cleanup handler that unlinks the
> file. Only Win32's open() actually supports this on the OS-level.
Maybe APR doesn't use the Unix filesystem behaviour for the compatibility
reasons I mentioned, but on a normal ext3 filesystem, when I rm a file, the
actual file will only be gone after any open handles are closed. To test,
run this in one shell (create a small file called "data" in the same dir)
open my $fh, "data";
while (1) {
seek($fh, 0, 0);
print <$fh>;
sleep 2;
}
and in another shell, rm the file while the script runs. ls won't show the
file anymore, but the script will continue to work. "lsof | grep perl" will
list the file as "/tmp/data (deleted)". This is also one of the reasons why
you can overwrite binaries while they're running on Unix.
Re: [apreq-2] $upload->tempname on Win32
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Joe Schaefer <jo...@sunstarsys.com> writes:
[...]
> Keep in mind that apreq2 has no reason to assume whatever cleanup
> handlers we install will ever be run. The server can abort prematurely
> on a signal, segfault, or an untrapped exception. If we start leaving our
> tempfiles lying around whenever that happens, we create an opportunity
> for a DoS attack.
I just looked over apr's Unix implementation of APR_DELONCLOSE,
and it simply registers a pool cleanup handler that unlinks the
file. Only Win32's open() actually supports this on the OS-level.
So I'm +0 for dumbing down the Win32 port to Unix's level by removing
that flag from apreq_file_mktemp and having it register a pool cleanup
handler that unlinks the tempfile.
--
Joe Schaefer
Re: [apreq-2] $upload->tempname on Win32
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Markus Wichitill <ma...@gmx.de> writes:
[...]
> The practical problem on Win32 is that I can't pass $upload->tempname to
> someone else's code (maybe an external program that can't accept handles)
> since that code won't use APR::PerlIO and therefore won't be able to make
> any use of the exclusively locked tempfile. I haven't tested $upload->link
> yet, but I'd assume a created NTFS hardlink will be similarly useless
> during the request, if the linking works at all.
New link() tests added- please run the test suite and see what happens
on Win32.
> I don't know enough of the internals to actually suggest anything. I'm just
> wondering if relying on the OS to delete the file doesn't make things more
> complicated than necessary, and who knows what other problems could come up
> with other filesystems (NFS? SMB shares? What do I know).
Keep in mind that apreq2 has no reason to assume whatever cleanup
handlers we install will ever be run. The server can abort prematurely
on a signal, segfault, or an untrapped exception. If we start leaving our
tempfiles lying around whenever that happens, we create an opportunity
for a DoS attack.
--
Joe Schaefer
Re: [apreq-2] $upload->tempname on Win32
Posted by Markus Wichitill <ma...@gmx.de>.
>> From the discussion so far, I'll once again point out that
>> everyone seems to be able to open $upload->tempname using
>> either "<" or "<:APR". The associated issues need to be
>> documented (I'm certainly _not_ volunteering for that),
>> and if there's a way to make the decision programatically,
>> let's please include that in our fh() implementation so our
>> users don't have to worry about making the right choice.
> I'm not sure what would be gained either (I assume this is
> all a Win32 issue). Markus, are you suggesting that the
> APR_DELONCLOSE flag used in the apr_file_mktemp() call
> within apreq_file_mktemp() of src/apreq.c be dropped, and
> that the temp files subsequently left around be cleaned up
> explicitly by apreq? That would allow Perl's open() to be
> used, instead of the current demand of APR::PerlIO, in this
> context, but would there be a strong reason for doing this?
> APR::PerlIO is already available, and Joe's recent fh()
> implementation within Apache::Upload already returns a
> filehandle using APR::PerlIO behind the scenes.
The practical problem on Win32 is that I can't pass $upload->tempname to
someone else's code (maybe an external program that can't accept handles)
since that code won't use APR::PerlIO and therefore won't be able to make
any use of the exclusively locked tempfile. I haven't tested $upload->link
yet, but I'd assume a created NTFS hardlink will be similarly useless during
the request, if the linking works at all.
I don't know enough of the internals to actually suggest anything. I'm just
wondering if relying on the OS to delete the file doesn't make things more
complicated than necessary, and who knows what other problems could come up
with other filesystems (NFS? SMB shares? What do I know).
Re: [apreq-2] $upload->tempname on Win32
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Sun, 4 Jul 2004, Joe Schaefer wrote:
> Markus Wichitill <ma...@gmx.de> writes:
>
> [...]
>
> > Would it be possible to delete the tempfile in a cleanup handler
> > instead of relying on flags like FILE_FLAG_DELETE_ON_CLOSE and Unix
> > filesystem behaviour?
>
> Seems unlikely that'll improve anything. The tempfile
> is closed by a pool cleanup handler, and the OS subsequently
> unlinks it *then* because of the flags apreq_mktemp() sets when
> the file was first created/opened. I don't see anything
> particularly Unix-specific about this situation; we're simply
> relying on apr to provide the C portability, and mp2 to provide
> access to it from perl.
>
> From the discussion so far, I'll once again point out that
> everyone seems to be able to open $upload->tempname using
> either "<" or "<:APR". The associated issues need to be
> documented (I'm certainly _not_ volunteering for that),
> and if there's a way to make the decision programatically,
> let's please include that in our fh() implementation so our
> users don't have to worry about making the right choice.
I'm not sure what would be gained either (I assume this is
all a Win32 issue). Markus, are you suggesting that the
APR_DELONCLOSE flag used in the apr_file_mktemp() call
within apreq_file_mktemp() of src/apreq.c be dropped, and
that the temp files subsequently left around be cleaned up
explicitly by apreq? That would allow Perl's open() to be
used, instead of the current demand of APR::PerlIO, in this
context, but would there be a strong reason for doing this?
APR::PerlIO is already available, and Joe's recent fh()
implementation within Apache::Upload already returns a
filehandle using APR::PerlIO behind the scenes.
--
best regards,
randy
Re: [apreq-2] $upload->tempname on Win32
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Markus Wichitill <ma...@gmx.de> writes:
[...]
> Would it be possible to delete the tempfile in a cleanup handler
> instead of relying on flags like FILE_FLAG_DELETE_ON_CLOSE and Unix
> filesystem behaviour?
Seems unlikely that'll improve anything. The tempfile
is closed by a pool cleanup handler, and the OS subsequently
unlinks it *then* because of the flags apreq_mktemp() sets when
the file was first created/opened. I don't see anything
particularly Unix-specific about this situation; we're simply
relying on apr to provide the C portability, and mp2 to provide
access to it from perl.
>From the discussion so far, I'll once again point out that
everyone seems to be able to open $upload->tempname using
either "<" or "<:APR". The associated issues need to be
documented (I'm certainly _not_ volunteering for that),
and if there's a way to make the decision programatically,
let's please include that in our fh() implementation so our
users don't have to worry about making the right choice.
--
Joe Schaefer
Re: [apreq-2] $upload->tempname on Win32
Posted by Markus Wichitill <ma...@gmx.de>.
> sub fh {
> my $upload = shift;
> open my $fh, "<:APR", $upload->tempname, $upload->pool or die $!;
> return $fh;
> }
Now that I've got MP2-CVS compiled, I've tested this implementation on
WinXP, too. While $upload->fh works, $upload->tempname can't be opened the
normal way anymore, since it's completely locked (and I'm not using
$upload->fh in parallel or anything):
$upload = $apr->upload('file');
open my $fh, "<:APR", $upload->tempname, $upload->pool or die $!;
# Ok
$upload = $apr->upload('file');
open my $fh, "<", $upload->tempname or die $!;
# Permission denied
Would it be possible to delete the tempfile in a cleanup handler instead of
relying on flags like FILE_FLAG_DELETE_ON_CLOSE and Unix filesystem
behaviour?
Re: [apreq-2] $upload->tempname on Win32
Posted by Stas Bekman <st...@stason.org>.
Markus Wichitill wrote:
>>>+ open my $fh, "<:APR", $upload->tempname, $p or die $!;
>
>
>>The difficult request.t tests will still pass, and users
>>will get a seekable filehandle just like the one from apreq1.
>
>
> How many people will really get a seekable APR::PerlIO handle? My SuSE 9.0
> box is a run-of-the-mill Linux system, and those handles are not seekable
> because Perl by default compiles with largefile support and APR doesn't.
>
> "[Sun Jul 04 18:57:14 2004] [error] PerlIO::APR::seek with non-zero offsetis
> not supported with Perl built w/ -Duselargefiles and APR w/o largefiles
> support"
>
> And, as Stas confirmed, it's Perl 5.8 only. Not that I care much about 5.6
> personally.
You could manually tweak the APR source to enable it. Or recompile perl to
disable it.
Every time I have raised the issue on apr-dev, the problem was the sendfile
interface. It's simply hardcoded to disable LFS on linux. Apparently no-one
over apr-dev cares about the LFS issue to fix it. I suppose if more people
start complaining posting directly to apr-dev, it will get fixed sooner.
--
__________________________________________________________________
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
Re: [apreq-2] $upload->tempname on Win32
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Sun, 4 Jul 2004, Joe Schaefer wrote:
> Markus Wichitill <ma...@gmx.de> writes:
>
> [...]
> > How many people will really get a seekable APR::PerlIO
> > handle? My SuSE 9.0 box is a run-of-the-mill Linux
> > system, and those handles are not seekable because Perl
> > by default compiles with largefile support and APR
> > doesn't.
> >
> > "[Sun Jul 04 18:57:14 2004] [error] PerlIO::APR::seek
> > with non-zero offsetis not supported with Perl built w/
> > -Duselargefiles and APR w/o largefiles support"
>
> IMO what we need now (since I've already committed the
> change) is for fh() to decide when to pass "<:APR" to
> open, and when to pass "<".
>
> Patches only, please. At this point I'm very tired of
> futzing with fh() support, because these problems are
> really not within apreq2's domain.
Not quite a patch yet, as I'm not sure I appreciate
everything involved. mp2's Apache::Build has a sub
has_large_files_conflict(), with the embedded comments
indicating that it decides whether or not such a conflict
exists. If that's the case, then could something like the
following:
=======================================================
#!/opt/bin/perl
use strict;
use warnings;
use Apache2;
use Apache::Build();
use Config;
$Apache::Build::APXS = '/opt/httpd/bin/apxs';
print has_large_files_conflict();
sub has_large_files_conflict {
my $b = Apache::Build->new();
my $apxs_flags = join $b->apxs('-q' => 'EXTRA_CFLAGS'),
$b->apxs('-q' => 'EXTRA_CPPFLAGS');
my $apr_lfs64 = $apxs_flags =~ /-D_FILE_OFFSET_BITS=64/;
my $perl_lfs64 = $Config{ccflags} =~ /-D_FILE_OFFSET_BITS=64/;
return $perl_lfs64 ^ $apr_lfs64;
}
=================================================================
(with $Apache::Build::APXS adjusted to the given apxs
utility) be used to decide if a conflict exists, and
if so, use '<', rather than '<:APR', in constructing
the filehandle in Apache::Upload's fh()?
--
best regards,
randy
Re: [apreq-2] $upload->tempname on Win32
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Markus Wichitill <ma...@gmx.de> writes:
[...]
> How many people will really get a seekable APR::PerlIO handle? My SuSE
> 9.0 box is a run-of-the-mill Linux system, and those handles are not
> seekable because Perl by default compiles with largefile support and
> APR doesn't.
>
> "[Sun Jul 04 18:57:14 2004] [error] PerlIO::APR::seek with non-zero
> offsetis not supported with Perl built w/ -Duselargefiles and APR w/o
> largefiles support"
IMO what we need now (since I've already committed the change) is for
fh() to decide when to pass "<:APR" to open, and when to pass "<".
Patches only, please. At this point I'm very tired of futzing
with fh() support, because these problems are really not within
apreq2's domain.
--
Joe Schaefer
Re: [apreq-2] $upload->tempname on Win32
Posted by Markus Wichitill <ma...@gmx.de>.
>> + open my $fh, "<:APR", $upload->tempname, $p or die $!;
> The difficult request.t tests will still pass, and users
> will get a seekable filehandle just like the one from apreq1.
How many people will really get a seekable APR::PerlIO handle? My SuSE 9.0
box is a run-of-the-mill Linux system, and those handles are not seekable
because Perl by default compiles with largefile support and APR doesn't.
"[Sun Jul 04 18:57:14 2004] [error] PerlIO::APR::seek with non-zero offsetis
not supported with Perl built w/ -Duselargefiles and APR w/o largefiles
support"
And, as Stas confirmed, it's Perl 5.8 only. Not that I care much about 5.6
personally.
Re: [apreq-2] $upload->tempname on Win32
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:
[...]
> ================================================================
> With this, all the glue/perl/t/apreq/request tests pass for
> me (without any tweaking of \r\n issues I had before). We
> may not want to change the test to do this (but rather, just
> skip it for Win32), but in any case, there does seem to be a
> well-defined way to manipulate $upload->tempname on Win32.
Excellent! I think it's great that APR::*
is able to handle this situation so easily.
+1 to the patch, since it's a portable solution that
we should definitely promote. It'd also be nice if
this issue was mentioned in our perldocs (and FAQ.pod
too).
--
Joe Schaefer
Re: [apreq-2] $upload->tempname on Win32
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:
>
> [...]
>
>
>>+ open my $fh, "<:APR", $upload->tempname, $p or die $!;
>
>
> Since this is a nicely portable solution and
> should be perfectly seekable, how would folks
> feel about implementing fh() like so:
>
> sub fh {
> my $upload = shift;
> open my $fh, "<:APR", $upload->tempname, $upload->pool or die $!;
At the moment APR::Perlio suffers from the pool object disappearance, before
$fh is finished to be used. But I suppose since $upload->pool has the same
life-scope as fh, it should work just fine.
--
__________________________________________________________________
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
Re: [apreq-2] $upload->tempname on Win32
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:
[...]
> + open my $fh, "<:APR", $upload->tempname, $p or die $!;
Since this is a nicely portable solution and
should be perfectly seekable, how would folks
feel about implementing fh() like so:
sub fh {
my $upload = shift;
open my $fh, "<:APR", $upload->tempname, $upload->pool or die $!;
return $fh;
}
(Adding a ->pool() method is straightforward enough).
The difficult request.t tests will still pass, and users
will get a seekable filehandle just like the one from apreq1.
We can just rename the current fh() implementation io(), for
the performance freaks who aren't afraid to dabble in brigades.
Thoughts?
--
Joe Schaefer
Re: [apreq-2] $upload->tempname on Win32
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Sat, 3 Jul 2004, Joe Schaefer wrote:
> Joe Schaefer <jo...@sunstarsys.com> writes:
>
> > Joe Schaefer <jo...@sunstarsys.com> writes:
> >
> > [...]
> >
> > > Do you know which file in apr I should be looking at?
> >
> > Never mind, I found it- the relevant files are
> > apr/apr_file_io/unix/mktemp.c and apr/apr_file_io/win32/open.c.
> >
> > According to msdn FILE_FLAG_DELETE_ON_CLOSE requires
> > FILE_SHARE_DELETE to permit reopening the file. It looks
> > like that's being added at line 227 in open.c:
> >
> > if (apr_os_level >= APR_WIN_NT)
> > sharemode |= FILE_SHARE_DELETE;
> >
> > Not sure if this conditional is being satisfied, you
> > might want to start by double-checking it.
>
> Assuming apr is doing this correctly, is there
> a way to open the file in Perl with FILE_SHARE_DELETE
> set? It could be that the test script's attempt
> to reopen the tempfile fails because Perl's open()
> isn't passing this flag to the underlying system call.
>
> Otherwise I think we should just document this issue
> and xfail some portion of the tempname tests on Win32.
Thanks, Joe - that looks exactly what's happening. There is
a Win32 module that gives access to FILE_SHARE_DELETE, but
based upon your tracking things down, I think things stay
consistent if we let apr handle things as much as possible.
So I tried, at least for this test, to use APR::PerlIO to
handle the opening:
===================================================
Index: t/response/TestApreq/request.pm
===================================================================
RCS file: /home/cvs/httpd-apreq-2/glue/perl/t/response/TestApReq/request.pm,v
retrieving revision 1.17
diff -u -r1.17 request.pm
--- t/response/TestApreq/request.pm 2 Jul 2004 20:50:16 -0000 1.17
+++ t/response/TestApreq/request.pm 3 Jul 2004 22:39:09 -0000
@@ -8,6 +8,10 @@
use Apache::Request ();
use Apache::Connection;
use Apache::Upload;
+use APR::Pool;
+use APR::PerlIO;
+
+my $p = APR::Pool->new();
sub handler {
my $r = shift;
@@ -62,7 +66,7 @@
}
elsif ($test eq 'tempname') {
my $upload = $req->upload("HTTPUPLOAD");
- open my $fh, "<", $upload->tempname or die $!;
+ open my $fh, "<:APR", $upload->tempname, $p or die $!;
$r->print(<$fh>);
}
elsif ($test eq 'bad') {
================================================================
With this, all the glue/perl/t/apreq/request tests pass for
me (without any tweaking of \r\n issues I had before). We
may not want to change the test to do this (but rather, just
skip it for Win32), but in any case, there does seem to be a
well-defined way to manipulate $upload->tempname on Win32.
Great work!
--
best regards,
randy
Re: [apreq-2] $upload->tempname on Win32
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Joe Schaefer <jo...@sunstarsys.com> writes:
> Joe Schaefer <jo...@sunstarsys.com> writes:
>
> [...]
>
> > Do you know which file in apr I should be looking at?
>
> Never mind, I found it- the relevant files are
> apr/apr_file_io/unix/mktemp.c and apr/apr_file_io/win32/open.c.
>
> According to msdn FILE_FLAG_DELETE_ON_CLOSE requires
> FILE_SHARE_DELETE to permit reopening the file. It looks
> like that's being added at line 227 in open.c:
>
> if (apr_os_level >= APR_WIN_NT)
> sharemode |= FILE_SHARE_DELETE;
>
> Not sure if this conditional is being satisfied, you
> might want to start by double-checking it.
Assuming apr is doing this correctly, is there
a way to open the file in Perl with FILE_SHARE_DELETE
set? It could be that the test script's attempt
to reopen the tempfile fails because Perl's open()
isn't passing this flag to the underlying system call.
Otherwise I think we should just document this issue
and xfail some portion of the tempname tests on Win32.
--
Joe Schaefer
Re: [apreq-2] $upload->tempname on Win32
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Joe Schaefer <jo...@sunstarsys.com> writes:
[...]
> Do you know which file in apr I should be looking at?
Never mind, I found it- the relevant files are
apr/apr_file_io/unix/mktemp.c and apr/apr_file_io/win32/open.c.
According to msdn FILE_FLAG_DELETE_ON_CLOSE requires
FILE_SHARE_DELETE to permit reopening the file. It looks
like that's being added at line 227 in open.c:
if (apr_os_level >= APR_WIN_NT)
sharemode |= FILE_SHARE_DELETE;
Not sure if this conditional is being satisfied, you
might want to start by double-checking it.
--
Joe Schaefer
Re: [apreq-2] $upload->tempname on Win32
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:
> The current cvs implementation of $upload->tempname on Win32
> in the perl glue fails because, for some reason, although
> the temp file gets created and populated OK in the
> appropriate temp directory, it gets deleted prematurely. For
> example, in src/apreq.c, if I remove the APR_DELONCLOSE flag
> in the apr_file_mktemp() call in the apreq_file_mktemp()
> sub, then the tempname tests in glue/perl/t/apreq/request.t
> pass (save for a \r\n line-ending tweak).
apr_file_mktemp might DTRT if we pass it a flags = 0 argument in
apreq_file_mktemp. That seems to work on linux, but I'm concerned
about the APR_BINARY option on Win32.
> I couldn't see though within apreq-2 at what point the deletion
> occurs;
It may be removing the tempfile immediately after opening it,
but I can't track down exactly where apr_file_mktemp is actually
implemented for Win32. Do you know which file in apr I should
be looking at?
> apreq_xs_upload_tempname within Apache__Upload.h seems to run to
> completion successfully.
Not surprising- it's just pulling out the name used when
the tempfile was created.
--
Joe Schaefer