Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 127 additions & 3 deletions extensions/Needinfo/Extension.pm
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,18 @@ use warnings;

use base qw(Bugzilla::Extension);

use Bugzilla::Constants;
use Bugzilla::Error;
use Bugzilla::Flag;
use Bugzilla::FlagType;
use Bugzilla::Logging;
use Bugzilla::User;
use Bugzilla::User::Setting;

our $VERSION = '0.01';

use constant MAX_MENTIONS => 10;

BEGIN {
*Bugzilla::User::needinfo_blocked = \&_user_needinfo_blocked;
}
Expand Down Expand Up @@ -60,7 +64,7 @@ sub install_update_db {
is_active => 1,
is_requestable => 1,
is_requesteeble => 1,
is_multiplicable => 0,
is_multiplicable => 1,
request_group => '',
grant_group => '',
inclusions => ['0:0'],
Expand Down Expand Up @@ -97,10 +101,19 @@ sub bug_end_of_create {
$self->bug_start_of_update($args);
}

# Clear the needinfo? flag if comment is being given by
# requestee or someone used the override flag.
sub bug_start_of_update {
my ($self, $args) = @_;
_process_needinfo_params($args);

# Runs after the explicit needinfo handling so that the duplicate check
# sees any flags that were just requested through the form.
_process_mentions($args->{bug}) if $args->{old_bug};
}

# Clear the needinfo? flag if comment is being given by
# requestee or someone used the override flag.
sub _process_needinfo_params {
my ($args) = @_;
my $bug = $args->{bug};
my $old_bug = $args->{old_bug};

Expand Down Expand Up @@ -248,6 +261,117 @@ sub bug_start_of_update {
}
}

# Returns the unique nicknames @mentioned in a comment, in order of first
# appearance. Mentions inside quoted lines (> ...) and code are ignored, as
# are email addresses (the @ must not follow a word character).
sub _extract_mentions {
my ($text) = @_;
return () unless defined $text;

$text =~ s/^[ \t]*(`{3,}|~{3,}).*?(?:^[ \t]*\1[^\n]*$|\z)//msg;
# A code span closes on a backtick run of the same length, within a paragraph.
$text = join "\n\n",
map { s/(?<!`)(`+)(?!`).+?(?<!`)\1(?!`)//sgr } split /\n[ \t]*\n/, $text;
$text =~ s/^[ \t]*>.*$//mg;

my (@nicks, %seen);
# Same character set as extract_nicks in Bugzilla/Util.pm.
while ($text =~ /(?<![\w@.\/-])@([\p{IsAlnum}|._-]+)/g) {
(my $nick = $1) =~ s/[.|]+$//;
next if $nick eq '' || $seen{lc $nick}++;
push @nicks, $nick;
}
return @nicks;
}

# Maps nicknames to enabled user accounts. A nickname shared by more than one
# account is ambiguous and dropped rather than guessed at.
sub _users_for_mentions {
my (@nicks) = @_;
return () unless @nicks;

my $dbh = Bugzilla->dbh;
my $rows = $dbh->selectall_arrayref(
'SELECT userid, nickname FROM profiles WHERE is_enabled = 1 AND '
. $dbh->sql_in('nickname', [map { $dbh->quote($_) } @nicks]));

my %ids_by_nick;
push @{$ids_by_nick{lc $_->[1]}}, $_->[0] foreach @$rows;
my @ids = map { $_->[0] } grep { @$_ == 1 } values %ids_by_nick;
return @{Bugzilla::User->new_from_list(\@ids)};
}

# GitHub-style mentions: each @nickname in a new comment is CC'd and gets a
# needinfo request. Mentions never block the comment from being saved; anyone
# who can't be needinfo'd (blocked, already asked, no permission, or the
# needinfo type isn't multiplicable and a flag exists) is only CC'd.
#
# Safeguards against misuse:
# - only editbugs users can trigger mentions; for everyone else they are text
# - only the first MAX_MENTIONS distinct nicknames per update are processed
# - users who can't already see the bug are skipped, so a mention (or a typo,
# or a squatted nickname) can never grant access to a restricted bug
sub _process_mentions {
my ($bug) = @_;
# Lowercased nick => true if mentioned in at least one public comment.
my %public;

# The first MAX_MENTIONS distinct lowercased nicks, in order of appearance.
my @nicks;

foreach my $comment (@{$bug->{added_comments} || []}) {
foreach my $nick (map {lc} _extract_mentions($comment->{thetext})) {
push @nicks, $nick if !exists $public{$nick} && @nicks < MAX_MENTIONS;
$public{$nick} ||= !$comment->{isprivate};
}
}
return unless @nicks;

# Checked after parsing so updates without mentions skip the group lookup.
my $user = Bugzilla->user;
return unless $user->in_group('editbugs', $bug->product_id);

my @mentioned = grep {
$_->id != $user->id
&& ($public{lc $_->nick} || $_->is_insider)
&& $_->can_see_bug($bug->id)
} _users_for_mentions(@nicks);
return unless @mentioned;

my ($type) = grep { $_->name eq 'needinfo' } @{$bug->flag_types};
undef $type
unless $type && $bug->check_can_change_field('flagtypes.name', 0, 1)->{allowed};

# A failure for one user (e.g. add_cc's strict_isolation check) skips that
# user rather than rolling back the whole update. Throw*Error only dies
# (instead of printing an error page and exiting) in ERROR_MODE_DIE, so the
# eval can catch it.
local Bugzilla->request_cache->{error_mode} = ERROR_MODE_DIE;
local $@;
foreach my $mentioned (@mentioned) {
eval { _cc_and_needinfo($bug, $mentioned, $type); 1 }
or WARN('Skipped mention of ' . $mentioned->login . " on bug "
. $bug->id . ": $@");
}
}

sub _cc_and_needinfo {
my ($bug, $mentioned, $type) = @_;
$bug->add_cc($mentioned);

return if !$type || $mentioned->needinfo_blocked;

# A non-multiplicable type allows only one needinfo flag per bug.
return if !$type->is_multiplicable && @{$type->{flags}};
return if grep {
$_->status eq '?' && ($_->requestee_id // 0) == $mentioned->id
} @{$type->{flags}};

# One call per user so $type->{flags} reflects each flag as it is added.
$bug->set_flags([],
[{type_id => $type->id, status => '?', requestee => $mentioned->login}]);
}

sub _check_requestee {
my ($requestee) = @_;
my $user
Expand Down
171 changes: 171 additions & 0 deletions extensions/Needinfo/t/mentions.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
#!/usr/bin/env perl
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
#
# This Source Code Form is "Incompatible With Secondary Licenses", as
# defined by the Mozilla Public License, v. 2.0.
use strict;
use warnings;
use lib qw( . lib local/lib/perl5 );

BEGIN {
$ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf';
$ENV{BUGZILLA_DISABLE_HOSTAGE} = 1;
}

use Bugzilla::Test::MockDB;
use Bugzilla::Test::MockParams;

use Test2::V0;

use Bugzilla;
BEGIN { Bugzilla->extensions }

my $extract = \&Bugzilla::Extension::Needinfo::_extract_mentions;
Comment thread
dklawren marked this conversation as resolved.

is([$extract->(undef)], [], 'undef text');
is([$extract->('no mentions here')], [], 'no mentions');
is([$extract->('@alice can you look?')], ['alice'], 'leading mention');
is([$extract->('thanks @alice, @bob and @carol.')],
['alice', 'bob', 'carol'], 'multiple mentions with punctuation');
is([$extract->('@alice and @Alice again @alice')], ['alice'],
'duplicates collapsed case-insensitively');
is([$extract->('(@dk.l_x-y)')], ['dk.l_x-y'], 'nick symbols, parens');
is([$extract->('mail [email protected] or a/@b or x.@y')], [],
'emails and embedded @ ignored');
is([$extract->("> \@quoted said\n\@alice")], ['alice'], 'quoted lines ignored');
is([$extract->('run `@decorator` @alice')], ['alice'], 'inline code ignored');
is([$extract->('``@alice`` and `` a`@b ``')], [], 'double backtick code ignored');
is([$extract->("it's a ` stray\n\n\@alice `x`")],
['alice'], 'unclosed backtick does not span paragraphs');
is([$extract->('@foo|bar and |@alice|')], ['foo|bar', 'alice'],
'pipe allowed in nick, trailing pipe stripped');
is(
[$extract->("```perl\n\@inside\n```\n\@after")],
['after'], 'fenced code block ignored'
);
is([$extract->("~~~\n\@unclosed\n\@more")], [], 'unclosed fence runs to end');

# _process_mentions safeguards, using minimal stand-ins for users and bugs.
{

package FakeUser;
sub new { my ($class, %args) = @_; bless {%args}, $class }
sub id { $_[0]{id} }
sub nick { $_[0]{nick} }
sub login { "$_[0]{nick}\@example.com" }
sub in_group { $_[0]{editbugs} }
sub is_insider { $_[0]{insider} }
sub can_see_bug { $_[0]{sees_bug} // 1 }
sub needinfo_blocked { 0 }

package FakeBug;
sub new {
my ($class, $text, %type) = @_;
bless {
added_comments => [{thetext => $text}],
cc => [],
needinfo => [],
type => bless({flags => [], multiplicable => 1, %type}, 'FakeType'),
}, $class;
}
sub id {1}
sub product_id {1}
sub flag_types { [$_[0]{type}] }
sub check_can_change_field { {allowed => 1} }
sub add_cc {
my ($self, $user) = @_;
die "add_cc failed\n" if $user->{cc_fails};
push @{$self->{cc}}, $user->nick;
}

# Like Bugzilla::Flag->set_flag, new flags are added to the type's list.
sub set_flags {
my ($self, undef, $new_flags) = @_;
foreach my $flag (@$new_flags) {
push @{$self->{needinfo}}, $flag->{requestee};
push @{$self->{type}{flags}},
bless({status => '?', requestee_id => 0}, 'FakeFlag');
}
}

package FakeType;
sub name {'needinfo'}
sub id {1}
sub is_multiplicable { $_[0]{multiplicable} }

package FakeFlag;
sub status { $_[0]{status} }
sub requestee_id { $_[0]{requestee_id} }
}

my %users = map { $_->nick => $_ } (
FakeUser->new(id => 2, nick => 'alice'),
FakeUser->new(id => 3, nick => 'bob'),
FakeUser->new(id => 4, nick => 'hidden', sees_bug => 0),
FakeUser->new(id => 6, nick => 'boom', cc_fails => 1),
map { FakeUser->new(id => 10 + $_, nick => "u$_") } 1 .. 12,
);
my $commenter = FakeUser->new(id => 1, nick => 'me', editbugs => 1);
my @looked_up;

my $mock_bugzilla = mock 'Bugzilla' => (override => [user => sub {$commenter}]);
my $mock_ext = mock 'Bugzilla::Extension::Needinfo' => (
override => [
_users_for_mentions => sub {
@looked_up = @_;
return grep {defined} map { $users{$_} } @_;
},
],
);

my $process = \&Bugzilla::Extension::Needinfo::_process_mentions;

my $bug = FakeBug->new('@alice and @bob please look, cc @me');
$process->($bug);
is($bug->{cc}, ['alice', 'bob'], 'mentioned users CCd, self skipped');
is($bug->{needinfo}, ['[email protected]', '[email protected]'],
'mentioned users needinfod');

$bug = FakeBug->new('@hidden @alice');
$process->($bug);
is($bug->{cc}, ['alice'], 'user who cannot see the bug is not CCd');
is($bug->{needinfo}, ['[email protected]'], '... nor needinfod');

$bug = FakeBug->new(join ' ', map {"\@u$_"} 1 .. 12);
$process->($bug);
is(\@looked_up, [map {"u$_"} 1 .. 10], 'only the first 10 nicknames are looked up');
is($bug->{cc}, [map {"u$_"} 1 .. 10], '... and only they are CCd');

$bug = FakeBug->new('@alice @bob', multiplicable => 0);
$process->($bug);
is($bug->{cc}, ['alice', 'bob'], 'non-multiplicable type: everyone CCd');
is($bug->{needinfo}, ['[email protected]'], '... but only one needinfo');

$bug = FakeBug->new('@alice', multiplicable => 0);
push @{$bug->{type}{flags}}, bless({status => '?', requestee_id => 9}, 'FakeFlag');
$process->($bug);
is($bug->{cc}, ['alice'], 'non-multiplicable with existing flag: CCd');
is($bug->{needinfo}, [], '... and no needinfo, instead of an error');

$bug = FakeBug->new('@alice');
push @{$bug->{type}{flags}}, bless({status => '?', requestee_id => 2}, 'FakeFlag');
$process->($bug);
is($bug->{needinfo}, [], 'no duplicate needinfo for a pending request');

$bug = FakeBug->new('@boom @alice');
ok(lives { $process->($bug) },
'a failing CC (e.g. strict_isolation) does not throw');
is($bug->{cc}, ['alice'], '... that user is skipped');
is($bug->{needinfo}, ['[email protected]'], '... others still processed');

$commenter->{editbugs} = 0;
@looked_up = ();
$bug = FakeBug->new('@alice');
$process->($bug);
is(\@looked_up, [], 'no lookup for commenters without editbugs');
is($bug->{cc}, [], 'no CC for commenters without editbugs');
is($bug->{needinfo}, [], 'no needinfo for commenters without editbugs');

done_testing;
Loading