unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
* [PATCH 0/8] net_reader: simplify + fix bugs
@ 2021-09-09  5:24 Eric Wong
  2021-09-09  5:24 ` [PATCH 1/8] net_reader: do not set "SSL" fields for non-TLS Eric Wong
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Eric Wong @ 2021-09-09  5:24 UTC (permalink / raw)
  To: meta

What started off as a series of cleanups leads to some minor bug
fixes for some uncommon corner-cases.  These changes will
hopefully simplify IPC for sharing auth with lei/store and
IMAP IDLE watchers.

Eric Wong (8):
  net_reader: do not set "SSL" fields for non-TLS
  net_reader: set IMAPClient Keepalive flag late
  net_reader: preserve memoized IMAPClient arg for SOCKS
  net_reader: nntp_opt => cfg_opt
  net_reader: imap_opt => cfg_opt
  net_reader: combine Net::NNTP and IMAPClient args
  net_reader: improve naming of common args
  net_reader: support Mail::IMAPClient Ignoresizeerrors

 lib/PublicInbox/NetReader.pm | 94 ++++++++++++++++++------------------
 lib/PublicInbox/Watch.pm     | 10 ++--
 2 files changed, 53 insertions(+), 51 deletions(-)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/8] net_reader: do not set "SSL" fields for non-TLS
  2021-09-09  5:24 [PATCH 0/8] net_reader: simplify + fix bugs Eric Wong
@ 2021-09-09  5:24 ` Eric Wong
  2021-09-09  5:24 ` [PATCH 2/8] net_reader: set IMAPClient Keepalive flag late Eric Wong
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Eric Wong @ 2021-09-09  5:24 UTC (permalink / raw)
  To: meta

This will save a little bit of memory and IPC I/O for users
connecting to localhost and the majority of Tor .onions.
---
 lib/PublicInbox/NetReader.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index 08166415..b5d14f13 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -79,6 +79,7 @@ sub mic_for ($$$$) { # mic = Mail::IMAPClient
 		Keepalive => 1, # SO_KEEPALIVE
 		%$common, # may set Starttls, Compress, Debug ....
 	};
+	$mic_arg->{Ssl} = 1 if $uri->scheme eq 'imaps';
 	require PublicInbox::IMAPClient;
 	my $mic = mic_new($self, $mic_arg, $sec, $uri) or
 			die "E: <$uri> new: $@\n";
@@ -197,9 +198,9 @@ sub nn_for ($$$$) { # nn = Net::NNTP
 	my $nn_arg = {
 		Port => $uri->port,
 		Host => $host,
-		SSL => $uri->secure, # snews == nntps
 		%$common, # may Debug ....
 	};
+	$nn_arg->{SSL} = 1 if $uri->secure; # snews == nntps
 	my $sa = $self->{-proxy_cli};
 	%$nn_arg = (%$nn_arg, %$sa) if $sa;
 	my $nn = nn_new($nn_arg, $nntp_opt, $uri);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/8] net_reader: set IMAPClient Keepalive flag late
  2021-09-09  5:24 [PATCH 0/8] net_reader: simplify + fix bugs Eric Wong
  2021-09-09  5:24 ` [PATCH 1/8] net_reader: do not set "SSL" fields for non-TLS Eric Wong
@ 2021-09-09  5:24 ` Eric Wong
  2021-09-09  5:25 ` [PATCH 3/8] net_reader: preserve memoized IMAPClient arg for SOCKS Eric Wong
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Eric Wong @ 2021-09-09  5:24 UTC (permalink / raw)
  To: meta

Since we always enable SO_KEEPALIVE unconditionally, having it
in {mic_arg} leads to unnecessary IPC overhead and memory use.
---
 lib/PublicInbox/NetReader.pm | 3 +--
 lib/PublicInbox/Watch.pm     | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index b5d14f13..9faa362c 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -51,7 +51,7 @@ sub mic_new ($$$$) {
 		$socks{Socket} = IO::Socket::Socks->new(%opt) or die
 			"E: <$$uri> ".eval('$IO::Socket::Socks::SOCKS_ERROR');
 	}
