user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH] *search: avoid HV modification on conditional assignment
Date: Sat, 27 Jul 2024 08:37:48 +0000	[thread overview]
Message-ID: <20240727083748.4159188-1-e@80x24.org> (raw)

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.
---
 lib/PublicInbox/CodeSearch.pm | 2 +-
 lib/PublicInbox/MiscSearch.pm | 4 ++--
 lib/PublicInbox/Search.pm     | 6 +++---
 lib/PublicInbox/XapHelper.pm  | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/CodeSearch.pm b/lib/PublicInbox/CodeSearch.pm
index e5fa4480..3838fd1b 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 5fb47d03..ad7ebeb4 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 eb5e67ba..649157be 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 ba41b5d2..7e61631c 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;

                 reply	other threads:[~2024-07-27  8:37 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://public-inbox.org/README

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240727083748.4159188-1-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).