From: Eric Wong <e@80x24.org>
Date: Sat, 27 Jul 2024 08:37:48 +0000 (+0000)
Subject: *search: avoid HV modification on conditional assignment
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8ea1774201c1a9228d5329070d31446c92062a02;p=thirdparty%2Fpublic-inbox.git

*search: avoid HV modification on conditional assignment

Setting {qp} higher up the stack via `//=' is unsafe since
->qp_init_common calls ->xdb which can also set {xdb}, {nshard},
and {qp_flags}.

While this doesn't fix any known bugs in current versions of
Perl or our current code, it surfaced while working on new
search features on our end.
---

diff --git a/lib/PublicInbox/CodeSearch.pm b/lib/PublicInbox/CodeSearch.pm
index e5fa4480a..3838fd1b0 100644
--- a/lib/PublicInbox/CodeSearch.pm
+++ b/lib/PublicInbox/CodeSearch.pm
@@ -159,7 +159,7 @@ sub roots_filter { # retry_reopen callback
 
 sub mset {
 	my ($self, $qry_str, $opt) = @_;
-	my $qp = $self->{qp} //= qparse_new($self);
+	my $qp = $self->{qp} // qparse_new($self);
 	my $qry = $qp->parse_query($qry_str, $self->{qp_flags});
 
 	# limit to commits with shared roots
diff --git a/lib/PublicInbox/MiscSearch.pm b/lib/PublicInbox/MiscSearch.pm
index 5fb47d03d..ad7ebeb4e 100644
--- a/lib/PublicInbox/MiscSearch.pm
+++ b/lib/PublicInbox/MiscSearch.pm
@@ -38,7 +38,7 @@ sub new {
 sub mi_qp_new ($) {
 	my ($self) = @_;
 	my $xdb = $self->{xdb};
-	my $qp = $PublicInbox::Search::X{QueryParser}->new;
+	my $qp = $self->{qp} = $PublicInbox::Search::X{QueryParser}->new;
 	$qp->set_default_op(PublicInbox::Search::OP_AND());
 	$qp->set_database($xdb);
 	$qp->set_stemmer(PublicInbox::Search::stemmer($self));
@@ -76,7 +76,7 @@ sub mset {
 	my ($self, $qs, $opt) = @_;
 	$opt ||= {};
 	reopen($self);
-	my $qp = $self->{qp} //= mi_qp_new($self);
+	my $qp = $self->{qp} // mi_qp_new($self);
 	$qs = 'type:inbox' if $qs eq '';
 	my $qr = $qp->parse_query($qs, $PublicInbox::Search::QP_FLAGS);
 	$opt->{relevance} = 1 unless exists $opt->{relevance};
diff --git a/lib/PublicInbox/Search.pm b/lib/PublicInbox/Search.pm
index eb5e67ba5..649157bef 100644
--- a/lib/PublicInbox/Search.pm
+++ b/lib/PublicInbox/Search.pm
@@ -413,7 +413,7 @@ sub query_approxidate {
 # read-only, for mail only (codesearch has different rules)
 sub mset {
 	my ($self, $qry_str, $opt) = @_;
-	my $qp = $self->{qp} //= $self->qparse_new;
+	my $qp = $self->{qp} // $self->qparse_new;
 	my $qry = $qp->parse_query($qry_str, $self->{qp_flags});
 	if (defined(my $eidx_key = $opt->{eidx_key})) {
 		$qry = $X{Query}->new(OP_FILTER(), $qry, 'O'.$eidx_key);
@@ -570,7 +570,7 @@ sub stemmer { $X{Stem}->new($LANG) }
 
 sub qp_init_common {
 	my ($self) = @_;
-	my $qp = $X{QueryParser}->new;
+	my $qp = $self->{qp} = $X{QueryParser}->new;
 	$qp->set_default_op(OP_AND());
 	$qp->set_database(xdb($self));
 	$qp->set_stemmer(stemmer($self));
@@ -654,7 +654,7 @@ EOM
 
 sub help {
 	my ($self) = @_;
-	$self->{qp} //= $self->qparse_new; # parse altids
+	$self->{qp} // $self->qparse_new; # parse altids
 	my @ret = @HELP;
 	if (my $user_pfx = $self->{-user_pfx}) {
 		push @ret, @$user_pfx;
diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index ba41b5d23..7e61631c0 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -225,7 +225,7 @@ sub dispatch {
 		}
 		bless $new, $req->{c} ? 'PublicInbox::CodeSearch' :
 					'PublicInbox::Search';
-		$new->{qp} = $new->qparse_new;
+		$new->qparse_new;
 		$SRCH{$key} = $new;
 	};
 	$req->{srch}->{xdb}->reopen unless $new;