From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF shortcircuit=no autolearn=ham autolearn_force=no version=3.4.6 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 7458B1F47D for ; Fri, 7 Apr 2023 12:40:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1680871254; bh=WvR5kvnb2rPsNq8oM+OPGMUi0nrb1y6c187Q5KZOIpA=; h=From:To:Subject:Date:In-Reply-To:References:From; b=pgPYcQwD9tnJZSEPkPEW7vYCyC68LVr74wDb0FOKc/GXJ3Af4Bs4AOa3npuyqem1C 9QHEst8SmwpGS5MFZOtVrW9bIbvCqsdxlECjCucx+5fYjPGMw9ln3F/Z7QI0XzQusu SEip8ZEAsHDZdsoytO7/0o8BuZ8hymlJP30ptCuk= From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 3/6] umask: hoist out of InboxWritable Date: Fri, 7 Apr 2023 12:40:50 +0000 Message-Id: <20230407124053.2233988-4-e@80x24.org> In-Reply-To: <20230407124053.2233988-1-e@80x24.org> References: <20230407124053.2233988-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: Since CodeSearchIdx doesn't deal with inboxes, it makes sense to split it out from inbox-specific code and start moving towards using OnDestroy to restore the umask at the end of scope and reducing extra functions. --- MANIFEST | 1 + lib/PublicInbox/CodeSearchIdx.pm | 28 ++++--------- lib/PublicInbox/ExtSearchIdx.pm | 3 +- lib/PublicInbox/InboxWritable.pm | 70 +------------------------------- lib/PublicInbox/SearchIdx.pm | 6 ++- lib/PublicInbox/Umask.pm | 70 ++++++++++++++++++++++++++++++++ t/search.t | 7 ++-- 7 files changed, 91 insertions(+), 94 deletions(-) create mode 100644 lib/PublicInbox/Umask.pm diff --git a/MANIFEST b/MANIFEST index a0e64c6a..c338f0f4 100644 --- a/MANIFEST +++ b/MANIFEST @@ -340,6 +340,7 @@ lib/PublicInbox/TestCommon.pm lib/PublicInbox/Tmpfile.pm lib/PublicInbox/URIimap.pm lib/PublicInbox/URInntps.pm +lib/PublicInbox/Umask.pm lib/PublicInbox/Unsubscribe.pm lib/PublicInbox/UserContent.pm lib/PublicInbox/V2Writable.pm diff --git a/lib/PublicInbox/CodeSearchIdx.pm b/lib/PublicInbox/CodeSearchIdx.pm index 3a3fc03e..78032c00 100644 --- a/lib/PublicInbox/CodeSearchIdx.pm +++ b/lib/PublicInbox/CodeSearchIdx.pm @@ -770,28 +770,23 @@ sub init_tmp_git_dir ($) { sub prep_umask ($) { my ($self) = @_; - my $um; - my $cur = umask; if ($self->{-cidx_internal}) { # respect core.sharedRepository @{$self->{git_dirs}} == 1 or die 'BUG: only for GIT_DIR'; - # yuck, FIXME move umask handling out of inbox-specific stuff - require PublicInbox::InboxWritable; - my $git = PublicInbox::Git->new($self->{git_dirs}->[0]); - chomp($um = $git->qx('config', 'core.sharedRepository') // ''); - $um = PublicInbox::InboxWritable::_git_config_perm(undef, $um); - $um = PublicInbox::InboxWritable::_umask_for($um); - umask == $um or progress($self, 'umask from git: ', - sprintf('0%03o', $um)); + local $self->{git} = + PublicInbox::Git->new($self->{git_dirs}->[0]); + $self->with_umask; } elsif (-d $self->{cidx_dir}) { # respect existing perms my @st = stat(_); - $um = (~$st[2] & 0777); + my $um = (~$st[2] & 0777); + $self->{umask} = $um; # for SearchIdx->with_umask umask == $um or progress($self, 'using umask from ', $self->{cidx_dir}, ': ', sprintf('0%03o', $um)); - } - defined($um) ? - PublicInbox::OnDestroy->new(\&CORE::umask, umask($um)) : + PublicInbox::OnDestroy->new($$, \&CORE::umask, umask($um)); + } else { + $self->{umask} = umask; # for SearchIdx->with_umask undef; + } } sub start_prune ($) { @@ -896,9 +891,4 @@ sub shard_done_wait { # awaitpid cb via ipc_worker_reap PublicInbox::DS::enqueue_reap() if !shards_active(); # once more for PLC } -sub with_umask { # TODO get rid of this treewide and rely on OnDestroy - my ($self, $cb, @arg) = @_; - $cb->(@arg); -} - 1; diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm index 5445b156..6f3711ba 100644 --- a/lib/PublicInbox/ExtSearchIdx.pm +++ b/lib/PublicInbox/ExtSearchIdx.pm @@ -16,7 +16,7 @@ package PublicInbox::ExtSearchIdx; use strict; use v5.10.1; -use parent qw(PublicInbox::ExtSearch PublicInbox::Lock); +use parent qw(PublicInbox::ExtSearch PublicInbox::Umask PublicInbox::Lock); use Carp qw(croak carp); use Scalar::Util qw(blessed); use Sys::Hostname qw(hostname); @@ -1397,7 +1397,6 @@ sub eidx_watch { # public-inbox-extindex --watch main loop no warnings 'once'; *done = \&PublicInbox::V2Writable::done; -*with_umask = \&PublicInbox::InboxWritable::with_umask; *parallel_init = \&PublicInbox::V2Writable::parallel_init; *nproc_shards = \&PublicInbox::V2Writable::nproc_shards; *sync_prepare = \&PublicInbox::V2Writable::sync_prepare; diff --git a/lib/PublicInbox/InboxWritable.pm b/lib/PublicInbox/InboxWritable.pm index 022e2a69..65952aa2 100644 --- a/lib/PublicInbox/InboxWritable.pm +++ b/lib/PublicInbox/InboxWritable.pm @@ -1,25 +1,17 @@ -# Copyright (C) 2018-2021 all contributors +# Copyright (C) all contributors # License: AGPL-3.0+ # Extends read-only Inbox for writing package PublicInbox::InboxWritable; use strict; use v5.10.1; -use parent qw(PublicInbox::Inbox Exporter); +use parent qw(PublicInbox::Inbox PublicInbox::Umask Exporter); use PublicInbox::Import; use PublicInbox::Filter::Base qw(REJECT); use Errno qw(ENOENT); our @EXPORT_OK = qw(eml_from_path); use Fcntl qw(O_RDONLY O_NONBLOCK); -use constant { - PERM_UMASK => 0, - OLD_PERM_GROUP => 1, - OLD_PERM_EVERYBODY => 2, - PERM_GROUP => 0660, - PERM_EVERYBODY => 0664, -}; - sub new { my ($class, $ibx, $creat_opt) = @_; return $ibx if ref($ibx) eq $class; @@ -176,64 +168,6 @@ sub import_mbox { $im->done; } -sub _read_git_config_perm { - my ($self) = @_; - chomp(my $perm = $self->git->qx('config', 'core.sharedRepository')); - $perm; -} - -sub _git_config_perm { - my $self = shift; - my $perm = scalar @_ ? $_[0] : _read_git_config_perm($self); - return PERM_UMASK if (!defined($perm) || $perm eq ''); - return PERM_UMASK if ($perm eq 'umask'); - return PERM_GROUP if ($perm eq 'group'); - if ($perm =~ /\A(?:all|world|everybody)\z/) { - return PERM_EVERYBODY; - } - return PERM_GROUP if ($perm =~ /\A(?:true|yes|on|1)\z/); - return PERM_UMASK if ($perm =~ /\A(?:false|no|off|0)\z/); - - my $i = oct($perm); - return PERM_UMASK if ($i == PERM_UMASK); - return PERM_GROUP if ($i == OLD_PERM_GROUP); - return PERM_EVERYBODY if ($i == OLD_PERM_EVERYBODY); - - if (($i & 0600) != 0600) { - die "core.sharedRepository mode invalid: ". - sprintf('%.3o', $i) . "\nOwner must have permissions\n"; - } - ($i & 0666); -} - -sub _umask_for { - my ($perm) = @_; # _git_config_perm return value - my $rv = $perm; - return umask if $rv == 0; - - # set +x bit if +r or +w were set - $rv |= 0100 if ($rv & 0600); - $rv |= 0010 if ($rv & 0060); - $rv |= 0001 if ($rv & 0006); - (~$rv & 0777); -} - -sub with_umask { - my ($self, $cb, @arg) = @_; - my $old = umask($self->{umask} //= umask_prepare($self)); - my $rv = eval { $cb->(@arg) }; - my $err = $@; - umask $old; - die $err if $err; - $rv; -} - -sub umask_prepare { - my ($self) = @_; - my $perm = _git_config_perm($self); - _umask_for($perm); -} - sub cleanup ($) { delete @{$_[0]}{qw(over mm git search)}; } diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm index 699af432..69ab30e6 100644 --- a/lib/PublicInbox/SearchIdx.pm +++ b/lib/PublicInbox/SearchIdx.pm @@ -9,7 +9,8 @@ package PublicInbox::SearchIdx; use strict; use v5.10.1; -use parent qw(PublicInbox::Search PublicInbox::Lock Exporter); +use parent qw(PublicInbox::Search PublicInbox::Lock PublicInbox::Umask + Exporter); use PublicInbox::Eml; use PublicInbox::Search qw(xap_terms); use PublicInbox::InboxWritable; @@ -821,7 +822,8 @@ sub unindex_both { # git->cat_async callback sub with_umask { my $self = shift; - ($self->{ibx} // $self->{eidx})->with_umask(@_); + my $owner = $self->{ibx} // $self->{eidx}; + $owner ? $owner->with_umask(@_) : $self->SUPER::with_umask(@_) } # called by public-inbox-index diff --git a/lib/PublicInbox/Umask.pm b/lib/PublicInbox/Umask.pm new file mode 100644 index 00000000..00772ce5 --- /dev/null +++ b/lib/PublicInbox/Umask.pm @@ -0,0 +1,70 @@ +# Copyright (C) all contributors +# License: AGPL-3.0+ + +# base class to ensures Xapian||SQLite files respect core.sharedRepository +# of git repos +package PublicInbox::Umask; +use v5.12; +use PublicInbox::OnDestroy; + +use constant { + PERM_UMASK => 0, + OLD_PERM_GROUP => 1, + OLD_PERM_EVERYBODY => 2, + PERM_GROUP => 0660, + PERM_EVERYBODY => 0664, +}; + +sub _read_git_config_perm { + my ($self) = @_; + chomp(my $perm = $self->git->qx('config', 'core.sharedRepository')); + $perm; +} + +sub _git_config_perm { + my $self = shift; + my $perm = scalar @_ ? $_[0] : _read_git_config_perm($self); + $perm //= ''; + return PERM_UMASK if $perm eq '' || $perm eq 'umask'; + return PERM_GROUP if $perm eq 'group'; + return PERM_EVERYBODY if $perm =~ /\A(?:all|world|everybody)\z/; + return PERM_GROUP if ($perm =~ /\A(?:true|yes|on|1)\z/); + return PERM_UMASK if ($perm =~ /\A(?:false|no|off|0)\z/); + + my $i = oct($perm); + return PERM_UMASK if $i == PERM_UMASK; + return PERM_GROUP if $i == OLD_PERM_GROUP; + return PERM_EVERYBODY if $i == OLD_PERM_EVERYBODY; + + if (($i & 0600) != 0600) { + die "core.sharedRepository mode invalid: ". + sprintf('%.3o', $i) . "\nOwner must have permissions\n"; + } + ($i & 0666); +} + +sub _umask_for { + my ($perm) = @_; # _git_config_perm return value + my $rv = $perm; + return umask if $rv == 0; + + # set +x bit if +r or +w were set + $rv |= 0100 if ($rv & 0600); + $rv |= 0010 if ($rv & 0060); + $rv |= 0001 if ($rv & 0006); + (~$rv & 0777); +} + +sub with_umask { + my ($self, $cb, @arg) = @_; + my $old = umask($self->{umask} //= umask_prepare($self)); + my $restore = PublicInbox::OnDestroy->new($$, \&CORE::umask, $old); + $cb ? $cb->(@arg) : $restore; +} + +sub umask_prepare { + my ($self) = @_; + _umask_for(_git_config_perm($self)); +} + +1; diff --git a/t/search.t b/t/search.t index cf639a6d..ea5a2a5a 100644 --- a/t/search.t +++ b/t/search.t @@ -41,8 +41,9 @@ sub oct_is ($$$) { { # git repository perms + use_ok 'PublicInbox::Umask'; oct_is($ibx->_git_config_perm(), - &PublicInbox::InboxWritable::PERM_GROUP, + PublicInbox::Umask::PERM_GROUP(), 'undefined permission is group'); my @t = ( [ '0644', 0022, '644 => umask(0022)' ], @@ -54,8 +55,8 @@ sub oct_is ($$$) { ); for (@t) { my ($perm, $exp, $msg) = @$_; - my $got = PublicInbox::InboxWritable::_umask_for( - PublicInbox::InboxWritable->_git_config_perm($perm)); + my $got = PublicInbox::Umask::_umask_for( + PublicInbox::Umask->_git_config_perm($perm)); oct_is($got, $exp, $msg); } }