[2884] in BarnOwl Developers
Re: [PATCH] Append sender's realm to Cc'd unqualified names when
daemon@ATHENA.MIT.EDU (David A Benjamin)
Sat Feb 5 22:11:26 2011
Date: Sat, 5 Feb 2011 22:11:23 -0500 (EST)
From: David A Benjamin <davidben@MIT.EDU>
To: Adam Glasgall <adam@crossproduct.net>
cc: barnowl-dev@mit.edu
In-Reply-To: <1296865593-24448-1-git-send-email-adam@crossproduct.net>
Comments inline.
Tangentally, it'd be really awesome if we could get unit tests for this
kind of stuff. I don't get the impression cross-realm zephyr is tested by
the developers very much. (I certainly don't test it.) But I imagine that
would be hairy until we get perlmessages merged. We should add some when
that happens...
David
On Fri, 4 Feb 2011, Adam Glasgall wrote:
> +sub principal_realm {
> + my $principal = shift;
> + my ($user, $realm) = split(/@/,$principal);
> + return $realm;
> +}
> +
> sub login_type {
> return (shift->zsig eq "") ? "(PSEUDO)" : "";
> }
> @@ -183,13 +189,29 @@ sub replycmd {
> }
>
> push @cmd, context_reply_cmd($class, $instance);
> +
> if ($to ne '') {
> + my $sender_realm = principal_realm($to);
Nit: May as well move that inside the 'if (defined $cc...' conditional.
It's not used anywhere else.
Would it be better to use $self->sender instead of $to? I believe it's
possible to hit the CC codepath if you are replying to a message sent to
@EXAMPLE.COM that happens to have a CC line. Though I doubt we did the
right thing there anyway.
Actually, I should probably amend the patch I just sent out to only define
$cc when $to = $self->sender.
[ Is there really no better way to find out what realm a zephyr comes
from? I would suggest $self->realm, but it looks like that is something
else. It is computed from the recipient. I guess that's relevant if you
can sub to zephyrs sent to another realm? If that even makes sense... I'm
not completely clear on how cross-realm zephyr works. ]
> $to = strip_realm($to);
> if (defined $cc and not $sender) {
> my @cc = grep /^[^-]/, ($to, split /\s+/, $cc);
> my %cc = map {$_ => 1} @cc;
> + # this isn't quite right - it doesn't strip off the
> + # user if the message was addressed to them by fully qualified
> + # name
> delete $cc{strip_realm(BarnOwl::zephyr_getsender())};
> @cc = keys %cc;
> + my $realm = BarnOwl::zephyr_getrealm();
> +
> + if($sender !~ /\@\Q$realm\E$/) {
So, $sender is actually a boolean for we wanted reply or reply to sender.
(Yeah, it's really badly named.) You probably want $self->sender. Or maybe
just
if (BarnOwl::zephyr_getrealm() ne $sender_realm) {
> + @cc = map {
> + if($_ !~ /@/) {
> + "${_}\@${sender_realm}";
> + } else {
> + $_;
> + }
> + } @cc;
> + }
> push @cmd, '-C', @cc;
> } else {
> if(BarnOwl::getvar('smartstrip') eq 'on') {
> --
> 1.7.1
>
>
