From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF shortcircuit=no autolearn=ham autolearn_force=no version=3.4.6 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id CC4E81F55F; Fri, 22 Sep 2023 02:18:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1695349097; bh=7IEX4TMdWvqf5qfrMzfTpeY3Y0aofZSCL+YqeSJKrnQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=JgrWTAxxIjibCQvJM/zXpWwh90yQsSmG2hj79tG3Xyf7rW1dxCwwViGyfDs9Sp4CX v/ETeQYdHGr1uBZ8TEBboGd2sZjZT+8VPl4L2mZY9njJegLufSgQK4AyZytv+FSPXi shyxd7ivnauDE8RfuvTiRGoUzGzDyQ77ki7nQOIY= Date: Fri, 22 Sep 2023 02:18:17 +0000 From: Eric Wong To: Konstantin Ryabitsev Cc: meta@public-inbox.org Subject: [PATCH] pop3: support initial_limit parameter in mailbox name Message-ID: <20230922021817.M698225@dcvr> References: <20230912-impart-swinger-4c2434@meerkat> <20230912224034.M689061@dcvr> <20230913-tarot-monogamy-2f614c@meerkat> <20230914003828.M101484@dcvr> <20230915-reputable-maverick-13736d@meerkat> <20230915204110.M732304@dcvr> <20230918-barrel-unhearing-b63869@meerkat> <20230918211422.M309741@dcvr> <20230919-buggy-unsent-411b34@meerkat> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20230919-buggy-unsent-411b34@meerkat> List-Id: Konstantin Ryabitsev 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 +# License: AGPL-3.0+ +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(< +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 <{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( ) ], + "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() ], + "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( + ) ], + "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() ], + "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() ], + "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( ) ], + "excessive limit ($login)") or diag explain(\@msg); +} + +done_testing;