The recent rawstr fix caused some errors for existing --stdin searches, these patches fix them and make some minor improvements along the way. Eric Wong (4): ipc: note failing sub name lei up: infer rawstr from old searches via trailing "\n" lei q: disallow "\n" in argv[] elements lei q: make HTTP(S) query strings even less ugly lib/PublicInbox/IPC.pm | 2 +- lib/PublicInbox/LeiQuery.pm | 1 + lib/PublicInbox/LeiUp.pm | 4 +++- lib/PublicInbox/LeiXSearch.pm | 2 +- t/lei.t | 3 +++ 5 files changed, 9 insertions(+), 3 deletions(-)
For --stdin searches created prior to commit 666dde69a3f6 (lei q|up: fix saved searches for single-phrase search, 2021-11-08) we still want to be able to run "lei up" on them without regressions. So assume nobody manages to enter "\n" as an argv[] element and consider the presence of "\n" as a previous --stdin use. This fixes errors from "lei up" such as: lei_xsearch 2 wq_worker: Exception: Key too long: length was 840 bytes, maximum length of a key is 255 bytes at ../PublicInbox/IPC.pm line 250. Fixes: 666dde69a3f6 ("lei q|up: fix saved searches for single-phrase search") --- lib/PublicInbox/LeiUp.pm | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/PublicInbox/LeiUp.pm b/lib/PublicInbox/LeiUp.pm index d7873a3f3469..b8a9836075ba 100644 --- a/lib/PublicInbox/LeiUp.pm +++ b/lib/PublicInbox/LeiUp.pm @@ -29,7 +29,9 @@ sub up1 ($$) { my $q = $lss->{-cfg}->get_all('lei.q') // die("lei.q unset in $f (out=$out)\n"); my $lse = $lei->{lse} // die 'BUG: {lse} missing'; - if ($lss->{-cfg}->{'lei.internal.rawstr'}) { + my $rawstr = $lss->{-cfg}->{'lei.internal.rawstr'} // + (scalar(@$q) == 1 && substr($q->[0], -1) eq "\n"); + if ($rawstr) { scalar(@$q) > 1 and die "$f: lei.q has multiple values (@$q) (out=$out)\n"; $lse->query_approxidate($lse->git, $mset_opt->{qstr} = $q->[0]);
Hopefully problems can get diagnosed more quickly with the sub name in the error message. --- lib/PublicInbox/IPC.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm index 3e299448b334..8376275e3c95 100644 --- a/lib/PublicInbox/IPC.pm +++ b/lib/PublicInbox/IPC.pm @@ -247,7 +247,7 @@ sub recv_and_run { undef $buf; my $sub = shift @$args; eval { $self->$sub(@$args) }; - warn "$$ $0 wq_worker: $@" if $@; + warn "$$ $0 wq_worker: $sub: $@" if $@; delete @$self{0..($nfd-1)}; $n; }
I don't expect this to be hit in real-world use via normal interactive shells. However, somebody could accidentally add "\n" in languages (e.g. Perl, C) where it's easy to pass "\n" in argv[]. --- lib/PublicInbox/LeiQuery.pm | 1 + t/lei.t | 3 +++ 2 files changed, 4 insertions(+) diff --git a/lib/PublicInbox/LeiQuery.pm b/lib/PublicInbox/LeiQuery.pm index 352ee60131aa..51ee3d9c83e4 100644 --- a/lib/PublicInbox/LeiQuery.pm +++ b/lib/PublicInbox/LeiQuery.pm @@ -141,6 +141,7 @@ no query allowed on command-line with --stdin PublicInbox::InputPipe::consume($self->{0}, \&qstr_add, $self); return; } + chomp(@argv) and $self->qerr("# trailing `\\n' removed"); $mset_opt{q_raw} = [ @argv ]; # copy $mset_opt{qstr} = $self->{lse}->query_argv_to_string($self->{lse}->git, \@argv); diff --git a/t/lei.t b/t/lei.t index f7de1b711a83..b10c9b59c72b 100644 --- a/t/lei.t +++ b/t/lei.t @@ -143,6 +143,9 @@ my $test_fail = sub { lei('-C', '/dev/null', 'q', 'whatever'); is($? >> 8, 1, 'chdir at beginning fails to /dev/null'); + lei_ok('q', "foo\n"); + like($lei_err, qr/trailing `\\n' removed/s, "noted `\\n' removal"); + for my $lk (qw(ei inbox)) { my $d = "$home/newline\n$lk"; mkdir $d;
Following commit 57fed2e4b78ed394 (lei: normalize whitespace in remote queries, 2021-09-11), leaving the trailing `\n' from stdin queries to be normalized to ` ' (SP) causes it to appear as `+' in URLs, which Xapian ignores. --- lib/PublicInbox/LeiXSearch.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm index 29df07e0c8a8..2958d3f910b0 100644 --- a/lib/PublicInbox/LeiXSearch.pm +++ b/lib/PublicInbox/LeiXSearch.pm @@ -342,7 +342,7 @@ sub query_remote_mboxrd { local $SIG{TERM} = sub { exit(0) }; # for DESTROY (File::Temp, $reap) my $lei = $self->{lei}; my $opt = $lei->{opt}; - my $qstr = $lei->{mset_opt}->{qstr}; + chomp(my $qstr = $lei->{mset_opt}->{qstr}); $qstr =~ s/[ \n\t]+/ /sg; # make URLs less ugly my @qform = (x => 'm'); push(@qform, t => 1) if $opt->{threads};