From 07c97b4f2ebefd8a6739487373eb5afbe0301b98 Mon Sep 17 00:00:00 2001 From: Norbert Orzechowicz Date: Wed, 24 Jun 2026 22:07:10 +0200 Subject: [PATCH] fix(flow-php/pg-query-ext): verify libpg_query PostgreSQL major before linking - reject system libpg_query whose PG major differs from the pinned version - hard error on mismatched --with-pg-query, skip auto-discovered mismatches - report decoded node and parse-result PG version on SelectStmt mismatch --- documentation/contributing/c.md | 6 +++ src/extension/pg-query-ext/ext/config.m4 | 38 ++++++++++---- .../PostgreSql/Parser/ColumnTypeParser.php | 9 +++- .../PostgreSql/Parser/ExpressionParser.php | 9 +++- .../Exception/InvalidAstException.php | 11 ++++- .../Exception/InvalidAstExceptionTest.php | 49 +++++++++++++++++++ 6 files changed, 107 insertions(+), 15 deletions(-) create mode 100644 src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/QueryBuilder/Exception/InvalidAstExceptionTest.php diff --git a/documentation/contributing/c.md b/documentation/contributing/c.md index ebc1ed95d3..141adac84a 100644 --- a/documentation/contributing/c.md +++ b/documentation/contributing/c.md @@ -47,6 +47,12 @@ Build the extension (first build downloads libpg_query automatically): nix-shell --arg with-pg-query-ext false --arg with-c true --run "cd src/extension/pg-query-ext && make build" ``` +A system `libpg_query` is only used when its `PG_MAJORVERSION` matches the pinned `LIBPG_QUERY_VERSION` +in `config.m4`. A mismatched `--with-pg-query=DIR` is a hard configure error; a mismatched +auto-discovered library (e.g. a Homebrew keg for a different PostgreSQL major) is skipped in favor of +downloading the pinned version. This prevents silently baking the wrong PostgreSQL grammar into +`pg_query.so` ([#2483](https://github.com/flow-php/flow/issues/2483)). + Run PHPT tests: ```bash diff --git a/src/extension/pg-query-ext/ext/config.m4 b/src/extension/pg-query-ext/ext/config.m4 index 39169778e9..cff0ccf7f2 100644 --- a/src/extension/pg-query-ext/ext/config.m4 +++ b/src/extension/pg-query-ext/ext/config.m4 @@ -12,29 +12,47 @@ if test "$PHP_PG_QUERY" != "no"; then PG_QUERY_DIR="" + dnl Expected PostgreSQL major derived from the pinned version ("18-latest" -> "18", "18.0.0" -> "18") + PG_EXPECTED_MAJOR=`echo "$LIBPG_QUERY_VERSION" | sed -E 's/^([[0-9]]+).*/\1/'` + dnl Search for existing libpg_query installation if test "$PHP_PG_QUERY" != "yes" && test -n "$PHP_PG_QUERY"; then SEARCH_PATH="$PHP_PG_QUERY" + PG_QUERY_EXPLICIT="yes" else SEARCH_PATH="/usr/local /usr /opt/local /opt/homebrew" + PG_QUERY_EXPLICIT="no" fi AC_MSG_CHECKING([for libpg_query]) for i in $SEARCH_PATH ; do - dnl Check flat directory structure (headers and lib in same dir) + dnl Locate headers + static lib in either flat or include/lib layout + _hdr=""; _inc=""; _lib="" if test -r "$i/pg_query.h" && test -r "$i/postgres_deparse.h" && test -r "$i/libpg_query.a"; then - PG_QUERY_DIR=$i - AC_MSG_RESULT([found in $i]) - break + _hdr="$i/pg_query.h"; _inc="$i"; _lib="$i" + elif test -r "$i/include/pg_query.h" && test -r "$i/include/postgres_deparse.h" && test -r "$i/lib/libpg_query.a"; then + _hdr="$i/include/pg_query.h"; _inc="$i/include"; _lib="$i/lib" + fi + + if test -z "$_hdr"; then + continue fi - dnl Check standard include/lib directory structure - if test -r "$i/include/pg_query.h" && test -r "$i/include/postgres_deparse.h" && test -r "$i/lib/libpg_query.a"; then - PG_QUERY_DIR=$i - PG_QUERY_INCLUDE_DIR="$i/include" - PG_QUERY_LIB_DIR="$i/lib" - AC_MSG_RESULT([found in $i]) + + dnl PG major is a macro in the header we already require: #define PG_MAJORVERSION "18" + _found_major=`sed -nE 's/^#define[[:space:]]+PG_MAJORVERSION[[:space:]]+"([0-9]+)".*/\1/p' "$_hdr"` + + if test "x$_found_major" = "x$PG_EXPECTED_MAJOR"; then + PG_QUERY_DIR="$i" + PG_QUERY_INCLUDE_DIR="$_inc" + PG_QUERY_LIB_DIR="$_lib" + AC_MSG_RESULT([found in $i (PostgreSQL $_found_major)]) break + elif test "$PG_QUERY_EXPLICIT" = "yes"; then + AC_MSG_RESULT([version mismatch]) + AC_MSG_ERROR([libpg_query in $i is for PostgreSQL ${_found_major:-unknown}, but this extension targets PostgreSQL $PG_EXPECTED_MAJOR (libpg_query $LIBPG_QUERY_VERSION). Point --with-pg-query at a matching install, or omit it to download the pinned version.]) + else + AC_MSG_WARN([ignoring libpg_query in $i: PostgreSQL ${_found_major:-unknown}, need $PG_EXPECTED_MAJOR]) fi done diff --git a/src/lib/postgresql/src/Flow/PostgreSql/Parser/ColumnTypeParser.php b/src/lib/postgresql/src/Flow/PostgreSql/Parser/ColumnTypeParser.php index 4802a43fbf..f291706bd6 100644 --- a/src/lib/postgresql/src/Flow/PostgreSql/Parser/ColumnTypeParser.php +++ b/src/lib/postgresql/src/Flow/PostgreSql/Parser/ColumnTypeParser.php @@ -28,10 +28,15 @@ public function parse(string $typeName): ColumnType throw InvalidAstException::invalidFieldValue('stmts', 'ParseResult', 'expected at least one statement'); } - $selectStmt = $stmts[0]->getStmt()?->getSelectStmt(); + $stmt = $stmts[0]->getStmt(); + $selectStmt = $stmt?->getSelectStmt(); if ($selectStmt === null) { - throw InvalidAstException::unexpectedNodeType('SelectStmt', 'unknown'); + throw InvalidAstException::unexpectedNodeType( + 'SelectStmt', + $stmt?->getNode() ?: 'unknown', + $parsed->raw()->getVersion(), + ); } $targetList = $selectStmt->getTargetList(); diff --git a/src/lib/postgresql/src/Flow/PostgreSql/Parser/ExpressionParser.php b/src/lib/postgresql/src/Flow/PostgreSql/Parser/ExpressionParser.php index 04fb43a3b1..2deab663fa 100644 --- a/src/lib/postgresql/src/Flow/PostgreSql/Parser/ExpressionParser.php +++ b/src/lib/postgresql/src/Flow/PostgreSql/Parser/ExpressionParser.php @@ -79,10 +79,15 @@ public function parse(string $expression): Node throw InvalidAstException::invalidFieldValue('stmts', 'ParseResult', 'expected at least one statement'); } - $selectStmt = $stmts[0]->getStmt()?->getSelectStmt(); + $stmt = $stmts[0]->getStmt(); + $selectStmt = $stmt?->getSelectStmt(); if ($selectStmt === null) { - throw InvalidAstException::unexpectedNodeType('SelectStmt', 'unknown'); + throw InvalidAstException::unexpectedNodeType( + 'SelectStmt', + $stmt?->getNode() ?: 'unknown', + $parsed->raw()->getVersion(), + ); } $targetList = $selectStmt->getTargetList(); diff --git a/src/lib/postgresql/src/Flow/PostgreSql/QueryBuilder/Exception/InvalidAstException.php b/src/lib/postgresql/src/Flow/PostgreSql/QueryBuilder/Exception/InvalidAstException.php index 5789dd9247..7f82fbb978 100644 --- a/src/lib/postgresql/src/Flow/PostgreSql/QueryBuilder/Exception/InvalidAstException.php +++ b/src/lib/postgresql/src/Flow/PostgreSql/QueryBuilder/Exception/InvalidAstException.php @@ -21,8 +21,17 @@ public static function missingRequiredField(string $field, string $nodeType): se return new self(sprintf('Missing required field "%s" in %s node', $field, $nodeType)); } - public static function unexpectedNodeType(string $expected, string $actual): self + public static function unexpectedNodeType(string $expected, string $actual, ?int $version = null): self { + if ($version !== null) { + return new self(sprintf( + 'Expected %s node, got "%s" (parse result reports PostgreSQL %d); the pg_query extension was likely built against a different PostgreSQL major than this package expects', + $expected, + $actual, + $version, + )); + } + return new self(sprintf('Expected %s node, got %s', $expected, $actual)); } } diff --git a/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/QueryBuilder/Exception/InvalidAstExceptionTest.php b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/QueryBuilder/Exception/InvalidAstExceptionTest.php new file mode 100644 index 0000000000..0e9800c3f8 --- /dev/null +++ b/src/lib/postgresql/tests/Flow/PostgreSql/Tests/Unit/QueryBuilder/Exception/InvalidAstExceptionTest.php @@ -0,0 +1,49 @@ +getMessage(), + ); + } + + public function test_missing_required_field(): void + { + static::assertSame( + 'Missing required field "val" in ResTarget node', + InvalidAstException::missingRequiredField('val', 'ResTarget')->getMessage(), + ); + } + + public function test_unexpected_node_type_with_version(): void + { + $message = InvalidAstException::unexpectedNodeType('SelectStmt', 'update_stmt', 170007)->getMessage(); + + static::assertStringContainsString('Expected SelectStmt node', $message); + static::assertStringContainsString('"update_stmt"', $message); + static::assertStringContainsString('170007', $message); + static::assertStringContainsString('different PostgreSQL major', $message); + } + + public function test_unexpected_node_type_without_version(): void + { + static::assertSame( + 'Expected SelectStmt node, got unknown', + InvalidAstException::unexpectedNodeType('SelectStmt', 'unknown')->getMessage(), + ); + } +}