diff --git a/core/lib/Foswiki/Users/HtPasswdUser.pm b/core/lib/Foswiki/Users/HtPasswdUser.pm index fcec34b..cf35a78 100644 --- a/core/lib/Foswiki/Users/HtPasswdUser.pm +++ b/core/lib/Foswiki/Users/HtPasswdUser.pm @@ -19,6 +19,9 @@ our @ISA = ('Foswiki::Users::Password'); use Assert; use Error qw( :try ); +use Fcntl qw( :DEFAULT :flock); +use Errno qw(EINVAL ENOLCK); +use Time::HiRes qw(sleep); # 'Use locale' for internationalisation of Perl sorting in getTopicNames # and other routines - main locale settings are done in Foswiki::setupLocale @@ -49,6 +52,10 @@ sub new { print STDERR "ERROR: crypt-md5 FAILS on OSX (no fix in 2008)\n"; throw Error::Simple("ERROR: crypt-md5 FAILS on OSX (no fix in 2008)"); } + $this->{_flock} = { + 'lock' => undef, + 'fh' => undef, + }; return $this; } @@ -64,8 +71,9 @@ Break circular references. # documentation" of the live fields in the object. sub finish { my $this = shift; - $this->SUPER::finish(); undef $this->{passworddata}; + undef $this->{_flock}; + $this->SUPER::finish(); } =begin TML @@ -94,12 +102,59 @@ sub canFetchUsers { sub fetchUsers { my $this = shift; - my $db = _readPasswd($this); + my $db = {}; + try { + $this->_lockPasswd(LOCK_SH); + $db = _readPasswd($this); + } + catch Error::Simple with { + $this->{error} = shift->{-text}; + print STDERR "ERROR: failed to fetchPass - ($this->{error})\n"; + } + finally { + $this->_unlockPasswd(); + }; my @users = sort keys %$db; require Foswiki::ListIterator; return new Foswiki::ListIterator( \@users ); } +sub _lockPasswd { + my ( $this, $op ) = @_; + return if defined $this->{_flock}{'lock'} && $this->{_flock}{'lock'} == $op; + unless ( defined $this->{_flock}{fh} ) { + open( $this->{_flock}{fh}, '+<', "$Foswiki::cfg{Htpasswd}{Filename}" ) + || throw Error::Simple( + $Foswiki::cfg{Htpasswd}{FileName} . ' open failed: ' . $! ); + } + my $timer = 0.1; + while ( $timer <= 3.2 ) { + my $rv = flock( $this->{_flock}{fh}, $op | LOCK_NB ); + if ($rv) { + $this->{_flock}{'lock'} = $op; + last; + } + else { + throw Error::Simple("Error acquiring lock: $!") if $! == ENOLCK; + last if $! == EINVAL; # flock operation not supported. + # Fallback to "no lock" + sleep($timer); + $timer *= 2; + } + } +} + +sub _unlockPasswd { + my $this = shift; + if ( defined $this->{_flock}{fh} ) { + close( $this->{_flock}{fh} ); + $this->{_flock} = { + 'lock' => undef, + fh => undef, + }; + } +} + sub _readPasswd { my $this = shift; return $this->{passworddata} if ( defined( $this->{passworddata} ) ); @@ -239,6 +294,7 @@ sub fetchPass { if ($login) { try { + $this->_lockPasswd(LOCK_SH); my $db = $this->_readPasswd(); if ( exists $db->{$login} ) { $ret = $db->{$login}->{pass}; @@ -249,7 +305,11 @@ sub fetchPass { } } catch Error::Simple with { - $this->{error} = $!; + $this->{error} = shift->{-text}; + print STDERR "ERROR: failed to fetchPass - ($this->{error})\n"; + } + finally { + $this->_unlockPasswd(); }; } else { @@ -272,6 +332,7 @@ sub setPassword { } try { + $this->_lockPasswd(LOCK_EX); my $db = $this->_readPasswd(); $db->{$login}->{pass} = $this->encrypt( $login, $newUserPassword, 1 ); $db->{$login}->{emails} ||= ''; @@ -284,6 +345,9 @@ sub setPassword { $this->{error} = 'unknown error in resetPassword' unless ( $this->{error} && length( $this->{error} ) ); return; + } + finally { + $this->_unlockPasswd(); }; $this->{error} = undef; @@ -296,6 +360,7 @@ sub removeUser { $this->{error} = undef; try { + $this->_lockPasswd(LOCK_EX); my $db = $this->_readPasswd(); unless ( $db->{$login} ) { $this->{error} = 'No such user ' . $login; @@ -308,6 +373,10 @@ sub removeUser { } catch Error::Simple with { $this->{error} = shift->{-text}; + print STDERR "ERROR: failed to removeUser - ($this->{error})\n"; + } + finally { + $this->_unlockPasswd(); }; return $result; } @@ -342,7 +411,18 @@ sub getEmails { my ( $this, $login ) = @_; # first try the mapping cache - my $db = $this->_readPasswd(); + my $db; + try { + $this->_lockPasswd(LOCK_SH); + $db = $this->_readPasswd(); + } + except { + $this->{error} = shift->{-text}; + print STDERR "ERROR: failed to getEmails - ($this->{error})\n"; + } + finally { + $this->_unlockPasswd(); + }; if ( $db->{$login}->{emails} ) { return split( /;/, $db->{$login}->{emails} ); } @@ -355,20 +435,30 @@ sub setEmails { my $login = shift; ASSERT($login) if DEBUG; - my $db = $this->_readPasswd(); - unless ( $db->{$login} ) { - $db->{$login}->{pass} = ''; - } + try { + $this->_lockPasswd(LOCK_EX); + my $db = $this->_readPasswd(); + unless ( $db->{$login} ) { + $db->{$login}->{pass} = ''; + } #SMELL: this makes no sense. - the if above suggests that we can get to this point without $db->{$login} # what use is going on if the user is not in the auth system? - if ( defined( $_[0] ) ) { - $db->{$login}->{emails} = join( ';', @_ ); + if ( defined( $_[0] ) ) { + $db->{$login}->{emails} = join( ';', @_ ); + } + else { + $db->{$login}->{emails} = ''; + } + _savePasswd($db); } - else { - $db->{$login}->{emails} = ''; + except { + $this->{error} = shift->{-text}; + print STDERR "ERROR: failed to setEmails - ($this->{error})\n"; } - _savePasswd($db); + finally { + $this->_unlockPasswd(); + }; return 1; } @@ -376,7 +466,18 @@ sub setEmails { sub findUserByEmail { my ( $this, $email ) = @_; my $logins = []; - my $db = _readPasswd(); + my $db; + try { + $this->_lockPasswd(LOCK_SH); + $db = $this->_readPasswd(); + } + except { + $this->{error} = shift->{-text}; + print STDERR "ERROR: failed to getEmails - ($this->{error})\n"; + } + finally { + $this->_unlockPasswd(); + }; while ( my ( $k, $v ) = each %$db ) { my %ems = map { $_ => 1 } split( ';', $v->{emails} ); if ( $ems{$email} ) {