* Re: [PATCH] pop3: support initial_limit parameter in mailbox name
2023-09-22 18:02 10% ` Konstantin Ryabitsev
@ 2023-09-22 18:38 10% ` Eric Wong
0 siblings, 0 replies; 3+ results
From: Eric Wong @ 2023-09-22 18:38 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: meta
Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> On Fri, Sep 22, 2023 at 02:18:17AM +0000, Eric Wong wrote:
> > Subject: [PATCH] pop3: support initial_limit parameter in mailbox name
>
> That looks good in my tests. Thanks!
>
> Tested-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
Thanks. I also forgot to expand on the fix for limit for slice:
This also fixes an offset calculation bug with limit when used
on the newest (non-full) slice. The number of messages in the
newest slice is now taken into account.
(oops, pushed out without your tested-by :x)
^ permalink raw reply [relevance 10%]
* Re: [PATCH] pop3: support initial_limit parameter in mailbox name
2023-09-22 2:18 14% ` [PATCH] pop3: support initial_limit " Eric Wong
@ 2023-09-22 18:02 10% ` Konstantin Ryabitsev
2023-09-22 18:38 10% ` Eric Wong
0 siblings, 1 reply; 3+ results
From: Konstantin Ryabitsev @ 2023-09-22 18:02 UTC (permalink / raw)
To: Eric Wong; +Cc: meta
On Fri, Sep 22, 2023 at 02:18:17AM +0000, Eric Wong wrote:
> Subject: [PATCH] pop3: support initial_limit parameter in mailbox name
That looks good in my tests. Thanks!
Tested-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
-K
^ permalink raw reply [relevance 10%]
* [PATCH] pop3: support initial_limit parameter in mailbox name
@ 2023-09-22 2:18 14% ` Eric Wong
2023-09-22 18:02 10% ` Konstantin Ryabitsev
0 siblings, 1 reply; 3+ results
From: Eric Wong @ 2023-09-22 2:18 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: meta
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;
^ permalink raw reply related [relevance 14%]
Results 1-3 of 3 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2023-09-12 21:08 pop3 usability thoughts Konstantin Ryabitsev
2023-09-12 22:40 ` [RFC] pop3: support `?limit=$NUM' parameter in mailbox name Eric Wong
2023-09-13 16:08 ` Konstantin Ryabitsev
2023-09-14 0:38 ` Eric Wong
2023-09-15 20:03 ` Konstantin Ryabitsev
2023-09-15 20:41 ` Eric Wong
2023-09-18 13:46 ` Konstantin Ryabitsev
2023-09-18 21:14 ` Eric Wong
2023-09-19 21:28 ` Konstantin Ryabitsev
2023-09-22 2:18 14% ` [PATCH] pop3: support initial_limit " Eric Wong
2023-09-22 18:02 10% ` Konstantin Ryabitsev
2023-09-22 18:38 10% ` Eric Wong
Code repositories for project(s) associated with this public inbox
https://80x24.org/public-inbox.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).