index: account for CRLF conversion when storing bytes

NNTP and IMAP both require CRLF conversions on the wire.
They're also the only components which care about
$smsg->{bytes}, so store the CRLF-adjusted value in over.sqlite3
and Xapian DBs..

This will allow us to optimize RFC822.SIZE fetch item in IMAP
without triggering size mismatch errors in some clients' default
configurations (e.g. Mail::IMAPClient), but not most others.

It could also fix hypothetical problems with NNTP clients that
report discrepancies between overview and article data.

diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index ab75aa00dc2e3ed63e0b4ce74955cc43aeac88ed..af35905be4964b87ad9ea5dcb7eb78b7fe3e9d9a 100644 (file)
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -400,7 +400,7 @@ sub add {
        # v2: we need this for Xapian
        if ($smsg) {
                $smsg->{blob} = $self->get_mark(":$blob");
-               $smsg->{bytes} = $n;
+               $smsg->{raw_bytes} = $n;
                $smsg->{-raw_email} = \$raw_email;
        }
        my $ref = $self->{ref};

diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index a790ac4076acefbd6b50b68ae706fc2fe0ae40fc..85821ea706a41a66a9a0e1aee3c40aecab6c969a 100644 (file)
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -549,11 +549,23 @@ sub unindex_mm {
        $self->{mm}->mid_delete(mid_mime($mime));
 }
 
+# returns the number of bytes to add if given a non-CRLF arg
+sub crlf_adjust ($) {
+       if (index($_[0], "\r\n") < 0) {
+               # common case is LF-only, every \n needs an \r;
+               # so favor a cheap tr// over an expensive m//g
+               $_[0] =~ tr/\n/\n/;
+       } else { # count number of '\n' w/o '\r', expensive:
+               scalar(my @n = ($_[0] =~ m/(?<!\r)\n/g));
+       }
+}
+
 sub index_both { # git->cat_async callback
        my ($bref, $oid, $type, $size, $sync) = @_;
        my ($nr, $max) = @$sync{qw(nr max)};
        ++$$nr;
        $$max -= $size;
+       $size += crlf_adjust($$bref);
        my $smsg = bless { bytes => $size, blob => $oid }, 'PublicInbox::Smsg';
        my $self = $sync->{sidx};
        my $eml = PublicInbox::Eml->new($bref);

diff --git a/lib/PublicInbox/SearchIdxShard.pm b/lib/PublicInbox/SearchIdxShard.pm
index c1f52d8b884fd7e723275f8e8c259aaf2ace4092..f7ba293ff5bb70f91a20464ff504cc1a12e2748f 100644 (file)
--- a/lib/PublicInbox/SearchIdxShard.pm
+++ b/lib/PublicInbox/SearchIdxShard.pm
@@ -71,11 +71,11 @@ sub shard_worker_loop ($$$$$) {
                } else {
                        chomp $line;
                        # n.b. $mid may contain spaces(!)
-                       my ($bytes, $num, $blob, $ds, $ts, $mid) =
-                                                       split(/ /, $line, 6);
+                       my ($to_read, $bytes, $num, $blob, $ds, $ts, $mid) =
+                                                       split(/ /, $line, 7);
                        $self->begin_txn_lazy;
-                       my $n = read($r, my $msg, $bytes) or die "read: $!\n";
-                       $n == $bytes or die "short read: $n != $bytes\n";
+                       my $n = read($r, my $msg, $to_read) or die "read: $!\n";
+                       $n == $to_read or die "short read: $n != $to_read\n";
                        my $mime = PublicInbox::Eml->new(\$msg);
                        my $smsg = bless {
                                bytes => $bytes,
@@ -96,7 +96,8 @@ sub index_raw {
        my ($self, $msgref, $mime, $smsg) = @_;
        if (my $w = $self->{w}) {
                # mid must be last, it can contain spaces (but not LF)
-               print $w join(' ', @$smsg{qw(bytes num blob ds ts mid)}),
+               print $w join(' ', @$smsg{qw(raw_bytes bytes
+                                               num blob ds ts mid)}),
                        "\n", $$msgref or die "failed to write shard $!\n";
        } else {
                $$msgref = undef;

diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index 79bee7f9f3d1125fbd918d27ac9d0c7c85cd88f3..913794316330bbc261d6f236f0e5a355c5b11ec5 100644 (file)
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -155,10 +155,12 @@ sub add {
 # indexes a message, returns true if checkpointing is needed
 sub do_idx ($$$$) {
        my ($self, $msgref, $mime, $smsg) = @_;
+       $smsg->{bytes} = $smsg->{raw_bytes} +
+                       PublicInbox::SearchIdx::crlf_adjust($$msgref);
        $self->{over}->add_overview($mime, $smsg);
        my $idx = idx_shard($self, $smsg->{num} % $self->{shards});
        $idx->index_raw($msgref, $mime, $smsg);
-       my $n = $self->{transact_bytes} += $smsg->{bytes};
+       my $n = $self->{transact_bytes} += $smsg->{raw_bytes};
        $n >= ($PublicInbox::SearchIdx::BATCH_BYTES * $self->{shards});
 }
 
@@ -568,7 +570,7 @@ W: $list
        for my $smsg (@$need_reindex) {
                my $new_smsg = bless {
                        blob => $blob,
-                       bytes => $bytes,
+                       raw_bytes => $bytes,
                        num => $smsg->{num},
                        mid => $smsg->{mid},
                }, 'PublicInbox::Smsg';
@@ -962,7 +964,7 @@ sub reindex_oid_m ($$$$;$) {
        }
        $sync->{nr}++;
        my $smsg = bless {
-               bytes => $len,
+               raw_bytes => $len,
                num => $num,
                blob => $oid,
                mid => $mid0,
@@ -1054,7 +1056,7 @@ sub reindex_oid ($$$$) {
                die "failed to delete <$mid0> for article #$num\n";
        $sync->{nr}++;
        my $smsg = bless {
-               bytes => $len,
+               raw_bytes => $len,
                num => $num,
                blob => $oid,
                mid => $mid0,

diff --git a/t/import.t b/t/import.t
index f987b1141f7c37640195476908b816dce7b3d353..abbc8229d0e7aaf91099ca9b23462c40660ebce5 100644 (file)
--- a/t/import.t
+++ b/t/import.t
@@ -32,8 +32,9 @@ like($im->add($mime, undef, $smsg), qr/\A:[0-9]+\z/, 'added one message');
 
 if ($v2) {
        like($smsg->{blob}, qr/\A[a-f0-9]{40}\z/, 'got last object_id');
-       is($mime->as_string, ${$smsg->{-raw_email}}, 'string matches');
-       is($smsg->{bytes}, length(${$smsg->{-raw_email}}), 'length matches');
+       my $raw_email = $smsg->{-raw_email};
+       is($mime->as_string, $$raw_email, 'string matches');
+       is($smsg->{raw_bytes}, length($$raw_email), 'length matches');
        my @cmd = ('git', "--git-dir=$git->{git_dir}", qw(hash-object --stdin));
        my $in = tempfile();
        print $in $mime->as_string or die "write failed: $!";

diff --git a/t/nntpd.t b/t/nntpd.t
index eee67ea65bb8f72317baf291427fc21d4622d4f0..d2f313231159cc7df01942b869fc8b82b9f07190 100644 (file)
--- a/t/nntpd.t
+++ b/t/nntpd.t
@@ -73,7 +73,10 @@ EOF
                my $list_id = $addr;
                $list_id =~ s/@/./;
                $mime->header_set('List-Id', "<$list_id>");
-               $len = length($mime->as_string);
+               my $str = $mime->as_string;
+               $str =~ s/(?<!\r)\n/\r\n/sg;
+               $len = length($str);
+               undef $str;
                $im->add($mime);
                $im->done;
                if ($version == 1) {

diff --git a/t/search.t b/t/search.t
index d4ca28c794f8255b648100355c40726c3e3e88c5..82caf9e41c3dc5170514db48f82ac52e9602b54a 100644 (file)
--- a/t/search.t
+++ b/t/search.t
@@ -59,6 +59,14 @@ sub oct_is ($$$) {
        }
 }
 
+{
+       my $crlf_adjust = \&PublicInbox::SearchIdx::crlf_adjust;
+       is($crlf_adjust->("hi\r\nworld\r\n"), 0, 'no adjustment needed');
+       is($crlf_adjust->("hi\nworld\n"), 2, 'LF-only counts two CR');
+       is($crlf_adjust->("hi\r\nworld\n"), 1, 'CRLF/LF-mix 1 counts 1 CR');
+       is($crlf_adjust->("hi\nworld\r\n"), 1, 'CRLF/LF-mix 2 counts 1 CR');
+}
+
 $ibx->with_umask(sub {
        my $root = PublicInbox::Eml->new(<<'EOF');
 Date: Fri, 02 Oct 1993 00:00:00 +0000