-	PublicInbox::IMAPClient->new(%$mic_arg, %socks);
+	PublicInbox::IMAPClient->new(%$mic_arg, %socks, Keepalive => 1);
 }
 
 sub auth_anon_cb { '' }; # for Mail::IMAPClient::Authcallback
@@ -76,7 +76,6 @@ sub mic_for ($$$$) { # mic = Mail::IMAPClient
 		Port => $uri->port,
 		Server => $host,
 		Ssl => $uri->scheme eq 'imaps',
-		Keepalive => 1, # SO_KEEPALIVE
 		%$common, # may set Starttls, Compress, Debug ....
 	};
 	$mic_arg->{Ssl} = 1 if $uri->scheme eq 'imaps';
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index 482d35c4..7a35ee59 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -358,7 +358,7 @@ sub watch_imap_idle_1 ($$$) {
 	my $mic;
 	local $0 = $uri->mailbox." $sec";
 	until ($self->{quit}) {
-		$mic //= PublicInbox::IMAPClient->new(%$mic_arg);
+		$mic //= PublicInbox::IMAPClient->new(%$mic_arg,Keepalive => 1);
 		my $err;
 		if ($mic && $mic->IsConnected) {
 			local $self->{mics_cached}->{$sec} = $mic;

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/8] net_reader: preserve memoized IMAPClient arg for SOCKS
  2021-09-09  5:24 [PATCH 0/8] net_reader: simplify + fix bugs Eric Wong
  2021-09-09  5:24 ` [PATCH 1/8] net_reader: do not set "SSL" fields for non-TLS Eric Wong
  2021-09-09  5:24 ` [PATCH 2/8] net_reader: set IMAPClient Keepalive flag late Eric Wong
@ 2021-09-09  5:25 ` Eric Wong
  2021-09-09  5:25 ` [PATCH 4/8] net_reader: nntp_opt => cfg_opt Eric Wong
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Eric Wong @ 2021-09-09  5:25 UTC (permalink / raw)
  To: meta

Multiple invocations of mic_new may happen in long-lived
processes, so do not let mic_new make irreversible changes
to the cached args when using a SOCKS proxy.
---
 lib/PublicInbox/NetReader.pm | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index 9faa362c..5199a3a3 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -42,16 +42,16 @@ EOM
 
 sub mic_new ($$$$) {
 	my ($self, $mic_arg, $sec, $uri) = @_;
-	my %socks;
+	my %mic_arg = %$mic_arg;
 	my $sa = $self->{imap_opt}->{$sec}->{-proxy_cfg} || $self->{-proxy_cli};
 	if ($sa) {
 		my %opt = %$sa;
-		$opt{ConnectAddr} = delete $mic_arg->{Server};
-		$opt{ConnectPort} = delete $mic_arg->{Port};
-		$socks{Socket} = IO::Socket::Socks->new(%opt) or die
+		$opt{ConnectAddr} = delete $mic_arg{Server};
+		$opt{ConnectPort} = delete $mic_arg{Port};
+		$mic_arg{Socket} = IO::Socket::Socks->new(%opt) or die
 			"E: <$$uri> ".eval('$IO::Socket::Socks::SOCKS_ERROR');
 	}
-	PublicInbox::IMAPClient->new(%$mic_arg, %socks, Keepalive => 1);
+	PublicInbox::IMAPClient->new(%mic_arg, Keepalive => 1);
 }
 
 sub auth_anon_cb { '' }; # for Mail::IMAPClient::Authcallback

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/8] net_reader: nntp_opt => cfg_opt
  2021-09-09  5:24 [PATCH 0/8] net_reader: simplify + fix bugs Eric Wong
                   ` (2 preceding siblings ...)
  2021-09-09  5:25 ` [PATCH 3/8] net_reader: preserve memoized IMAPClient arg for SOCKS Eric Wong
@ 2021-09-09  5:25 ` Eric Wong
  2021-09-09  5:25 ` [PATCH 5/8] net_reader: imap_opt " Eric Wong
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Eric Wong @ 2021-09-09  5:25 UTC (permalink / raw)
  To: meta

Since this our internal NNTP options are keyed by URI section,
there's no need to have separate hashes for NNTP and IMAP
options since they URI already distinguishes them.

This will make future changes to support POP3 and JMAP and
arg caching with lei/store easier.
---
 lib/PublicInbox/NetReader.pm | 30 +++++++++++++++---------------
 lib/PublicInbox/Watch.pm     |  2 +-
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index 5199a3a3..e0a64202 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -138,7 +138,7 @@ sub try_starttls ($) {
 }
 
 sub nn_new ($$$) {
-	my ($nn_arg, $nntp_opt, $uri) = @_;
+	my ($nn_arg, $nntp_cfg, $uri) = @_;
 	my $nn;
 	if (defined $nn_arg->{ProxyAddr}) {
 		require PublicInbox::NetNNTPSocks;
@@ -151,19 +151,19 @@ sub nn_new ($$$) {
 	# default to using STARTTLS if it's available, but allow
 	# it to be disabled for localhost/VPN users
 	if (!$nn_arg->{SSL} && $nn->can('starttls')) {
-		if (!defined($nntp_opt->{starttls}) &&
+		if (!defined($nntp_cfg->{starttls}) &&
 				try_starttls($nn_arg->{Host})) {
 			# soft fail by default
 			$nn->starttls or warn <<"";
 W: <$uri> STARTTLS tried and failed (not requested)
 
-		} elsif ($nntp_opt->{starttls}) {
+		} elsif ($nntp_cfg->{starttls}) {
 			# hard fail if explicitly configured
 			$nn->starttls or die <<"";
 E: <$uri> STARTTLS requested and failed
 
 		}
-	} elsif ($nntp_opt->{starttls}) {
+	} elsif ($nntp_cfg->{starttls}) {
 		$nn->can('starttls') or
 			die "E: <$uri> Net::NNTP too old for STARTTLS\n";
 		$nn->starttls or die <<"";
@@ -176,7 +176,7 @@ E: <$uri> STARTTLS requested and failed
 sub nn_for ($$$$) { # nn = Net::NNTP
 	my ($self, $uri, $nn_args, $lei) = @_;
 	my $sec = uri_section($uri);
-	my $nntp_opt = $self->{nntp_opt}->{$sec} //= {};
+	my $nntp_cfg = $self->{cfg_opt}->{$sec} //= {};
 	my $host = $uri->host;
 	# Net::NNTP and Net::Netrc both mishandle `0', so we pass `127.0.0.1'
 	$host = '127.0.0.1' if $host eq '0';
@@ -202,27 +202,27 @@ sub nn_for ($$$$) { # nn = Net::NNTP
 	$nn_arg->{SSL} = 1 if $uri->secure; # snews == nntps
 	my $sa = $self->{-proxy_cli};
 	%$nn_arg = (%$nn_arg, %$sa) if $sa;
-	my $nn = nn_new($nn_arg, $nntp_opt, $uri);
+	my $nn = nn_new($nn_arg, $nntp_cfg, $uri);
 	if ($cred) {
 		$cred->fill($lei) unless defined($p); # may prompt user here
 		if ($nn->authinfo($u, $p)) {
-			push @{$nntp_opt->{-postconn}}, [ 'authinfo', $u, $p ];
+			push @{$nntp_cfg->{-postconn}}, [ 'authinfo', $u, $p ];
 		} else {
 			warn "E: <$uri> AUTHINFO $u XXXX failed\n";
 			$nn = undef;
 		}
 	}
 
-	if ($nntp_opt->{compress}) {
+	if ($nntp_cfg->{compress}) {
 		# https://rt.cpan.org/Ticket/Display.html?id=129967
 		if ($nn->can('compress')) {
 			if ($nn->compress) {
-				push @{$nntp_opt->{-postconn}}, [ 'compress' ];
+				push @{$nntp_cfg->{-postconn}}, [ 'compress' ];
 			} else {
 				warn "W: <$uri> COMPRESS failed\n";
 			}
 		} else {
-			delete $nntp_opt->{compress};
+			delete $nntp_cfg->{compress};
 			warn <<"";
 W: <$uri> COMPRESS not supported by Net::NNTP
 W: see https://rt.cpan.org/Ticket/Display.html?id=129967 for updates
@@ -348,13 +348,13 @@ sub nntp_common_init ($;$) {
 		# Net::NNTP post-connect commands
 		for my $k (qw(starttls compress)) {
 			$v = cfg_bool($cfg, "nntp.$k", $$uri) // next;
-			$self->{nntp_opt}->{$sec}->{$k} = $v;
+			$self->{cfg_opt}->{$sec}->{$k} = $v;
 		}
 
 		# -watch internal option
 		for my $k (qw(pollInterval)) {
 			$to = cfg_intvl($cfg, "nntp.$k", $$uri) // next;
-			$self->{nntp_opt}->{$sec}->{$k} = $to;
+			$self->{cfg_opt}->{$sec}->{$k} = $to;
 		}
 	}
 	# make sure we can connect and cache the credentials in memory
@@ -662,9 +662,9 @@ sub nn_get {
 	$nn = delete($cached->{$sec}) and return $nn;
 	my $nn_arg = $self->{nn_arg}->{$sec} or
 			die "BUG: no Net::NNTP->new arg for $sec";
-	my $nntp_opt = $self->{nntp_opt}->{$sec};
-	$nn = nn_new($nn_arg, $nntp_opt, $uri) or return;
-	if (my $postconn = $nntp_opt->{-postconn}) {
+	my $nntp_cfg = $self->{cfg_opt}->{$sec};
+	$nn = nn_new($nn_arg, $nntp_cfg, $uri) or return;
+	if (my $postconn = $nntp_cfg->{-postconn}) {
 		for my $m_arg (@$postconn) {
 			my ($method, @args) = @$m_arg;
 			$nn->$method(@args) and next;
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index 7a35ee59..96faa9f8 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -549,7 +549,7 @@ sub watch_nntp_init ($$) {
 	PublicInbox::NetReader::nntp_common_init($self);
 	for my $uri (@{$self->{nntp_order}}) {
 		my $sec = uri_section($uri);
-		my $intvl = $self->{nntp_opt}->{$sec}->{pollInterval};
+		my $intvl = $self->{cfg_opt}->{$sec}->{pollInterval};
 		push @{$poll->{$intvl || 120}}, $uri;
 	}
 }

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 5/8] net_reader: imap_opt => cfg_opt
  2021-09-09  5:24 [PATCH 0/8] net_reader: simplify + fix bugs Eric Wong
                   ` (3 preceding siblings ...)
  2021-09-09  5:25 ` [PATCH 4/8] net_reader: nntp_opt => cfg_opt Eric Wong
@ 2021-09-09  5:25 ` Eric Wong
  2021-09-09  5:25 ` [PATCH 6/8] net_reader: combine Net::NNTP and IMAPClient args Eric Wong
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Eric Wong @ 2021-09-09  5:25 UTC (permalink / raw)
  To: meta

Since this our internal IMAP options are keyed by URI section,
there's no need to have separate hashes for NNTP and IMAP
options since they URI already distinguishes them.

This will make future changes to support POP3 and JMAP and
arg caching with lei/store easier.
---
 lib/PublicInbox/NetReader.pm | 12 ++++++------
 lib/PublicInbox/Watch.pm     |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index e0a64202..f04752e5 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -43,7 +43,7 @@ EOM
 sub mic_new ($$$$) {
 	my ($self, $mic_arg, $sec, $uri) = @_;
 	my %mic_arg = %$mic_arg;
-	my $sa = $self->{imap_opt}->{$sec}->{-proxy_cfg} || $self->{-proxy_cli};
+	my $sa = $self->{cfg_opt}->{$sec}->{-proxy_cfg} || $self->{-proxy_cli};
 	if ($sa) {
 		my %opt = %$sa;
 		$opt{ConnectAddr} = delete $mic_arg{Server};
@@ -292,15 +292,15 @@ sub imap_common_init ($;$) {
 		my $to = cfg_intvl($cfg, 'imap.timeout', $$uri);
 		$mic_args->{$sec}->{Timeout} = $to if $to;
 		my $sa = socks_args($cfg->urlmatch('imap.Proxy', $$uri));
-		$self->{imap_opt}->{$sec}->{-proxy_cfg} = $sa if $sa;
+		$self->{cfg_opt}->{$sec}->{-proxy_cfg} = $sa if $sa;
 		for my $k (qw(pollInterval idleInterval)) {
 			$to = cfg_intvl($cfg, "imap.$k", $$uri) // next;
-			$self->{imap_opt}->{$sec}->{$k} = $to;
+			$self->{cfg_opt}->{$sec}->{$k} = $to;
 		}
 		my $k = 'imap.fetchBatchSize';
 		my $bs = $cfg->urlmatch($k, $$uri) // next;
 		if ($bs =~ /\A([0-9]+)\z/) {
-			$self->{imap_opt}->{$sec}->{batch_size} = $bs;
+			$self->{cfg_opt}->{$sec}->{batch_size} = $bs;
 		} else {
 			warn "$k=$bs is not an integer\n";
 		}
@@ -462,7 +462,7 @@ sub each_old_flags ($$$$) {
 	my ($self, $mic, $uri, $l_uid) = @_;
 	$l_uid ||= 1;
 	my $sec = uri_section($uri);
-	my $bs = ($self->{imap_opt}->{$sec}->{batch_size} // 1) * 10000;
+	my $bs = ($self->{cfg_opt}->{$sec}->{batch_size} // 1) * 10000;
 	my ($eml_cb, @args) = @{$self->{eml_each}};
 	$self->{quiet} or warn "# $uri syncing flags 1:$l_uid\n";
 	for (my $n = 1; $n <= $l_uid; $n += $bs) {
@@ -554,7 +554,7 @@ EOF
 		my $m = $mod ? " [(UID % $mod) == $shard]" : '';
 		warn "# $uri fetching UID $l_uid:$r_uid$m\n";
 	}
-	my $bs = $self->{imap_opt}->{$sec}->{batch_size} // 1;
+	my $bs = $self->{cfg_opt}->{$sec}->{batch_size} // 1;
 	my $req = $mic->imap4rev1 ? 'BODY.PEEK[]' : 'RFC822.PEEK';
 	my $key = $req;
 	$key =~ s/\.PEEK//;
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index 96faa9f8..20ce0616 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -530,9 +530,9 @@ sub watch_imap_init ($$) {
 	for my $uri (@{$self->{imap_order}}) {
 		my $sec = uri_section($uri);
 		my $mic = $mics->{$sec};
-		my $intvl = $self->{imap_opt}->{$sec}->{pollInterval};
+		my $intvl = $self->{cfg_opt}->{$sec}->{pollInterval};
 		if ($mic->has_capability('IDLE') && !$intvl) {
-			$intvl = $self->{imap_opt}->{$sec}->{idleInterval};
+			$intvl = $self->{cfg_opt}->{$sec}->{idleInterval};
 			push @$idle, [ $uri, $intvl // () ];
 		} else {
 			push @{$poll->{$intvl || 120}}, $uri;

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 6/8] net_reader: combine Net::NNTP and IMAPClient args
  2021-09-09  5:24 [PATCH 0/8] net_reader: simplify + fix bugs Eric Wong
                   ` (4 preceding siblings ...)
  2021-09-09  5:25 ` [PATCH 5/8] net_reader: imap_opt " Eric Wong
@ 2021-09-09  5:25 ` Eric Wong
  2021-09-09  5:25 ` [PATCH 7/8] net_reader: improve naming of common args Eric Wong
  2021-09-09  5:25 ` [PATCH 8/8] net_reader: support Mail::IMAPClient Ignoresizeerrors Eric Wong
  7 siblings, 0 replies; 9+ messages in thread
From: Eric Wong @ 2021-09-09  5:25 UTC (permalink / raw)
  To: meta

Since these are keyed by IMAP and NNTP URIs which can never
conflict, it simplifies our internals to keep them in one big
hash since we'll add POP3 and JMAP client support.
---
 lib/PublicInbox/NetReader.pm | 10 ++++------
 lib/PublicInbox/Watch.pm     |  2 +-
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index f04752e5..91e53913 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -108,7 +108,7 @@ sub mic_for ($$$$) { # mic = Mail::IMAPClient
 	my $err;
 	if ($mic->login && $mic->IsAuthenticated) {
 		# success! keep IMAPClient->new arg in case we get disconnected
-		$self->{mic_arg}->{$sec} = $mic_arg;
+		$self->{net_arg}->{$sec} = $mic_arg;
 		if ($cred) {
 			$uri->user($cred->{username}) if !defined($uri->user);
 		} elsif ($mic_arg->{Authmechanism} eq 'ANONYMOUS') {
@@ -230,7 +230,7 @@ W: see https://rt.cpan.org/Ticket/Display.html?id=129967 for updates
 		}
 	}
 
-	$self->{nn_arg}->{$sec} = $nn_arg;
+	$self->{net_arg}->{$sec} = $nn_arg;
 	$cred->run($nn ? 'approve' : 'reject') if $cred && $cred->{filled};
 	$nn;
 }
@@ -306,7 +306,6 @@ sub imap_common_init ($;$) {
 		}
 	}
 	# make sure we can connect and cache the credentials in memory
-	$self->{mic_arg} = {}; # schema://authority => IMAPClient->new args
 	my $mics = {}; # schema://authority => IMAPClient obj
 	for my $orig_uri (@{$self->{imap_order}}) {
 		my $sec = uri_section($orig_uri);
@@ -358,7 +357,6 @@ sub nntp_common_init ($;$) {
 		}
 	}
 	# make sure we can connect and cache the credentials in memory
-	$self->{nn_arg} = {}; # schema://authority => Net::NNTP->new args
 	my %nn; # schema://authority => Net::NNTP object
 	for my $uri (@{$self->{nntp_order}}) {
 		my $sec = uri_section($uri);
@@ -622,7 +620,7 @@ sub mic_get {
 		return $mic if $mic && $mic->IsConnected;
 		delete $cached->{$sec};
 	}
-	my $mic_arg = $self->{mic_arg}->{$sec} or
+	my $mic_arg = $self->{net_arg}->{$sec} or
 			die "BUG: no Mail::IMAPClient->new arg for $sec";
 	if (defined(my $cb_name = $mic_arg->{Authcallback})) {
 		if (ref($cb_name) ne 'CODE') {
@@ -660,7 +658,7 @@ sub nn_get {
 	my $cached = $self->{nn_cached} // {};
 	my $nn;
 	$nn = delete($cached->{$sec}) and return $nn;
-	my $nn_arg = $self->{nn_arg}->{$sec} or
+	my $nn_arg = $self->{net_arg}->{$sec} or
 			die "BUG: no Net::NNTP->new arg for $sec";
 	my $nntp_cfg = $self->{cfg_opt}->{$sec};
 	$nn = nn_new($nn_arg, $nntp_cfg, $uri) or return;
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index 20ce0616..43ee0714 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -353,7 +353,7 @@ sub imap_idle_once ($$$$) {
 sub watch_imap_idle_1 ($$$) {
 	my ($self, $uri, $intvl) = @_;
 	my $sec = uri_section($uri);
-	my $mic_arg = $self->{mic_arg}->{$sec} or
+	my $mic_arg = $self->{net_arg}->{$sec} or
 			die "BUG: no Mail::IMAPClient->new arg for $sec";
 	my $mic;
 	local $0 = $uri->mailbox." $sec";

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 7/8] net_reader: improve naming of common args
  2021-09-09  5:24 [PATCH 0/8] net_reader: simplify + fix bugs Eric Wong
                   ` (5 preceding siblings ...)
  2021-09-09  5:25 ` [PATCH 6/8] net_reader: combine Net::NNTP and IMAPClient args Eric Wong
@ 2021-09-09  5:25 ` Eric Wong
  2021-09-09  5:25 ` [PATCH 8/8] net_reader: support Mail::IMAPClient Ignoresizeerrors Eric Wong
  7 siblings, 0 replies; 9+ messages in thread
From: Eric Wong @ 2021-09-09  5:25 UTC (permalink / raw)
  To: meta

IMHO this makes things easier-to-follow than before.
---
 lib/PublicInbox/NetReader.pm | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index 91e53913..181741f6 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -58,7 +58,7 @@ sub auth_anon_cb { '' }; # for Mail::IMAPClient::Authcallback
 
 # mic_for may prompt the user and store auth info, prepares mic_get
 sub mic_for ($$$$) { # mic = Mail::IMAPClient
-	my ($self, $uri, $mic_args, $lei) = @_;
+	my ($self, $uri, $mic_common, $lei) = @_;
 	require PublicInbox::GitCredential;
 	my $cred = bless {
 		url => "$uri",
@@ -68,7 +68,7 @@ sub mic_for ($$$$) { # mic = Mail::IMAPClient
 		password => $uri->password,
 	}, 'PublicInbox::GitCredential';
 	my $sec = uri_section($uri);
-	my $common = $mic_args->{$sec} // {};
+	my $common = $mic_common->{$sec} // {};
 	# IMAPClient and Net::Netrc both mishandles `0', so we pass `127.0.0.1'
 	my $host = $cred->{host};
 	$host = '127.0.0.1' if $host eq '0';
@@ -174,7 +174,7 @@ E: <$uri> STARTTLS requested and failed
 }
 
 sub nn_for ($$$$) { # nn = Net::NNTP
-	my ($self, $uri, $nn_args, $lei) = @_;
+	my ($self, $uri, $nn_common, $lei) = @_;
 	my $sec = uri_section($uri);
 	my $nntp_cfg = $self->{cfg_opt}->{$sec} //= {};
 	my $host = $uri->host;
@@ -193,7 +193,7 @@ sub nn_for ($$$$) { # nn = Net::NNTP
 		($cred->{username}, $cred->{password}) = ($u, $p);
 		$p //= $cred->check_netrc;
 	}
-	my $common = $nn_args->{$sec} // {};
+	my $common = $nn_common->{$sec} // {};
 	my $nn_arg = {
 		Port => $uri->port,
 		Host => $host,
@@ -282,15 +282,15 @@ sub imap_common_init ($;$) {
 		die "DBD::SQLite is required for IMAP\n:$@\n";
 	require PublicInbox::URIimap;
 	my $cfg = $self->{pi_cfg} // $lei->_lei_cfg;
-	my $mic_args = {}; # scheme://authority => Mail:IMAPClient arg
+	my $mic_common = {}; # scheme://authority => Mail:IMAPClient arg
 	for my $uri (@{$self->{imap_order}}) {
 		my $sec = uri_section($uri);
 		for my $k (qw(Starttls Debug Compress)) {
 			my $bool = cfg_bool($cfg, "imap.$k", $$uri) // next;
-			$mic_args->{$sec}->{$k} = $bool;
+			$mic_common->{$sec}->{$k} = $bool;
 		}
 		my $to = cfg_intvl($cfg, 'imap.timeout', $$uri);
-		$mic_args->{$sec}->{Timeout} = $to if $to;
+		$mic_common->{$sec}->{Timeout} = $to if $to;
 		my $sa = socks_args($cfg->urlmatch('imap.Proxy', $$uri));
 		$self->{cfg_opt}->{$sec}->{-proxy_cfg} = $sa if $sa;
 		for my $k (qw(pollInterval idleInterval)) {
@@ -311,7 +311,7 @@ sub imap_common_init ($;$) {
 		my $sec = uri_section($orig_uri);
 		my $uri = PublicInbox::URIimap->new("$sec/");
 		my $mic = $mics->{$sec} //=
-				mic_for($self, $uri, $mic_args, $lei) //
+				mic_for($self, $uri, $mic_common, $lei) //
 				die "Unable to continue\n";
 		next unless $self->isa('PublicInbox::NetWriter');
 		my $dst = $orig_uri->mailbox // next;
@@ -331,10 +331,10 @@ sub nntp_common_init ($;$) {
 	($lei || eval { require PublicInbox::IMAPTracker }) or
 		die "DBD::SQLite is required for NNTP\n:$@\n";
 	my $cfg = $self->{pi_cfg} // $lei->_lei_cfg;
-	my $nn_args = {}; # scheme://authority => Net::NNTP->new arg
+	my $nn_common = {}; # scheme://authority => Net::NNTP->new arg
 	for my $uri (@{$self->{nntp_order}}) {
 		my $sec = uri_section($uri);
-		my $args = $nn_args->{$sec} //= {};
+		my $args = $nn_common->{$sec} //= {};
 
 		# Debug and Timeout are passed to Net::NNTP->new
 		my $v = cfg_bool($cfg, 'nntp.Debug', $$uri);
@@ -360,7 +360,7 @@ sub nntp_common_init ($;$) {
 	my %nn; # schema://authority => Net::NNTP object
 	for my $uri (@{$self->{nntp_order}}) {
 		my $sec = uri_section($uri);
-		$nn{$sec} //= nn_for($self, $uri, $nn_args, $lei);
+		$nn{$sec} //= nn_for($self, $uri, $nn_common, $lei);
 	}
 	\%nn; # for optional {nn_cached}
 }

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 8/8] net_reader: support Mail::IMAPClient Ignoresizeerrors
  2021-09-09  5:24 [PATCH 0/8] net_reader: simplify + fix bugs Eric Wong
                   ` (6 preceding siblings ...)
  2021-09-09  5:25 ` [PATCH 7/8] net_reader: improve naming of common args Eric Wong
@ 2021-09-09  5:25 ` Eric Wong
  7 siblings, 0 replies; 9+ messages in thread
From: Eric Wong @ 2021-09-09  5:25 UTC (permalink / raw)
  To: meta

Some proprietary servers may do wacky things and give the
wrong size, so Mail::IMAPClient has a knob for this which
we can expose to users to workaround this.
---
 lib/PublicInbox/NetReader.pm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index 181741f6..a0e52fc5 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -285,12 +285,16 @@ sub imap_common_init ($;$) {
 	my $mic_common = {}; # scheme://authority => Mail:IMAPClient arg
 	for my $uri (@{$self->{imap_order}}) {
 		my $sec = uri_section($uri);
-		for my $k (qw(Starttls Debug Compress)) {
+
+		# knobs directly for Mail::IMAPClient->new
+		for my $k (qw(Starttls Debug Compress Ignoresizeerrors)) {
 			my $bool = cfg_bool($cfg, "imap.$k", $$uri) // next;
 			$mic_common->{$sec}->{$k} = $bool;
 		}
 		my $to = cfg_intvl($cfg, 'imap.timeout', $$uri);
 		$mic_common->{$sec}->{Timeout} = $to if $to;
+
+		# knobs we use ourselves:
 		my $sa = socks_args($cfg->urlmatch('imap.Proxy', $$uri));
 		$self->{cfg_opt}->{$sec}->{-proxy_cfg} = $sa if $sa;
 		for my $k (qw(pollInterval idleInterval)) {

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-09-09  5:25 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-09  5:24 [PATCH 0/8] net_reader: simplify + fix bugs Eric Wong
2021-09-09  5:24 ` [PATCH 1/8] net_reader: do not set "SSL" fields for non-TLS Eric Wong
2021-09-09  5:24 ` [PATCH 2/8] net_reader: set IMAPClient Keepalive flag late Eric Wong
2021-09-09  5:25 ` [PATCH 3/8] net_reader: preserve memoized IMAPClient arg for SOCKS Eric Wong
2021-09-09  5:25 ` [PATCH 4/8] net_reader: nntp_opt => cfg_opt Eric Wong
2021-09-09  5:25 ` [PATCH 5/8] net_reader: imap_opt " Eric Wong
2021-09-09  5:25 ` [PATCH 6/8] net_reader: combine Net::NNTP and IMAPClient args Eric Wong
2021-09-09  5:25 ` [PATCH 7/8] net_reader: improve naming of common args Eric Wong
2021-09-09  5:25 ` [PATCH 8/8] net_reader: support Mail::IMAPClient Ignoresizeerrors Eric Wong

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 NNTP newsgroup(s).