From: Tom Lane Date: Fri, 30 Jan 2026 19:59:25 +0000 (-0500) Subject: Improve guards against false regex matches in BackgroundPsql.pm. X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fc84b3977e20bc6ca7db349401d7f8ad02a90d82;p=thirdparty%2Fpostgresql.git Improve guards against false regex matches in BackgroundPsql.pm. BackgroundPsql needs to wait for all the output from an interactive psql command to come back. To make sure that's happened, it issues the command, then issues \echo and \warn psql commands that echo a "banner" string (which we assume won't appear in the command's output), then waits for the banner strings to appear. The hazard in this approach is that the banner will also appear in the echoed psql commands themselves, so we need to distinguish those echoes from the desired output. Commit 8b886a4e3 tried to do that by positing that the desired output would be directly preceded and followed by newlines, but it turns out that that assumption is timing-sensitive. In particular, it tends to fail in builds made --without-readline, wherein the command echoes will be made by the pty driver and may be interspersed with prompts issued by psql proper. It does seem safe to assume that the banner output we want will be followed by a newline, since that should be the last output before things quiesce. Therefore, we can improve matters by putting quotes around the banner strings in the \echo and \warn psql commands, so that their echoes cannot include banner directly followed by newline, and then checking for just banner-and-newline in the match pattern. While at it, spruce up the pump() call in sub query() to look like the neater version in wait_connect(), and don't die on timeout until after printing whatever we got. Reported-by: Oleg Tselebrovskiy Diagnosed-by: Oleg Tselebrovskiy Author: Tom Lane Reviewed-by: Soumya S Murali Discussion: https://postgr.es/m/db6fdb35a8665ad3c18be01181d44b31@postgrespro.ru Backpatch-through: 14 --- diff --git a/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm b/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm index 4e0b73571dc..e0a67751564 100644 --- a/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm +++ b/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm @@ -152,11 +152,11 @@ sub wait_connect # # See query() for details about why/how the banner is used. my $banner = "background_psql: ready"; - my $banner_match = qr/(^|\n)$banner\r?\n/; - $self->{stdin} .= "\\echo $banner\n\\warn $banner\n"; + my $banner_match = qr/$banner\r?\n/; + $self->{stdin} .= "\\echo '$banner'\n\\warn '$banner'\n"; $self->{run}->pump() until ($self->{stdout} =~ /$banner_match/ - && $self->{stderr} =~ /$banner\r?\n/) + && $self->{stderr} =~ /$banner_match/) || $self->{timeout}->is_expired; note "connect output:\n", @@ -256,22 +256,17 @@ sub query # stderr (or vice versa), even if psql printed them in the opposite # order. We therefore wait on both. # - # We need to match for the newline, because we try to remove it below, and - # it's possible to consume just the input *without* the newline. In - # interactive psql we emit \r\n, so we need to allow for that. Also need - # to be careful that we don't e.g. match the echoed \echo command, rather - # than its output. + # In interactive psql we emit \r\n, so we need to allow for that. + # Also, include quotes around the banner string in the \echo and \warn + # commands, not because the string needs quoting but so that $banner_match + # can't match readline's echoing of these commands. my $banner = "background_psql: QUERY_SEPARATOR $query_cnt:"; - my $banner_match = qr/(^|\n)$banner\r?\n/; - $self->{stdin} .= "$query\n;\n\\echo $banner\n\\warn $banner\n"; - pump_until( - $self->{run}, $self->{timeout}, - \$self->{stdout}, qr/$banner_match/); - pump_until( - $self->{run}, $self->{timeout}, - \$self->{stderr}, qr/$banner_match/); - - die "psql query timed out" if $self->{timeout}->is_expired; + my $banner_match = qr/$banner\r?\n/; + $self->{stdin} .= "$query\n;\n\\echo '$banner'\n\\warn '$banner'\n"; + $self->{run}->pump() + until ($self->{stdout} =~ /$banner_match/ + && $self->{stderr} =~ /$banner_match/) + || $self->{timeout}->is_expired; note "results query $query_cnt:\n", explain { @@ -279,9 +274,12 @@ sub query stderr => $self->{stderr}, }; - # Remove banner from stdout and stderr, our caller doesn't care. The - # first newline is optional, as there would not be one if consuming an - # empty query result. + die "psql query timed out" if $self->{timeout}->is_expired; + + # Remove banner from stdout and stderr, our caller doesn't want it. + # Also remove the query output's trailing newline, if present (there + # would not be one if consuming an empty query result). + $banner_match = qr/\r?\n?$banner\r?\n/; $output = $self->{stdout}; $output =~ s/$banner_match//; $self->{stderr} =~ s/$banner_match//;