[PATCH] pop3: support initial_limit parameter in mailbox name

[Eric Wong <e@80x24.org>]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> I'm game with that. Maybe even shorten that to l= and il=? I'm still worried
> about the field size limit a bit.

I suspect they're not needed, but we can always add them later
if they are[1].  Longer parameters are better for documentation
purposes since they're user-facing.

[1] it's safe to stuff arbitrarily long `&foo=bar' pairs in the
    query parameters if someone is able to test a bunch of clients.
    `apt-get source mpop' reveals it handles lines up to 500 bytes
    (`#define LINEBUFSIZE 501' in src/conf.c).

-------8<-------
Subject: [PATCH] pop3: support initial_limit parameter in mailbox name

`initial_limit' only affects the fetch for new users while
`limit' affects all users.  `initial_limit' is intended to be
better than the existing, absolute `limit' for 24/7 servers
(e.g. scheduled POP3 imports via webmail).  The existing `limit'
parameter remains and is best suited for systems with sporadic
Internet access.

Link: https://public-inbox.org/meta/20230918-barrel-unhearing-b63869@meerkat/
Fixes: 392d251f97d4 (pop3: support `?limit=$NUM' parameter in mailbox name)
---
 MANIFEST                 |   1 +
 lib/PublicInbox/POP3.pm  |  46 +++++++-----
 lib/PublicInbox/POP3D.pm |   5 +-
 t/pop3d-limit.t          | 146 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 180 insertions(+), 18 deletions(-)
 create mode 100644 t/pop3d-limit.t

diff --git a/MANIFEST b/MANIFEST
index 7dba3836..4693cbe0 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -558,6 +558,7 @@ t/plack-2-txt-bodies.eml
 t/plack-attached-patch.eml
 t/plack-qp.eml
 t/plack.t
+t/pop3d-limit.t
 t/pop3d.t
 t/precheck.t
 t/psgi_attach.eml
diff --git a/lib/PublicInbox/POP3.pm b/lib/PublicInbox/POP3.pm
index f97eccfd..6d24b17c 100644
--- a/lib/PublicInbox/POP3.pm
+++ b/lib/PublicInbox/POP3.pm
@@ -62,7 +62,7 @@ sub new {
 	(bless { pop3d => $pop3d }, $cls)->greet($sock)
 }
 
-# POP user is $UUID1@$NEWSGROUP.$SLICE
+# POP user is $UUID1@$NEWSGROUP[.$SLICE][?QUERY_ARGS]
 sub cmd_user ($$) {
 	my ($self, $mailbox) = @_;
 	$self->{salt} // return \"-ERR already authed\r\n";
@@ -72,26 +72,25 @@ sub cmd_user ($$) {
 	$user =~ tr/-//d; # most have dashes, some (dbus-uuidgen) don't
 	$user =~ m!\A[a-f0-9]{32}\z!i or return \"-ERR user has no UUID\r\n";
 
-	my $limit;
-	$mailbox =~ s/\?limit=([0-9]+)\z// and
-		$limit = $1 > UID_SLICE ? UID_SLICE : $1;
-
+	my %l;
+	if ($mailbox =~ s/\?(.*)\z//) { # query args
+		for (split(/&+/, $1)) {
+			/\A(initial_limit|limit)=([0-9]+)\z/ and $l{$1} = $2;
+		}
+		$self->{limits} = \%l;
+	}
 	my $slice = $mailbox =~ s/\.([0-9]+)\z// ? $1 + 0 : undef;
 
 	my $ibx = $self->{pop3d}->{pi_cfg}->lookup_newsgroup($mailbox) //
 		return \"-ERR $mailbox does not exist\r\n";
-	my $uidmax = $ibx->mm(1)->num_highwater // 0;
+	my $uidmax = $self->{uidmax} = $ibx->mm(1)->num_highwater // 0;
 	if (defined $slice) {
 		my $max = int($uidmax / UID_SLICE);
 		my $tip = "$mailbox.$max";
 		return \"-ERR $mailbox.$slice does not exist ($tip does)\r\n"
 			if $slice > $max;
-		$limit //= UID_SLICE;
-		$self->{uid_base} = ($slice * UID_SLICE) + UID_SLICE - $limit;
 		$self->{slice} = $slice;
-	} else { # latest $limit messages, 1k if unspecified
-		my $base = $uidmax - ($limit // 1000);
-		$self->{uid_base} = $base < 0 ? 0 : $base;
+	} else { # latest messages:
 		$self->{slice} = -1;
 	}
 	$self->{ibx} = $ibx;
@@ -102,12 +101,27 @@ sub cmd_user ($$) {
 
 sub _login_ok ($) {
 	my ($self) = @_;
-	if ($self->{pop3d}->lock_mailbox($self)) {
-		$self->{uid_max} = $self->{ibx}->over(1)->max;
-		\"+OK logged in\r\n";
-	} else {
-		\"-ERR [IN-USE] unable to lock maildrop\r\n";
+	$self->{pop3d}->lock_mailbox($self) or
+		return \"-ERR [IN-USE] unable to lock maildrop\r\n";
+
+	my $l = delete $self->{limits};
+	$l = defined($self->{uid_dele}) ? $l->{limit}
+					: ($l->{initial_limit} // $l->{limit});
+	my $uidmax = delete $self->{uidmax};
+	if ($self->{slice} >= 0) {
+		$self->{uid_base} = $self->{slice} * UID_SLICE;
+		if (defined $l) { # n.b: the last slice is not full:
+			my $max = int($uidmax/UID_SLICE) == $self->{slice} ?
+					($uidmax % UID_SLICE) : UID_SLICE;
+			my $off = $max - $l;
+			$self->{uid_base} += $off if $off > 0;
+		}
+	} else { # latest $l messages, or 1k if unspecified
+		my $base = $uidmax - ($l // 1000);
+		$self->{uid_base} = $base < 0 ? 0 : $base;
 	}
+	$self->{uid_max} = $self->{ibx}->over(1)->max;
+	\"+OK logged in\r\n";
 }
 
 sub cmd_apop {
diff --git a/lib/PublicInbox/POP3D.pm b/lib/PublicInbox/POP3D.pm
index bee668fc..2a9ccfdd 100644
--- a/lib/PublicInbox/POP3D.pm
+++ b/lib/PublicInbox/POP3D.pm
@@ -157,6 +157,7 @@ sub lock_mailbox {
 	my ($user_id, $ngid, $mbid, $txn_id);
 	my $uuid = delete $pop3->{uuid};
 	$dbh->begin_work;
+	my $creat = 0;
 
 	# 1. make sure the user exists, update `last_seen'
 	my $sth = $dbh->prepare_cached(<<'');
@@ -219,13 +220,13 @@ SELECT mailbox_id FROM mailboxes WHERE newsgroup_id = ? AND slice = ?
 	$sth = $dbh->prepare_cached(<<'');
 INSERT OR IGNORE INTO deletes (user_id,mailbox_id) VALUES (?,?)
 
-	if ($sth->execute($user_id, $mbid) == 0) {
+	if ($sth->execute($user_id, $mbid) == 0) { # fetching into existing
 		$sth = $dbh->prepare_cached(<<'', undef, 1);
 SELECT txn_id,uid_dele FROM deletes WHERE user_id = ? AND mailbox_id = ?
 
 		$sth->execute($user_id, $mbid);
 		($txn_id, $pop3->{uid_dele}) = $sth->fetchrow_array;
-	} else {
+	} else { # new user/mailbox combo
 		$txn_id = $dbh->last_insert_id(undef, undef,
 						'deletes', 'txn_id');
 	}
diff --git a/t/pop3d-limit.t b/t/pop3d-limit.t
new file mode 100644
index 00000000..39d2f918
--- /dev/null
+++ b/t/pop3d-limit.t
@@ -0,0 +1,146 @@
+#!perl -w
+# Copyright (C) all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use v5.12;
+use PublicInbox::TestCommon;
+require_mods(qw(DBD::SQLite Net::POP3));
+$^O =~ /\A(?:linux|(?:free|net|open)bsd)\z/ or
+	require_mods(qw(File::FcntlLock));
+use autodie;
+my ($tmpdir, $for_destroy) = tmpdir();
+mkdir("$tmpdir/p3state");
+use PublicInbox::Eml;
+my $group = 'test.pop3d.limit';
+my $addr = 'pop3d-limit@example.com';
+
+my $add_msg = sub {
+	my ($im, $n) = @_;
+	$im->add(PublicInbox::Eml->new(<<EOM)) or die 'add dup';
+From: $n\@example.com
+Subject: msg $n
+To: $addr
+Message-ID: <mid-$n\@example.com>
+Date: Sat, 02 Oct 2010 00:00:00 +0000
+
+body $n
+EOM
+};
+
+my $ibx = create_inbox 'pop3d-limit', version => 2, -primary_address => $addr,
+			indexlevel => 'basic', tmpdir => "$tmpdir/ibx", sub {
+	my ($im, $ibx) = @_;
+	$add_msg->($im, $_) for (1..3);
+	$im->done;
+	diag 'done';
+}; # /create_inbox
+
+my $pi_config = "$tmpdir/pi_config";
+{
+	open my $fh, '>', $pi_config;
+	print $fh <<EOF;
+[publicinbox]
+	pop3state = $tmpdir/p3state
+[publicinbox "pop3"]
+	inboxdir = $ibx->{inboxdir}
+	address = $addr
+	indexlevel = basic
+	newsgroup = $group
+EOF
+	close $fh;
+}
+
+my $plain = tcp_server();
+my $plain_addr = tcp_host_port($plain);
+my $env = { PI_CONFIG => $pi_config };
+my $p3d = start_script([qw(-pop3d -W0),
+	"--stdout=$tmpdir/out.log", "--stderr=$tmpdir/err.log" ],
+	$env, { 3 => $plain });
+my @np3args = ($plain->sockhost, Port => $plain->sockport);
+my $fetch_delete = sub {
+	my ($np3) = @_;
+	map {
+		my $msg = $np3->get($_);
+		$np3->delete($_);
+		PublicInbox::Eml->new(join('', @$msg));
+	} sort { $a <=> $b } keys %{$np3->list};
+};
+
+my $login_a = ('a'x32)."\@$group?initial_limit=2&limit=1";
+my $login_a0 = ('a'x32)."\@$group.0?initial_limit=2&limit=1";
+my $login_b = ('b'x32)."\@$group?limit=1";
+my $login_b0 = ('b'x32)."\@$group.0?limit=1";
+my $login_c = ('c'x32)."\@$group?limit=10";
+my $login_c0 = ('c'x32)."\@$group.0?limit=10";
+my $login_d = ('d'x32)."\@$group?limit=100000";
+my $login_d0 = ('d'x32)."\@$group.0?limit=100000";
+
+for my $login ($login_a, $login_a0) {
+	my $np3 = Net::POP3->new(@np3args);
+	$np3->login($login, 'anonymous') or xbail "login $login ($!)";
+	my @msg = $fetch_delete->($np3);
+	$np3->quit;
+	is_deeply([ map { $_->header('Message-ID') } @msg ],
+		[ qw(<mid-2@example.com> <mid-3@example.com>) ],
+		"initial_limit ($login)") or diag explain(\@msg);
+}
+
+for my $login ($login_b, $login_b0) {
+	my $np3 = Net::POP3->new(@np3args);
+	$np3->login($login, 'anonymous') or xbail "login $login ($!)";
+	my @msg = $fetch_delete->($np3);
+	$np3->quit;
+	is_deeply([ map { $_->header('Message-ID') } @msg ],
+		[ qw(<mid-3@example.com>) ],
+		"limit-only ($login)") or diag explain(\@msg);
+}
+
+for my $login ($login_c, $login_c0, $login_d, $login_d0) {
+	my $np3 = Net::POP3->new(@np3args);
+	$np3->login($login, 'anonymous') or xbail "login $login ($!)";
+	my @msg = $fetch_delete->($np3);
+	$np3->quit;
+	is_deeply([ map { $_->header('Message-ID') } @msg ],
+		[ qw(<mid-1@example.com> <mid-2@example.com>
+			<mid-3@example.com>) ],
+		"excessive limit ($login)") or diag explain(\@msg);
+}
+
+{ # add some new messages
+	my $im = $ibx->importer(0);
+	$add_msg->($im, $_) for (4..5);
+	$im->done;
+}
+
+for my $login ($login_a, $login_a0) {
+	my $np3 = Net::POP3->new(@np3args);
+	$np3->login($login, 'anonymous') or xbail "login $login ($!)";
+	my @msg = $fetch_delete->($np3);
+	$np3->quit;
+	is_deeply([ map { $_->header('Message-ID') } @msg ],
+		[ qw(<mid-5@example.com>) ],
+		"limit used (initial_limit ignored, $login)") or
+			diag explain(\@msg);
+}
+
+for my $login ($login_b, $login_b0) {
+	my $np3 = Net::POP3->new(@np3args);
+	$np3->login($login, 'anonymous') or xbail "login $login ($!)";
+	my @msg = $fetch_delete->($np3);
+	$np3->quit;
+	is_deeply([ map { $_->header('Message-ID') } @msg ],
+		[ qw(<mid-5@example.com>) ],
+		"limit-only after new messages ($login)") or
+		diag explain(\@msg);
+}
+
+for my $login ($login_c, $login_c0, $login_d, $login_d0) {
+	my $np3 = Net::POP3->new(@np3args);
+	$np3->login($login, 'anonymous') or xbail "login $login ($!)";
+	my @msg = $fetch_delete->($np3);
+	$np3->quit;
+	is_deeply([ map { $_->header('Message-ID') } @msg ],
+		[ qw(<mid-4@example.com> <mid-5@example.com>) ],
+		"excessive limit ($login)") or diag explain(\@msg);
+}
+
+done_testing;