Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

open(FH, "+>>", undef) doesn't set O_APPEND #17058

Closed
p5pRT opened this issue Jun 24, 2019 · 12 comments
Closed

open(FH, "+>>", undef) doesn't set O_APPEND #17058

p5pRT opened this issue Jun 24, 2019 · 12 comments

Comments

@p5pRT
Copy link

p5pRT commented Jun 24, 2019

Migrated from rt.perl.org#134221 (status was 'pending release')

Searchable as RT134221$

@p5pRT
Copy link
Author

p5pRT commented Jun 24, 2019

From e@80x24.org

Created by e@80x24.org

I love using open(FH, MODE, undef) to create unlinked temporary
files with minimal typing.

But I was surprised when using "+>>" as the MODE did not set
O_APPEND on the temporary file handle (or warn about the ">>"
being ignored. I want to use O_APPEND so I can easily append to
it from one callback while while using sendfile or pread+SSL_write
to write to a socket.

I'm using this as a lazy write buffer for reading the output of
  process (e.g. git-http-backend/git-archive) while sending to
a client which may not be able to keep up with the output speed.

Thanks for keeping Perl 5 alive!

Perl Info

Flags:
    category=core
    severity=wishlist

Site configuration information for perl 5.24.1:

Configured by Debian Project at Thu Nov 29 11:11:57 UTC 2018.

Summary of my perl5 (revision 5 version 24 subversion 1) configuration:
   
  Platform:
    osname=linux, osvers=3.16.0, archname=i686-linux-gnu-thread-multi-64int
    uname='linux localhost 3.16.0 #1 smp debian 3.16.0 i686 gnulinux '
    config_args='-Dusethreads -Duselargefiles -Dcc=i686-linux-gnu-gcc -Dcpp=i686-linux-gnu-cpp -Dld=i686-linux-gnu-gcc -Dccflags=-DDEBIAN -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fdebug-prefix-map=/build/perl-s7MFkn/perl-5.24.1=. -fstack-protector-strong -Wformat -Werror=format-security -Dldflags= -Wl,-z,relro -Dlddlflags=-shared -Wl,-z,relro -Dcccdlflags=-fPIC -Darchname=i686-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.24 -Darchlib=/usr/lib/i386-linux-gnu/perl/5.24 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/i386-linux-gnu/perl5/5.24 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.24.1 -Dsitearch=/usr/local/lib/i386-linux-gnu/perl/5.24.1 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dusesitecustomize -Duse64bitint -Dman1ext=1 -Dman3ext=3perl
-Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -Ui_libutil -Uversiononly -DDEBUGGING=-g -Doptimize=-O2 -dEs -Duseshrplib -Dlibperl=libperl.so.5.24.1'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    use64bitint=define, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='i686-linux-gnu-gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2 -g',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include'
    ccversion='', gccversion='6.3.0 20170516', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=12345678, doublekind=3
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12, longdblkind=3
    ivtype='long long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define
  Linker and Libraries:
    ld='i686-linux-gnu-gcc', ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/gcc/i686-linux-gnu/6/include-fixed /usr/include/i386-linux-gnu /usr/lib /lib/i386-linux-gnu /lib/../lib /usr/lib/i386-linux-gnu /usr/lib/../lib /lib
    libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
    perllibs=-ldl -lm -lpthread -lc -lcrypt
    libc=libc-2.24.so, so=so, useshrplib=true, libperl=libperl.so.5.24
    gnulibc_version='2.24'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib -fstack-protector-strong'

Locally applied patches:
    DEBPKG:debian/cpan_definstalldirs - Provide a sensible INSTALLDIRS default for modules installed from CPAN.
    DEBPKG:debian/db_file_ver - https://bugs.debian.org/340047 Remove overly restrictive DB_File version check.
    DEBPKG:debian/doc_info - Replace generic man(1) instructions with Debian-specific information.
    DEBPKG:debian/enc2xs_inc - https://bugs.debian.org/290336 Tweak enc2xs to follow symlinks and ignore missing @INC directories.
    DEBPKG:debian/errno_ver - https://bugs.debian.org/343351 Remove Errno version check due to upgrade problems with long-running processes.
    DEBPKG:debian/libperl_embed_doc - https://bugs.debian.org/186778 Note that libperl-dev package is required for embedded linking
    DEBPKG:fixes/respect_umask - Respect umask during installation
    DEBPKG:debian/writable_site_dirs - Set umask approproately for site install directories
    DEBPKG:debian/extutils_set_libperl_path - EU:MM: set location of libperl.a under /usr/lib
    DEBPKG:debian/no_packlist_perllocal - Don't install .packlist or perllocal.pod for perl or vendor
    DEBPKG:debian/fakeroot - Postpone LD_LIBRARY_PATH evaluation to the binary targets.
    DEBPKG:debian/instmodsh_doc - Debian policy doesn't install .packlist files for core or vendor.
    DEBPKG:debian/ld_run_path - Remove standard libs from LD_RUN_PATH as per Debian policy.
    DEBPKG:debian/libnet_config_path - Set location of libnet.cfg to /etc/perl/Net as /usr may not be writable.
    DEBPKG:debian/mod_paths - Tweak @INC ordering for Debian
    DEBPKG:debian/prune_libs - https://bugs.debian.org/128355 Prune the list of libraries wanted to what we actually need.
    DEBPKG:fixes/net_smtp_docs - [rt.cpan.org #36038] https://bugs.debian.org/100195 Document the Net::SMTP 'Port' option
    DEBPKG:debian/perlivp - https://bugs.debian.org/510895 Make perlivp skip include directories in /usr/local
    DEBPKG:debian/deprecate-with-apt - https://bugs.debian.org/747628 Point users to Debian packages of deprecated core modules
    DEBPKG:debian/squelch-locale-warnings - https://bugs.debian.org/508764 Squelch locale warnings in Debian package maintainer scripts
    DEBPKG:debian/skip-upstream-git-tests - Skip tests specific to the upstream Git repository
    DEBPKG:debian/patchlevel - https://bugs.debian.org/567489 List packaged patches for 5.24.1-3+deb9u5 in patchlevel.h
    DEBPKG:debian/skip-kfreebsd-crash - https://bugs.debian.org/628493 [perl #96272] Skip a crashing test case in t/op/threads.t on GNU/kFreeBSD
    DEBPKG:fixes/document_makemaker_ccflags - https://bugs.debian.org/628522 [rt.cpan.org #68613] Document that CCFLAGS should include $Config{ccflags}
    DEBPKG:debian/find_html2text - https://bugs.debian.org/640479 Configure CPAN::Distribution with correct name of html2text
    DEBPKG:debian/perl5db-x-terminal-emulator.patch - https://bugs.debian.org/668490 Invoke x-terminal-emulator rather than xterm in perl5db.pl
    DEBPKG:debian/cpan-missing-site-dirs - https://bugs.debian.org/688842 Fix CPAN::FirstTime defaults with nonexisting site dirs if a parent is writable
    DEBPKG:fixes/memoize_storable_nstore - [rt.cpan.org #77790] https://bugs.debian.org/587650 Memoize::Storable: respect 'nstore' option not respected
    DEBPKG:debian/regen-skip - Skip a regeneration check in unrelated git repositories
    DEBPKG:debian/makemaker-pasthru - https://bugs.debian.org/758471 Pass LD settings through to subdirectories
    DEBPKG:debian/makemaker-manext - https://bugs.debian.org/247370 Make EU::MakeMaker honour MANnEXT settings in generated manpage headers
    DEBPKG:debian/devel-ppport-reproducibility - https://bugs.debian.org/801523 Sort the list of XS code files when generating RealPPPort.xs
    DEBPKG:debian/encode-unicode-bom-doc - https://bugs.debian.org/798727 Document Debian backport of Encode::Unicode fix
    DEBPKG:debian/kfreebsd-softupdates - https://bugs.debian.org/796798 Work around Debian Bug#796798
    DEBPKG:fixes/autodie-scope - https://bugs.debian.org/798096 Fix a scoping issue with "no autodie" and the "system" sub
    DEBPKG:fixes/crosscompile-no-targethost - [23695c0] [perl #127234] Fix the Configure escape with usecrosscompile but no targethost
    DEBPKG:fixes/memoize-pod - [rt.cpan.org #89441] Fix POD errors in Memoize
    DEBPKG:fixes/ok-pod - Added encoding for pod.
    DEBPKG:debian/hurd-softupdates - https://bugs.debian.org/822735 Fix t/op/stat.t failures on hurd
    DEBPKG:fixes/nntp_docs - https://bugs.debian.org/51962 Net::NNTP: Correct innd/nnrpd confusion in relation to Reader option
    DEBPKG:fixes/math_complex_doc_great_circle - https://bugs.debian.org/697567 [rt.cpan.org #114104] Math::Trig: clarify definition of great_circle_midpoint
    DEBPKG:fixes/math_complex_doc_see_also - https://bugs.debian.org/697568 [rt.cpan.org #114105] Math::Trig: add missing SEE ALSO
    DEBPKG:fixes/math_complex_doc_angle_units - https://bugs.debian.org/731505 [rt.cpan.org #114106] Math::Trig: document angle units
    DEBPKG:fixes/cpan_web_link - https://bugs.debian.org/367291 CPAN: Add link to main CPAN web site
    DEBPKG:fixes/time_piece_doc - https://bugs.debian.org/817925 Time::Piece: Improve documentation for add_months and add_years
    DEBPKG:fixes/perlbug-refactor - https://bugs.debian.org/822463 [perl #128020] perlbug: Refactor duplicated file reading code
    DEBPKG:fixes/perlbug-linewrap - https://bugs.debian.org/822463 [perl #128020] perlbug: wrap overly long lines
    DEBPKG:fixes/hurd_sigaction - https://bugs.debian.org/825016 [d54f4ed] ext/POSIX/t/sigaction.t: Skip uid and pid tests on GNU/Hurd
    DEBPKG:fixes/hurd_hints - [4694301] https://bugs.debian.org/825020 [perl #128279] Modify hints for Hurd per Debian ticket 825020.
    DEBPKG:fixes/extutils-parsexs-reproducibility - [perl #128517] https://bugs.debian.org/829296 Make the output of ExtUtils::ParseXS reproducible
    DEBPKG:debian/CVE-2016-1238/sitecustomize-in-etc - Look for sitecustomize.pl in /etc/perl rather than sitelib on Debian systems
    DEBPKG:debian/CVE-2016-1238/test-suite-without-dot - [perl #127810] Patch unit tests to explicitly insert "." into @INC when needed.
    DEBPKG:debian/CVE-2016-1238/eumm-without-dot - [perl #127810] Add PERL_USE_UNSAFE_INC support to EU::MM for fortify_inc support.
    DEBPKG:debian/CVE-2016-1238/cpan-without-dot - [perl #127810] Set PERL_USE_UNSAFE_INC for cpan usage
    DEBPKG:debian/document_inc_removal - Document in perlvar that we remove '.' from @INC by default
    DEBPKG:fixes/extutils_makemaker_reproducible - https://bugs.debian.org/835815 https://bugs.debian.org/834190 Make perllocal.pod files reproducible
    DEBPKG:debian/CVE-2016-1238/remove-inc-test - Remove test for '.' in @INC as it might not be
    DEBPKG:fixes/file_path_hurd_errno - File-Path: Fix test failure in Hurd due to hard-coded ENOENT
    DEBPKG:debian/hppa_op_optimize_workaround - https://bugs.debian.org/838613 Temporarily lower the optimization of op.c on hppa due to gcc-6 problems
    DEBPKG:fixes/test-builder-warning - https://bugs.debian.org/840968 Silence a 'used only once' warning in Test::Builder
    DEBPKG:fixes/longdblinf-randomness - [dd68853] [perl #130133] https://bugs.debian.org/844752 Configure: fix garbage filtering with 80-bit long doubles
    DEBPKG:debian/installman-utf8 - https://bugs.debian.org/840211 Generate man pages with UTF-8 characters
    DEBPKG:fixes/list_assign_leak - [1050723] [perl #130766] https://bugs.debian.org/855064 avoid a leak in list assign from/to magic values
    DEBPKG:fixes/perlfunc_inc_doc - [a03e9f8] https://bugs.debian.org/839536 [perl #130832] Documentation fixes for '.' possibly no longer being in @INC
    DEBPKG:fixes/file_path_chmod_race - https://bugs.debian.org/863870 [rt.cpan.org #121951] Prevent directory chmod race attack.
    DEBPKG:fixes/extutils_file_path_compat - Correct the order of tests of chmod(). (#294)
    DEBPKG:debian/customized - Update customized.dat for files patched in Debian
    DEBPKG:fixes/getopt-long-1 - https://bugs.debian.org/855532 [rt.cpan.org #114999] Fix bug RT#114999
    DEBPKG:fixes/getopt-long-2 - [rt.cpan.org #120300] Withdraw part of commit 5d9947fb445327c7299d8beb009d609bc70066c0, which tries to implement more GNU getopt_long campatibility. GNU
    DEBPKG:fixes/getopt-long-3 - provide a default value for optional arguments
    DEBPKG:fixes/getopt-long-4 - https://bugs.debian.org/864544 [rt.cpan.org #122068] Fix issue #122068.
    DEBPKG:fixes/fbm-instr-crash - [bb152a4] [perl #131575] https://bugs.debian.org/864782 don't call Perl_fbm_instr() with negative length
    DEBPKG:debian/CVE-2016-1238/base-pm-amends-pt2 - [1afa289] Limit dotless-INC effect on base.pm with guard:
    DEBPKG:fixes/CVE-2017-12837 - https://bugs.debian.org/875596 [perl #131582] [f7e5417] regcomp [perl #131582]
    DEBPKG:fixes/CVE-2017-12883 - https://bugs.debian.org/875597 [perl #131598] [40b3cda] PATCH: [perl #131598]
    DEBPKG:fixes/CVE-2018-6797 - [perl #132227] (perl #132227) restart a node if we change to uni rules within the node and encounter a sharp S
    DEBPKG:fixes/CVE-2018-6798/pt1 - [perl #132063] Heap buffer overflow
    DEBPKG:fixes/CVE-2018-6798/pt2 - [perl #132063] v5.24.3: fix TRIE_READ_CHAR and DECL_TRIE_TYPE to account for non-utf8 target
    DEBPKG:fixes/CVE-2018-6798/pt3 - [perl #132063] (perl #132063) we should no longer warn for this code
    DEBPKG:fixes/CVE-2018-6913 - [perl #131844] (perl #131844) fix various space calculation issues in pp_pack.c
    DEBPKG:fixes/CVE-2018-12015-Archive-Tar-directory-traversal - https://bugs.debian.org/900834 [rt.cpan.org #125523] Remove existing files before overwriting them
    DEBPKG:fixes/CVE-2018-18311 - Perl_my_setenv(); handle integer wrap
    DEBPKG:fixes/CVE-2018-18312 - for 5.26 maint
    DEBPKG:fixes/CVE-2018-18313 - regcomp.c: Convert some strchr to memchr
    DEBPKG:fixes/CVE-2018-18314 - fix #131649 - extended charclass can trigger assert


@INC for perl 5.24.1:
    /etc/perl
    /usr/local/lib/i386-linux-gnu/perl/5.24.1
    /usr/local/share/perl/5.24.1
    /usr/lib/i386-linux-gnu/perl5/5.24
    /usr/share/perl5
    /usr/lib/i386-linux-gnu/perl/5.24
    /usr/share/perl/5.24
    /usr/local/lib/site_perl
    /usr/lib/i386-linux-gnu/perl-base


Environment for perl 5.24.1:
    HOME=/home/ew
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/usr/lib/ccache:/home/ew/r/trunk/bin:/home/ew/bin:/usr/local/bin:/usr/bin:/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Jul 2, 2019

From @tonycoz

On Sun, 23 Jun 2019 18​:50​:24 -0700, e@​80x24.org wrote​:

I love using open(FH, MODE, undef) to create unlinked temporary
files with minimal typing.

But I was surprised when using "+>>" as the MODE did not set
O_APPEND on the temporary file handle (or warn about the ">>"
being ignored. I want to use O_APPEND so I can easily append to
it from one callback while while using sendfile or pread+SSL_write
to write to a socket.

I'm using this as a lazy write buffer for reading the output of
process (e.g. git-http-backend/git-archive) while sending to
a client which may not be able to keep up with the output speed.

I've attached a partial solution, that works on Linux (and presumably other POSIX-ish systems) and Win32.

From what I remember of a discussion with Craig back when working on the in-place editing re-work, the code currently guarded by !defined(VMS) should also work on VMS, except for the default to /tmp/.

Tony

@p5pRT
Copy link
Author

p5pRT commented Jul 2, 2019

From @tonycoz

open-temp-flags.diff

@p5pRT
Copy link
Author

p5pRT commented Jul 2, 2019

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Jul 2, 2019

From e@80x24.org

Tony Cook via RT <perlbug-followup@​perl.org> wrote​:

On Sun, 23 Jun 2019 18​:50​:24 -0700, e@​80x24.org wrote​:

I love using open(FH, MODE, undef) to create unlinked temporary
files with minimal typing.

But I was surprised when using "+>>" as the MODE did not set
O_APPEND on the temporary file handle (or warn about the ">>"
being ignored. I want to use O_APPEND so I can easily append to
it from one callback while while using sendfile or pread+SSL_write
to write to a socket.

I've attached a partial solution, that works on Linux (and
presumably other POSIX-ish systems) and Win32.

Thanks, I can confirm "+>>" works and sets O_APPEND with your
patches on top of 82651ab on
my FreeBSD 11.2 system.

However, there's a regression​:

"+>" (which previously worked and is documented) now fails
completely​:

  ./perl -e 'open(my $fh, "+&gt;", undef) or die "open​: $!"'

I couldn't even find the syscall being made via truss.

From what I remember of a discussion with Craig back when
working on the in-place editing re-work, the code currently
guarded by !defined(VMS) should also work on VMS, except for
the default to /tmp/.

I have no clue about non-POSIX-ish stuff :x

https://rt-archive.perl.org/perl5/Ticket/Display.html?id=134221

@p5pRT
Copy link
Author

p5pRT commented Jul 3, 2019

From @tonycoz

On Tue, 02 Jul 2019 00​:18​:35 -0700, e@​80x24.org wrote​:

Tony Cook via RT <perlbug-followup@​perl.org> wrote​:

On Sun, 23 Jun 2019 18​:50​:24 -0700, e@​80x24.org wrote​:

I love using open(FH, MODE, undef) to create unlinked temporary
files with minimal typing.

But I was surprised when using "+>>" as the MODE did not set
O_APPEND on the temporary file handle (or warn about the ">>"
being ignored. I want to use O_APPEND so I can easily append to
it from one callback while while using sendfile or pread+SSL_write
to write to a socket.

I've attached a partial solution, that works on Linux (and
presumably other POSIX-ish systems) and Win32.

Thanks, I can confirm "+>>" works and sets O_APPEND with your
patches on top of 82651ab on
my FreeBSD 11.2 system.

However, there's a regression​:

"+>" (which previously worked and is documented) now fails
completely​:

./perl -e 'open(my $fh, "+&gt;", undef) or die "open​: $!"'

I couldn't even find the syscall being made via truss.

Here's an updated patch.

Tony

@p5pRT
Copy link
Author

p5pRT commented Jul 3, 2019

From @tonycoz

open-temp-flags2.diff

@p5pRT
Copy link
Author

p5pRT commented Jul 3, 2019

From e@80x24.org

Tony Cook via RT <perlbug-followup@​perl.org> wrote​:

On Tue, 02 Jul 2019 00​:18​:35 -0700, e@​80x24.org wrote​:

However, there's a regression​:

"+>" (which previously worked and is documented) now fails
completely​:

./perl -e 'open(my $fh, "+&gt;", undef) or die "open​: $!"'

I couldn't even find the syscall being made via truss.

Here's an updated patch.

Looks good and confirmed both "+>" and "+>>" via truss output on
FreeBSD 11.2

@p5pRT
Copy link
Author

p5pRT commented Jul 16, 2019

From @tonycoz

On Tue, 02 Jul 2019 18​:34​:26 -0700, e@​80x24.org wrote​:

Tony Cook via RT <perlbug-followup@​perl.org> wrote​:

On Tue, 02 Jul 2019 00​:18​:35 -0700, e@​80x24.org wrote​:

However, there's a regression​:

"+>" (which previously worked and is documented) now fails
completely​:

./perl -e 'open(my $fh, "+&gt;", undef) or die "open​: $!"'

I couldn't even find the syscall being made via truss.

Here's an updated patch.

Looks good and confirmed both "+>" and "+>>" via truss output on
FreeBSD 11.2

Craig helped me out with the VMS support.

So​:

ae73d7e append mode for POSIX-like systems
0424723 for Win32
74b421c and VMS now (sort of) uses the same mkostemp() code that POSIX-like systems do.

Tony

@p5pRT
Copy link
Author

p5pRT commented Jul 16, 2019

@tonycoz - Status changed from 'open' to 'pending release'

@p5pRT p5pRT closed this as completed Jul 16, 2019
@p5pRT
Copy link
Author

p5pRT commented Jul 18, 2019

From @steve-m-hay

Unfortunately, this has broken VC++ builds on Windows (at least VC6 thru VC2015) - there is no O_ACCMODE here.

@p5pRT
Copy link
Author

p5pRT commented Jul 19, 2019

From @tonycoz

On Thu, 18 Jul 2019 12​:24​:20 -0700, shay wrote​:

Unfortunately, this has broken VC++ builds on Windows (at least VC6
thru VC2015) - there is no O_ACCMODE here.

Fixed by 52bd4b1.

Tony

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant