From 1b7943ee9dade40ce37c273c1d7bd4c8f3211e87 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 4 Aug 2026 15:33:25 +0000 Subject: [PATCH 01/17] Skip the open_basedir test on Windows Both paths in 003-open-basedir.phpt are POSIX absolute paths and the test has no --SKIPIF--, so on Windows it fails for reasons unrelated to what it tests. That is what aborted the Windows run in maxmind/MaxMind-DB-Reader-php-ext#2 before the self-containment gate step was reached, leaving that repository's gate unexercised on the platform it was added for. The condition is deliberately the platform alone. 001 and 002 both skip when the extension is not loaded, which makes this the one test that *fails* rather than skips when a build produces an extension that cannot load -- the property that made run-tests.php notice the Windows object-collision bug. An extension_loaded() guard here would remove it, so the comment says so. PHP_OS_FAMILY is available from PHP 7.2, this extension's floor. Co-Authored-By: Claude Opus 5 (1M context) --- ext/tests/003-open-basedir.phpt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ext/tests/003-open-basedir.phpt b/ext/tests/003-open-basedir.phpt index 26e97819..b70191bc 100644 --- a/ext/tests/003-open-basedir.phpt +++ b/ext/tests/003-open-basedir.phpt @@ -1,5 +1,14 @@ --TEST-- openbase_dir is followed +--SKIPIF-- + --INI-- open_basedir=/--dne-- --FILE-- From cfece4ade6d0cadb551d8228a1dd772a20d7a341 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 4 Aug 2026 15:33:40 +0000 Subject: [PATCH 02/17] Add a self-containment gate for the built extension Refuses to ship an extension object that is not self-contained. Every check exists because the failure it catches would otherwise reach users as a binary that loads on the build machine and nowhere else. On Linux: no libmaxminddb in DT_NEEDED, nothing outside the C runtime and the loader in DT_NEEDED at all, no RUNPATH or RPATH, no undefined MMDB_ symbols, no *exported* MMDB_ symbols, get_module still exported, no GLIBC_PRIVATE, and a measured glibc floor within a documented maximum. On macOS the same shape against otool and Mach-O, with the deployment target standing in for the glibc floor and every slice of a universal object measured rather than just the first. Two rules shape the implementation. Every tool runs in its own assignment and the greps read captured output. `set -e` is suspended inside an `if` condition, so `if some-tool "$so" | grep ...; then fail; fi` cannot distinguish "the tool found nothing" from "the tool is not installed" -- both are a non-zero pipeline and both skip the fail. A gate whose checks pass when their tools are missing is worse than no gate, because it reports success. An unmeasurable result is a failure, not a pass. An empty DT_NEEDED list, an unreadable glibc floor, an unreadable LC_BUILD_VERSION minos and an empty operand to at_most are all fatal, because each means the measurement did not happen rather than that it came back clean. The limit comes from the caller. MAX_GLIBC and MACOSX_DEPLOYMENT_TARGET are read from the environment and `set -u` turns a caller that forgets them into a failure rather than a skip. maxmind/MaxMind-DB-Reader-php-ext runs this same script over the objects it publishes, holding itself to a lower glibc floor because it builds in a digest-pinned bookworm container; a single implementation is the only way that bar cannot drift between the two. Co-Authored-By: Claude Opus 5 (1M context) --- dev-bin/gate-extension.sh | 209 +++++++++++++++++++++++++++++++++ dev-bin/test-gate-extension.sh | 37 ++++++ 2 files changed, 246 insertions(+) create mode 100755 dev-bin/gate-extension.sh create mode 100755 dev-bin/test-gate-extension.sh diff --git a/dev-bin/gate-extension.sh b/dev-bin/gate-extension.sh new file mode 100755 index 00000000..fd255fb9 --- /dev/null +++ b/dev-bin/gate-extension.sh @@ -0,0 +1,209 @@ +#!/usr/bin/env bash +# +# Refuse to ship an extension object that is not self-contained. +# +# Shared with maxmind/MaxMind-DB-Reader-php-ext, which reaches this file +# through its submodule checkout and gates the objects it publishes with it. +# One implementation is the only way the bar cannot drift between the two. +# +# Every tool runs in its own assignment and the greps read captured output, +# never a live pipe: `set -e` is suspended inside an `if` condition, so +# `if some-tool "$so" | grep ...` cannot tell "no match" from "not installed". +# For the same reason an unmeasurable result -- an empty NEEDED list, glibc +# floor or minos -- is fatal rather than a pass. +# +# Usage: gate-extension.sh +# Reads MAX_GLIBC (Linux) or MACOSX_DEPLOYMENT_TARGET (macOS); the two callers +# set different values, so the limit belongs at the call site. + +set -euo pipefail + +fail() { + echo "::error::$*" + exit 1 +} + +# Before "$1" is dereferenced: under `set -u` a bare `so="$1"` would abort with +# bash's own message instead of this one. +[ $# -eq 1 ] || fail "Usage: gate-extension.sh " + +so="$1" + +# True when $2 is no higher a version than $1. Both operands are padded to the +# same component count first, so `at_most 11 11.0` is not read as greater. +at_most() { + local a="$1" b="$2" + # An empty operand would compare as "no higher" and waive the check. + [ -n "$a" ] || fail "at_most called with an empty ceiling." + [ -n "$b" ] || fail "at_most called with an empty measurement." + while [ "$(awk -F. '{print NF}' <<<"$a")" -lt "$(awk -F. '{print NF}' <<<"$b")" ]; do a="$a.0"; done + while [ "$(awk -F. '{print NF}' <<<"$b")" -lt "$(awk -F. '{print NF}' <<<"$a")" ]; do b="$b.0"; done + [ "$(printf '%s\n%s\n' "$a" "$b" | sort -V | tail -n1)" = "$a" ] +} + +[ -f "$so" ] || fail "$so does not exist or is not a regular file." + +# Checks 3, 3b and 3c. They differ between platforms only in the nm invocation +# and Mach-O's leading underscore, so the caller sets $undefined and $defined +# and passes the prefix. Keeping one copy is not cosmetic: the two have already +# drifted once, when only the Linux greps were anchored. +# +# 3b exists because vendoring makes libmaxminddb's API part of this object's +# exports, and PHP dlopens extensions with RTLD_GLOBAL, so a process that also +# loads a system libmaxminddb could bind across the two. config.m4 passes +# -fvisibility=hidden to prevent it; this asserts that worked. MSVC exports +# nothing unmarked, so config.w32 needs no equivalent. +# +# 3c is its counterweight: -fvisibility=hidden covers our own maxminddb.c too, +# and get_module survives only because ZEND_GET_MODULE expands through +# ZEND_DLEXPORT, which carries visibility("default"). +check_symbols() { + local prefix="$1" exported + + if grep -E "(^|[[:space:]])${prefix}MMDB_" <<<"$undefined"; then + fail "Undefined MMDB_ symbols remain." + fi + + exported="$(grep -E "(^|[[:space:]])${prefix}MMDB_" <<<"$defined" || true)" + if [ -n "$exported" ]; then + printf '%s\n' "$exported" + fail "The object exports libmaxminddb's MMDB_ symbols; they should be hidden." + fi + + if ! grep -qE "(^|[[:space:]])${prefix}get_module$" <<<"$defined"; then + fail "The object does not export get_module; PHP will reject it as not a PHP library." + fi +} + + +# Informational only; file(1) ships separately from binutils. +file "$so" || echo "file(1) is unavailable; skipping the object summary." + +case "$(uname -s)" in +Linux) + dynamic="$(readelf -d "$so")" + + # 1. No libmaxminddb dependency -- the point of the bundled build is that + # users need nothing but libc. The parenthesis is optional because GNU + # readelf prints `(NEEDED)` and llvm-readelf a bare `NEEDED`. + needed="$(sed -n 's/.*(\{0,1\}NEEDED)\{0,1\}.*\[\(.*\)\]/\1/p' <<<"$dynamic")" + printf 'NEEDED:\n%s\n' "$needed" + # Every shared object needs at least libc, so none means the parse failed. + if [ -z "$needed" ]; then + fail "Could not read any DT_NEEDED entries from $so; the gate proved nothing." + fi + if grep -qi maxminddb <<<"$needed"; then + fail "The object still links libmaxminddb." + fi + + # 2. No RUNPATH/RPATH -- a search path baked in from the build container is + # meaningless, or worse, on a user's machine. + if grep -E '\(?(RUNPATH|RPATH)\)?' <<<"$dynamic"; then + fail "The object carries a RUNPATH/RPATH." + fi + + # 3, 3b, 3c -- the symbol checks. + undefined="$(nm -D -u "$so")" + defined="$(nm -D --defined-only "$so")" + check_symbols '' + + # 4a. Nothing but the C runtime. Check 1 rejects libmaxminddb by name; this + # makes any other new dependency a deliberate decision. musl spells its + # libc libc.musl-.so.1, and libatomic turns up for 64-bit atomics + # on some 32-bit architectures; neither is built here today. + while read -r lib; do + [ -n "$lib" ] || continue + case "$lib" in + libc.so.* | libc.musl-*.so.* | libm.so.* | libdl.so.* | librt.so.* | \ + libpthread.so.* | ld-linux*.so.* | ld-musl-*.so.* | libgcc_s.so.* | \ + libatomic.so.*) ;; + *) fail "Unexpected runtime dependency $lib; the object should need nothing but the C runtime." ;; + esac + done <<<"$needed" + + # 4. Measured glibc floor must not exceed the documented maximum. `|| true` + # covers only the grep, which legitimately exits non-zero on no match. + syms="$(objdump -T "$so")" + # Unstable at any version, so an absolute bar rather than part of the floor. + if grep -qE 'GLIBC_PRIVATE' <<<"$syms"; then + fail "The object references GLIBC_PRIVATE, which is not a stable interface." + fi + floor="$(grep -oE 'GLIBC_[0-9]+(\.[0-9]+)+' <<<"$syms" | sed 's/^GLIBC_//' | sort -uV | tail -n1 || true)" + if [ -z "$floor" ]; then + fail "Could not measure a glibc floor for $so; the gate proved nothing." + fi + # GLIBC_ABI_DT_RELR carries no version, so the pattern above cannot see it, + # yet it needs glibc >= 2.36. Raise the floor rather than reject: requiring + # 2.36 is only wrong against a lower ceiling. Ordered after the empty-floor + # guard so it cannot paper over a failed measurement. + if grep -qE 'GLIBC_ABI_DT_RELR' <<<"$syms"; then + echo "The object requires GLIBC_ABI_DT_RELR, so its floor is at least 2.36." + floor="$(printf '%s\n2.36\n' "$floor" | sort -uV | tail -n1)" + fi + echo "Measured glibc floor: $floor (documented maximum $MAX_GLIBC)" + if ! at_most "$MAX_GLIBC" "$floor"; then + fail "Requires glibc $floor, above the documented maximum $MAX_GLIBC." + fi + ;; +Darwin) + # Reached only from the extension repository's macOS lane; the + # bundled-build workflow here is Linux-only. + linked="$(otool -L "$so")" + loadcmds="$(otool -l "$so")" + undefined="$(nm -u "$so")" + + # 1. No libmaxminddb dependency. tail -n +2 drops otool's echo of the + # object's own path. + printf '%s\n' "$linked" + if tail -n +2 <<<"$linked" | grep -i maxminddb; then + fail "The object still links libmaxminddb." + fi + + # 2. No LC_RPATH: a Homebrew prefix baked in here would not exist on a + # user's machine. + if grep -A3 LC_RPATH <<<"$loadcmds"; then + fail "The object carries an LC_RPATH." + fi + + # 3, 3b, 3c -- the symbol checks, with Mach-O's leading underscore. + # `nm -gU` (external only, defined only) means the same to Apple's nm and + # to the llvm-nm now behind it. + defined="$(nm -gU "$so")" + check_symbols _ + + # 4a. Nothing but libSystem, the Mach-O counterpart of the NEEDED + # allowlist. Only indented lines whose first field is an absolute path + # are dependencies, which skips otool's echo of the object's own path + # and, on a universal binary, the architecture headers. + dylibs="$(awk '/^[[:space:]]+\// {print $1}' <<<"$linked")" + if [ -z "$dylibs" ]; then + fail "Could not read any linked dylibs from $so; the gate proved nothing." + fi + while read -r lib; do + [ -n "$lib" ] || continue + case "$lib" in + /usr/lib/libSystem.B.dylib | /usr/lib/system/*) ;; + *) fail "Unexpected runtime dependency $lib; the object should need nothing but libSystem." ;; + esac + done <<<"$dylibs" + + # 4. The macOS analogue of the glibc floor. Every slice is measured, not + # just the first: otool -l emits load commands per architecture, arm64 + # has a hard 11.0 floor, and Xcode clamps minos per architecture, so an + # x86_64 slice at 11.0 can hide an arm64 slice at 14.0. + all_minos="$(awk '/LC_BUILD_VERSION/{f=1} f && $1=="minos"{print $2; f=0}' <<<"$loadcmds")" + minos="$(sort -V <<<"$all_minos" | tail -n1)" + printf 'Measured minimum macOS per slice: %s (documented maximum %s)\n' \ + "${all_minos:-none}" "$MACOSX_DEPLOYMENT_TARGET" + if [ -z "$minos" ]; then + # A target of 10.13 or lower emits LC_VERSION_MIN_MACOSX instead. + fail "Could not read an LC_BUILD_VERSION minos from $so; the deployment target is unverified. An object targeting 10.13 or lower carries LC_VERSION_MIN_MACOSX instead." + fi + if ! at_most "$MACOSX_DEPLOYMENT_TARGET" "$minos"; then + fail "Requires macOS $minos, above the documented maximum $MACOSX_DEPLOYMENT_TARGET." + fi + ;; +*) + fail "No gate implemented for $(uname -s)." + ;; +esac diff --git a/dev-bin/test-gate-extension.sh b/dev-bin/test-gate-extension.sh new file mode 100755 index 00000000..ad297dc6 --- /dev/null +++ b/dev-bin/test-gate-extension.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +# +# Assert that gate-extension.sh rejects what it should. +# +# Every other caller runs the gate over a good object, so it is only ever +# observed succeeding. An inverted grep, a dropped `!`, or a pattern written in +# the wrong regex dialect would leave both this repository and +# maxmind/MaxMind-DB-Reader-php-ext green while the gate certified anything -- +# not hypothetical, since the ERE/BRE distinction has already bitten that way. +# +# These are the cheap cases: a ceiling below the measured floor, an input that +# is not an object, a path that does not exist, and no argument at all. +# +# Usage: test-gate-extension.sh + +set -euo pipefail + +so="$1" +gate="$(dirname "${BASH_SOURCE[0]}")/gate-extension.sh" + +[ -f "$so" ] || { echo "::error::$so does not exist; nothing to test the gate against."; exit 1; } + +refute() { # + local what="$1" + shift + if "$@" >/dev/null 2>&1; then + echo "::error::the gate accepted $what" + exit 1 + fi +} + +refute "an object above its glibc ceiling" env MAX_GLIBC=2.0 "$gate" "$so" +refute "a file that is not an object" "$gate" /etc/hostname +refute "a path that does not exist" "$gate" /nonexistent/maxminddb.so +refute "a missing argument" "$gate" + +echo "the gate rejected all four." From 9952c0c36638705922b51a2f5385ad6d46ec72fa Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 4 Aug 2026 15:34:01 +0000 Subject: [PATCH 03/17] Add an opt-in bundled libmaxminddb build mode `--with-maxminddb-bundled` vendors libmaxminddb as a submodule at ext/libmaxminddb and compiles it into the extension, so the resulting object needs nothing but libc. That is a prerequisite for distributing precompiled builds: an object linking a system libmaxminddb is not much use as a prebuilt, because the user still has to install the library. The default is unchanged -- without the flag the extension links a system libmaxminddb exactly as before. libmaxminddb's maxminddb.h includes "maxminddb_config.h" unconditionally, a header its own build system generates and which we never run. ext/bundled-include supplies it as a tracked file and both build systems pass the values it would have defined on the command line, which keeps those authoritative even where a generated header is also present. It is tracked rather than written at configure time because generating it meant writing inside the submodule, which left it dirty after every build. config.w32 reads the libmaxminddb version out of the submodule's configure.ac rather than repeating it: nothing else checks that file, since there is no Windows workflow here. config.m4 keeps a literal, which test-bundled.yml asserts against configure.ac at runtime on every build. Three macro choices are load-bearing and documented where they are made. HAVE_CONFIG_H=0 works only because libmaxminddb tests its value with `#if` while our own maxminddb.c tests definedness with `#ifdef`; -UHAVE_CONFIG_H has to precede it because PHP's CPPFLAGS already define it. MMDB_UINT128_IS_BYTE_ARRAY=1 is set identically on both platforms because it decides the layout of the MMDB_entry_data_s union that maxminddb.c and the vendored sources pass between each other. MMDB_LITTLE_ENDIAN is deliberately left undefined when AC_C_BIGENDIAN cannot determine the answer or the build is universal, so maxminddb.h derives it from __BYTE_ORDER__ per architecture; a wrong value here compiles, links, loads, and returns garbage for every float and double while strings and integers stay correct. -fvisibility=hidden keeps the vendored MMDB_* API out of the object's export table. Vendoring turns those from someone else's exports into ours, and PHP dlopens extensions with RTLD_GLOBAL on common builds, so a process that also loads something linked against a system libmaxminddb could bind across the two. Windows needs no equivalent, MSVC exporting nothing unmarked. On Windows, ADD_SOURCES needs an explicit object directory: under MODE_PHPIZE confutils.js derives it from each entry's dirname, and our entries are bare filenames, so libmaxminddb/src/maxminddb.c and ext/maxminddb.c both resolved to maxminddb.obj. Whichever compiled last won, and when libmaxminddb's did the DLL linked without get_module and PHP rejected it as "Invalid library". test-bundled.yml builds the whole matrix, 9 PHP versions on two architectures, in containers with no system libmaxminddb and no PKG_CONFIG_PATH. It runs the phpt suite, the PHPUnit suite against the built object, the shared gate, and a standalone load of the object with no ini file. The PHPUnit suite matters most: MMDB_UINT128_IS_BYTE_ARRAY=1 is the path every bundled build takes and no other job in this repository compiles it, so ReaderTest's assertion of the exact string returned for 2^120 is the only thing standing between a swapped high/low word and a plausible-looking wrong answer. Its float and double cases are likewise the only exercise MMDB_LITTLE_ENDIAN gets. Every check asserts its own preconditions: a missing pkg-config, a `make test` that ran no tests, and an unreadable version are all failures rather than quiet passes. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test-bundled.yml | 193 +++++++++++++++++++++++++ .gitmodules | 3 + CHANGELOG.md | 8 + dev-bin/run-ext-tests.sh | 42 ++++++ dev-bin/verify-extension.php | 41 ++++++ ext/bundled-include/maxminddb_config.h | 17 +++ ext/config.m4 | 135 ++++++++++++++--- ext/config.w32 | 99 ++++++++++++- ext/libmaxminddb | 1 + 9 files changed, 519 insertions(+), 20 deletions(-) create mode 100644 .github/workflows/test-bundled.yml create mode 100755 dev-bin/run-ext-tests.sh create mode 100644 dev-bin/verify-extension.php create mode 100644 ext/bundled-include/maxminddb_config.h create mode 160000 ext/libmaxminddb diff --git a/.github/workflows/test-bundled.yml b/.github/workflows/test-bundled.yml new file mode 100644 index 00000000..8ba634f9 --- /dev/null +++ b/.github/workflows/test-bundled.yml @@ -0,0 +1,193 @@ +name: Bundled libmaxminddb + +on: + push: + pull_request: + schedule: + - cron: '15 4 * * SUN' + +permissions: {} + +env: + # The highest glibc symbol version a bundled build here is allowed to + # reference. This repository publishes no binaries, so nobody inherits this + # number directly -- it is a tripwire on the sources, and the floor that + # actually reaches users is the extension repository's. + # + # The floor measured here is the *build container's*, not the runner's: the + # jobs below run in shivammathur/node:latest-. That tag is unpinned, so + # this is an assertion about a moving target -- an upstream image rebuild + # onto a newer glibc will either fail this job or shift what the number + # means. Pinning the image by digest is the way to make it stable. + # + # The extension repository builds what it publishes inside a digest-pinned + # bookworm container and holds itself to 2.36. The two numbers differ on + # purpose, which is why dev-bin/gate-extension.sh takes the limit from its + # caller rather than hard-coding one. + MAX_GLIBC: '2.38' + +jobs: + bundled: + runs-on: ${{ matrix.runner }} + container: shivammathur/node:latest-${{ matrix.arch }} + strategy: + fail-fast: false + matrix: + arch: ["amd64", "arm64v8"] + php-version: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3', '8.4', '8.5'] + include: + - arch: amd64 + runner: ubuntu-latest + - arch: arm64v8 + runner: ubuntu-24.04-arm + + name: "PHP ${{ matrix.php-version }} bundled build on ${{ matrix.runner }}" + steps: + - name: Install PHP + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 + with: + php-version: ${{ matrix.php-version }} + # gmp so ReaderTest's uint128 boundary cases run rather than skip; + # they are gated on it, and uint128 is the branch this job is the + # only one to compile. + extensions: "mbstring, intl, gmp" + tools: "composer, phpize" + + - name: Checkout + # We use v1 due to https://github.com/actions/checkout/issues/334 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + submodules: true + persist-credentials: false + + # We deliberately do not install libmaxminddb, and we do not set + # PKG_CONFIG_PATH, so that the build cannot silently fall back to a + # system library. + - name: Check that no system libmaxminddb is available + run: | + # Without this, an image with no pkg-config makes the check below + # exit 127, the `if` false, and the step green having proved + # nothing. + if ! command -v pkg-config >/dev/null; then + echo "::error::pkg-config is not installed, so this precondition cannot be verified" + exit 1 + fi + if pkg-config --exists libmaxminddb; then + echo "::error::a system libmaxminddb is installed, so this job would not test the bundled sources" + exit 1 + fi + # pkg-config only sees libraries that ship a .pc file on + # PKG_CONFIG_PATH, so a `make install` into /usr/local would slip + # past it. What actually proves the bundled code is inside the + # object is gate-extension.sh's NEEDED and undefined-MMDB_ checks. + + - name: Build extension + run: | + cd ext + phpize + ./configure --with-maxminddb --with-maxminddb-bundled + make clean + make + ../dev-bin/run-ext-tests.sh + + # Shared with maxmind/MaxMind-DB-Reader-php-ext, which runs this same + # script over the objects it publishes. Keeping one implementation means + # a check added for the released binaries is a check this job performs + # too, and neither can quietly weaken relative to the other. + - name: Check that the extension is self-contained + run: dev-bin/gate-extension.sh ext/modules/maxminddb.so + + # A prebuilt extension is installed as a lone maxminddb.so, so check that + # the object loads with no ini file and no build tree beside it. Note + # that this is not a self-containment proof: every library present at + # build time is still installed on this machine. gate-extension.sh above + # is what establishes that. + # A prebuilt extension is installed as a lone maxminddb.so, so check that + # the object loads with no ini file and no build tree beside it. Note + # that this is not a self-containment proof: every library present at + # build time is still installed on this machine. gate-extension.sh above + # is what establishes that. + - name: Load the extension on its own + run: | + # autoupdate and autoconf >= 2.70 routinely rewrite AC_INIT's + # spacing, at which point this sed yields nothing and the + # comparison below would fail with an empty "expected". + version="$(sed -n 's/^AC_INIT(\[libmaxminddb\], \[\([^]]*\)\].*/\1/p' \ + ext/libmaxminddb/configure.ac)" + if [ -z "$version" ]; then + echo "::error::could not read the libmaxminddb version from ext/libmaxminddb/configure.ac" + exit 1 + fi + dir="$(mktemp -d)" + cp ext/modules/maxminddb.so "$dir/" + php -n -d extension="$dir/maxminddb.so" dev-bin/verify-extension.php \ + tests/data/test-data/GeoIP2-City-Test.mmdb "$version" + + # ext/tests is three .phpt files that decode nothing. Every behavioural + # assertion against the C code -- all twelve MMDB data types, the + # corrupt-database paths, the closed-reader paths -- lives in the PHPUnit + # suite, which test.yml runs against a system-libmaxminddb build and + # which this job did not run at all. + # + # It matters most for uint128. MMDB_UINT128_IS_BYTE_ARRAY=1 is the path + # every bundled build takes, and no other job in this repository + # exercises it: they all link a system libmaxminddb, whose own configure + # finds unsigned __int128 and compiles the other branch. ReaderTest + # asserts the exact string the extension returns for 2^120, so a swapped + # high/low word or a wrong shift is caught here and nowhere else -- it + # would otherwise produce a plausible-looking hex string and crash + # nothing. + # + # The suite also reads floats and doubles, which are the only consumers + # of MMDB_LITTLE_ENDIAN. + - name: Install dependencies + run: composer install --no-progress --prefer-dist --optimize-autoloader + + - name: Test with phpunit using the bundled extension + run: php -d extension=ext/modules/maxminddb.so vendor/bin/phpunit + + bundled-debug: + runs-on: ubuntu-latest + container: shivammathur/node:latest-amd64 + + name: "Bundled build with debug flags" + steps: + - name: Install PHP + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 + with: + php-version: '8.4' + extensions: "mbstring, intl" + tools: "composer, phpize" + + - name: Checkout + # We use v1 due to https://github.com/actions/checkout/issues/334 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + submodules: true + persist-credentials: false + + # --enable-maxminddb-debug adds -Werror, so this shows that the bundled + # libmaxminddb sources build warning free too. + - name: Build extension + run: | + cd ext + phpize + ./configure --with-maxminddb --with-maxminddb-bundled --enable-maxminddb-debug + make clean + make + ../dev-bin/run-ext-tests.sh + + # The debug build differs in flags and codegen, so it is the object most + # likely to pick up something the release build does not -- a RUNPATH + # from a different link line, say. It was the one bundled object the gate + # never saw. + - name: Check that the extension is self-contained + run: dev-bin/gate-extension.sh ext/modules/maxminddb.so + + # Nothing else exercises the gate's failure paths: every other caller + # runs it over a good object, so an inverted grep or a dropped `!` would + # leave this repository and the extension repository green for good. + # These are the cheap cases -- a ceiling below the measured floor, and an + # input that is not an object at all. + - name: Check that the gate rejects what it should + run: dev-bin/test-gate-extension.sh ext/modules/maxminddb.so diff --git a/.gitmodules b/.gitmodules index e8246baa..682d9301 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,6 @@ [submodule "tests/data"] path = tests/data url = https://github.com/maxmind/MaxMind-DB +[submodule "ext/libmaxminddb"] + path = ext/libmaxminddb + url = https://github.com/maxmind/libmaxminddb diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bc4732d..4d238096 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,14 @@ CHANGELOG Jean-Baptiste Nahan. GitHub #231. * Replaced `XtOffsetOf()` with `offsetof()`. The `XtOffsetOf()` alias has been removed in PHP 8.6. Pull request by Remi Collet. GitHub #252. +* The extension can now be built from a bundled copy of libmaxminddb, on + both Unix-like systems and Windows, by passing `--with-maxminddb-bundled` + to `configure` (or to `configure.bat` on Windows). This produces an + extension that does not depend on a system libmaxminddb, which is a + prerequisite for distributing precompiled builds; on Windows it also + replaces the 1.5.0 import library that PHP publishes for Windows builds. + The default is unchanged: without the flag, the extension links against a + system libmaxminddb as before. GitHub #265. 1.13.1 (2025-11-21) ------------------- diff --git a/dev-bin/run-ext-tests.sh b/dev-bin/run-ext-tests.sh new file mode 100755 index 00000000..91b3746b --- /dev/null +++ b/dev-bin/run-ext-tests.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +# +# Run the extension's phpt suite and assert it actually ran. +# +# The Makefile phpize generates ends its `test` target with an echo and no +# `exit 1`, so `make test` succeeds having executed nothing when there is no +# CLI sapi. run-tests.php also counts SKIPPED as a pass, and two of the three +# phpt files skip when the extension is not loaded -- so a build that produces +# an unloadable object could otherwise go green. +# +# Run from the directory holding the generated Makefile (ext/). +# +# Shared with maxmind/MaxMind-DB-Reader-php-ext, which builds the same +# extension and needs the same assertion, for the reason dev-bin/ +# gate-extension.sh gives for sharing the gate. + +set -euo pipefail + +log="$(mktemp)" +trap 'rm -f "$log"' EXIT + +NO_INTERACTION=1 make test 2>&1 | tee "$log" + +fail() { + echo "::error::$*" + exit 1 +} + +if grep -q "Cannot run tests without CLI sapi" "$log"; then + fail "make test ran no tests: no CLI sapi was found" +fi +if ! grep -qE "Tests +(passed|failed)" "$log"; then + fail "make test produced no test summary; it likely ran nothing" +fi + +# The summary prints even when every test skips, so read the count. +passed="$(sed -n 's/^Tests passed *: *\([0-9]*\).*/\1/p' "$log" | tail -n1)" +[ -n "$passed" ] || fail "could not read the passed count from the make test summary" +[ "$passed" -ge 1 ] || + fail "make test passed $passed tests; the extension probably did not load" + +echo "make test passed $passed tests." diff --git a/dev-bin/verify-extension.php b/dev-bin/verify-extension.php new file mode 100644 index 00000000..a2c7bff4 --- /dev/null +++ b/dev-bin/verify-extension.php @@ -0,0 +1,41 @@ +` so that no ini +// file can supply anything the object did not bring with it. +// +// Usage: php verify-extension.php +// +// Shared with maxmind/MaxMind-DB-Reader-php-ext, which needs the same check +// against the binaries it publishes. + +$reader = new Reader($argv[1]); +$record = $reader->get('81.2.69.160'); +$city = isset($record['city']['names']['en']) + ? $record['city']['names']['en'] : ''; + +// Asserting the value rather than merely that one came back: 81.2.69.160 is +// London in GeoIP2-City-Test.mmdb, so this is a correctness check for one +// decoded string instead of a liveness check. +if ($city !== 'London') { + fwrite(\STDERR, 'expected London, got: ' . var_export($city, true) . "\n"); + + exit(1); +} +echo "lookup returned city: {$city}\n"; + +// MMDB_lib_version() returns the PACKAGE_VERSION the build system defined, so +// this catches a submodule bump that did not update it -- and, on PHP 7.2 and +// 7.3, a define that never reached the compiler at all. +$expected = $argv[2]; +$actual = Reader::MMDB_LIB_VERSION; +if ($actual !== $expected) { + fwrite(\STDERR, "MMDB_LIB_VERSION is {$actual}, expected {$expected}\n"); + + exit(1); +} +echo "MMDB_LIB_VERSION: {$actual}\n"; diff --git a/ext/bundled-include/maxminddb_config.h b/ext/bundled-include/maxminddb_config.h new file mode 100644 index 00000000..be719637 --- /dev/null +++ b/ext/bundled-include/maxminddb_config.h @@ -0,0 +1,17 @@ +#ifndef MAXMINDDB_CONFIG_H +#define MAXMINDDB_CONFIG_H + +/* Intentionally empty. + * + * libmaxminddb's maxminddb.h includes "maxminddb_config.h" unconditionally, + * and its own build system generates that header. We do not run that build, so + * this satisfies the include while ext/config.m4 and ext/config.w32 pass the + * values it would have defined on the command line -- which keeps them + * authoritative even where a generated header is also present. + * + * It is a tracked file rather than something the two build systems write at + * configure time: generating it meant writing into the submodule on Windows, + * which left it dirty after every build. + */ + +#endif /* MAXMINDDB_CONFIG_H */ diff --git a/ext/config.m4 b/ext/config.m4 index c09151e4..51839d9b 100644 --- a/ext/config.m4 +++ b/ext/config.m4 @@ -2,32 +2,105 @@ PHP_ARG_WITH(maxminddb, [Whether to enable the MaxMind DB Reader extension], [ --with-maxminddb Enable MaxMind DB Reader extension support]) +PHP_ARG_WITH(maxminddb-bundled, + [Whether to build the bundled libmaxminddb sources into the extension], + [ --with-maxminddb-bundled Build the bundled libmaxminddb sources into the + extension instead of linking a system library], no, no) + PHP_ARG_ENABLE(maxminddb-debug, for MaxMind DB debug support, [ --enable-maxminddb-debug Enable MaxMind DB debug support], no, no) if test $PHP_MAXMINDDB != "no"; then - AC_PATH_PROG(PKG_CONFIG, pkg-config, no) + maxminddb_sources="maxminddb.c" - AC_MSG_CHECKING(for libmaxminddb) - if test -x "$PKG_CONFIG" && $PKG_CONFIG --exists libmaxminddb; then - dnl retrieve build options from pkg-config - if $PKG_CONFIG libmaxminddb --atleast-version 1.0.0; then - LIBMAXMINDDB_INC=`$PKG_CONFIG libmaxminddb --cflags` - LIBMAXMINDDB_LIB=`$PKG_CONFIG libmaxminddb --libs` - LIBMAXMINDDB_VER=`$PKG_CONFIG libmaxminddb --modversion` - AC_MSG_RESULT(found version $LIBMAXMINDDB_VER) - else - AC_MSG_ERROR(system libmaxminddb must be upgraded to version >= 1.0.0) - fi - PHP_EVAL_LIBLINE($LIBMAXMINDDB_LIB, MAXMINDDB_SHARED_LIBADD) - PHP_EVAL_INCLINE($LIBMAXMINDDB_INC) + if test "$PHP_MAXMINDDB_BUNDLED" != "no"; then + dnl The arguments are [if-big], [if-little], [if-unknown], [if-universal], + dnl and only the first two define the macro, so it is left undefined + dnl unless the answer is actually known. + dnl + dnl Guessing here is not safe. The macro is consumed only by the + dnl byte-swaps in get_ieee754_float() and get_ieee754_double(), so a + dnl wrong value builds, links and loads perfectly well and then returns + dnl garbage for every float and double -- latitude, longitude, + dnl accuracyRadius -- while strings and integers stay correct. + dnl + dnl Under a universal (multi -arch) build no single configure-time value + dnl can be right for every slice. In both cases libmaxminddb's own + dnl maxminddb.h derives it from __BYTE_ORDER__ behind + dnl `#if !defined(MMDB_LITTLE_ENDIAN)`, which is per-architecture correct + dnl and is what we want to fall through to. + dnl + dnl That fall-through relies on __BYTE_ORDER__, which every compiler + dnl able to build PHP defines, and maxminddb.h covers Windows + dnl separately. Where it is genuinely absent the macro stays undefined, + dnl `#if MMDB_LITTLE_ENDIAN` evaluates it as 0, and the build assumes + dnl big-endian silently -- correct on a big-endian target and wrong on a + dnl little-endian one. Upstream aborts instead, having no universal + dnl build to accommodate. Compiling with -Wundef surfaces exactly this + dnl case; it is not added here because PHP's own headers do not build + dnl warning-free under it, and --enable-maxminddb-debug turns warnings + dnl into errors. + dnl + dnl The last two actions must be non-empty: autoconf treats an empty + dnl argument as absent and substitutes its own default, which for + dnl action-if-unknown is to abort rather than fall through. + m4_define([_mmdb_endian_unknown], + [AC_MSG_NOTICE([endianness undetermined; letting maxminddb.h derive MMDB_LITTLE_ENDIAN from __BYTE_ORDER__])]) + AC_C_BIGENDIAN([CFLAGS="$CFLAGS -DMMDB_LITTLE_ENDIAN=0"], + [CFLAGS="$CFLAGS -DMMDB_LITTLE_ENDIAN=1"], + [_mmdb_endian_unknown], [_mmdb_endian_unknown]) + + dnl -fvisibility=hidden keeps libmaxminddb's MMDB_* API from becoming + dnl part of this object's export table. Vendoring turns those from + dnl someone else's exports into ours, and PHP dlopens extensions with + dnl RTLD_GLOBAL on common builds, so a process that also loads + dnl something linked against a system libmaxminddb could bind across + dnl the two. dev-bin/gate-extension.sh asserts the result. + dnl + dnl -UHAVE_CONFIG_H must come first: PHP's CPPFLAGS already define + dnl HAVE_CONFIG_H, and redefining it is a warning that becomes an error + dnl under --enable-maxminddb-debug, which adds -Werror. + dnl + dnl MMDB_lib_version() returns PACKAGE_VERSION, and the extension + dnl exposes that as MMDB_LIB_VERSION and through phpinfo(), so it must + dnl be kept in sync with the version of the libmaxminddb submodule. + dnl dev-bin/check-libmaxminddb-version.sh asserts that. + dnl + dnl These deliberately go to the global CFLAGS and not to + dnl PHP_NEW_EXTENSION's extra-cflags argument, which would be the + dnl tidier home for them: on PHP 7.2 and 7.3 that argument does not + dnl reach these sources, PACKAGE_VERSION is left to PHP's own empty + dnl definition, and MMDB_LIB_VERSION comes out as "". It builds and + dnl loads, so only a version assertion catches it. 7.4 and later are + dnl fine, which is what makes the breakage easy to miss. Do not move + dnl them while 7.2 and 7.3 are supported. + CFLAGS="$CFLAGS -fvisibility=hidden -UHAVE_CONFIG_H -DHAVE_CONFIG_H=0 -DMMDB_UINT128_USING_MODE=0 -DMMDB_UINT128_IS_BYTE_ARRAY=1 -DPACKAGE_VERSION='\"1.13.3\"'" + + maxminddb_sources="$maxminddb_sources libmaxminddb/src/maxminddb.c libmaxminddb/src/data-pool.c" else - AC_MSG_RESULT(pkg-config information missing) - AC_MSG_WARN(will use libmaxmxinddb from compiler default path) + AC_PATH_PROG(PKG_CONFIG, pkg-config, no) + + AC_MSG_CHECKING(for libmaxminddb) + if test -x "$PKG_CONFIG" && $PKG_CONFIG --exists libmaxminddb; then + dnl retrieve build options from pkg-config + if $PKG_CONFIG libmaxminddb --atleast-version 1.0.0; then + LIBMAXMINDDB_INC=`$PKG_CONFIG libmaxminddb --cflags` + LIBMAXMINDDB_LIB=`$PKG_CONFIG libmaxminddb --libs` + LIBMAXMINDDB_VER=`$PKG_CONFIG libmaxminddb --modversion` + AC_MSG_RESULT(found version $LIBMAXMINDDB_VER) + else + AC_MSG_ERROR(system libmaxminddb must be upgraded to version >= 1.0.0) + fi + PHP_EVAL_LIBLINE($LIBMAXMINDDB_LIB, MAXMINDDB_SHARED_LIBADD) + PHP_EVAL_INCLINE($LIBMAXMINDDB_INC) + else + AC_MSG_RESULT(pkg-config information missing) + AC_MSG_WARN(will use libmaxmxinddb from compiler default path) - PHP_CHECK_LIBRARY(maxminddb, MMDB_open) - PHP_ADD_LIBRARY(maxminddb, 1, MAXMINDDB_SHARED_LIBADD) + PHP_CHECK_LIBRARY(maxminddb, MMDB_open) + PHP_ADD_LIBRARY(maxminddb, 1, MAXMINDDB_SHARED_LIBADD) + fi fi if test $PHP_MAXMINDDB_DEBUG != "no"; then @@ -36,5 +109,29 @@ if test $PHP_MAXMINDDB != "no"; then PHP_SUBST(MAXMINDDB_SHARED_LIBADD) - PHP_NEW_EXTENSION(maxminddb, maxminddb.c, $ext_shared) + PHP_NEW_EXTENSION(maxminddb, $maxminddb_sources, $ext_shared) + + dnl These have to come after PHP_NEW_EXTENSION, which is what defines + dnl $ext_srcdir and $ext_builddir. Without the build directory, the object + dnl directory for the bundled sources is never created and they fail to + dnl compile. + if test "$PHP_MAXMINDDB_BUNDLED" != "no"; then + dnl config.w32 has had this check since it was written; config.m4 had + dnl none, so a clone without --recursive produced a stray "No such file + dnl or directory", a *successful* configure -- autoconf-generated + dnl configure does not run under set -e -- and then an opaque failure + dnl much later at maxminddb.h. + if test ! -f "$ext_srcdir/libmaxminddb/src/maxminddb.c"; then + AC_MSG_ERROR([--with-maxminddb-bundled needs the bundled libmaxminddb sources; run "git submodule update --init"]) + fi + + dnl ext/bundled-include supplies maxminddb_config.h, which + dnl libmaxminddb's maxminddb.h includes unconditionally and its own + dnl build system would generate. Nothing is included from the submodule + dnl root, so that directory is deliberately not on the path. + PHP_ADD_BUILD_DIR([$ext_builddir/libmaxminddb/src]) + PHP_ADD_INCLUDE([$ext_srcdir/bundled-include]) + PHP_ADD_INCLUDE([$ext_srcdir/libmaxminddb/include]) + PHP_ADD_INCLUDE([$ext_srcdir/libmaxminddb/src]) + fi fi diff --git a/ext/config.w32 b/ext/config.w32 index dc3e3cad..a94d32d1 100644 --- a/ext/config.w32 +++ b/ext/config.w32 @@ -1,7 +1,104 @@ ARG_WITH("maxminddb", "Enable MaxMind DB Reader extension support", "no"); +ARG_WITH("maxminddb-bundled", "Build the bundled libmaxminddb sources into the extension instead of linking a system library", "no"); if (PHP_MAXMINDDB == "yes") { - if (CHECK_HEADER_ADD_INCLUDE("maxminddb.h", "CFLAGS_MAXMINDDB", PHP_MAXMINDDB + ";" + PHP_PHP_BUILD + "\\include\\maxminddb") && + if (PHP_MAXMINDDB_BUNDLED != "no") { + var maxminddb_bundled = configure_module_dirname + "\\libmaxminddb"; + + if (!FSO.FolderExists(maxminddb_bundled + "\\src")) { + ERROR("--with-maxminddb-bundled needs the bundled libmaxminddb sources; run \"git submodule update --init\""); + } + + /* Read the version out of the submodule rather than repeating it here. + * Nothing else checks this file -- there is no Windows workflow in this + * repository -- so a bump that updated config.m4 and forgot this line + * would ship a PACKAGE_VERSION that lies, with nothing to notice it. + * config.m4 keeps a literal because test-bundled.yml compares + * MMDB_LIB_VERSION against configure.ac on every build. */ + var maxminddb_ac = file_get_contents(maxminddb_bundled + "\\configure.ac"); + if (!maxminddb_ac || !maxminddb_ac.match(/AC_INIT\(\[libmaxminddb\], \[([^\]]+)\]/)) { + ERROR("could not read the libmaxminddb version from ext/libmaxminddb/configure.ac"); + } + var maxminddb_version = RegExp.$1; + + /* ext/bundled-include supplies maxminddb_config.h, which + * libmaxminddb's maxminddb.h includes unconditionally and its own build + * system would generate. It is a tracked file: writing it at configure + * time meant writing into the submodule, which left it dirty after + * every build. Nothing is included from the submodule root, so that + * directory is deliberately not on the path. */ + ADD_FLAG("CFLAGS_MAXMINDDB", '/I "' + configure_module_dirname + '\\bundled-include" ' + + '/I "' + maxminddb_bundled + '\\include" ' + + '/I "' + maxminddb_bundled + '\\src"'); + + /* These match ext/config.m4, except that HAVE_CONFIG_H is left alone. + * config.m4 has to define it as 0 because PHP's Unix CPPFLAGS already + * define it, and libmaxminddb's maxminddb.c tests the *value* with + * `#if HAVE_CONFIG_H`, so it would otherwise include the config.h it + * finds on the include path -- PHP's. Our own maxminddb.c tests + * *definedness* with `#ifdef`, which is why =0 suppresses libmaxminddb's + * include without suppressing ours. That asymmetry is the hinge of this + * design and would break if upstream ever switched to `#ifdef`. + * + * The Windows build never defines HAVE_CONFIG_H, so libmaxminddb + * already does the right thing, and defining it here would instead + * make our own maxminddb.c include a config.h that does not exist. + * + * MSVC has no unsigned __int128, so the byte array is the only option + * for uint128 values. Both this file and config.m4 have to define + * MMDB_UINT128_IS_BYTE_ARRAY the same way, because it decides the + * layout of the MMDB_entry_data_s union that maxminddb.c and the + * bundled sources pass between each other. + * + * MMDB_lib_version() returns PACKAGE_VERSION, and the extension exposes + * that as MMDB_LIB_VERSION and through phpinfo(). The inner quotes are + * escaped so that they survive the compiler's own command line parsing + * and PACKAGE_VERSION ends up a string literal. */ + ADD_FLAG("CFLAGS_MAXMINDDB", '/D MMDB_LITTLE_ENDIAN=1 /D MMDB_UINT128_USING_MODE=0 ' + + '/D MMDB_UINT128_IS_BYTE_ARRAY=1 /D PACKAGE_VERSION=\\"' + maxminddb_version + '\\"'); + + /* MaxMind's Python reader passes -wd4068 for these same sources. + * libmaxminddb guards its #pragma clang lines with __clang__, so cl + * should not warn about them, but keeping the suppression costs nothing + * and survives a bump that drops a guard. */ + ADD_FLAG("CFLAGS_MAXMINDDB", "/wd4068"); + + /* MMDB_open() calls WSAStartup() and maxminddb.h pulls in winsock2.h. + * php-src already has ws2_32.lib in $(LIBS), but sapi/cli and sapi/cgi + * ask for it explicitly too, and the dependency is ours now. */ + ADD_FLAG("LIBS_MAXMINDDB", "ws2_32.lib"); + + EXTENSION("maxminddb", "maxminddb.c"); + + /* This has to come after EXTENSION, which is what makes CFLAGS_MAXMINDDB + * reach the compiler calls that ADD_SOURCES writes. + * + * The fourth argument is the object directory, and it is not optional + * here: libmaxminddb/src/maxminddb.c and our own maxminddb.c share a + * basename. In a php-src tree ADD_SOURCES derives the object directory + * from its first argument, so the two would land in separate + * directories and nothing would collide -- that is how ext/gd gets away + * with libgd/gd.c next to gd.c. Under phpize, which is how this + * extension is built for Windows, confutils.js takes a different branch + * and ignores that argument: + * + * if (MODE_PHPIZE) { + * var build_dir = (dirname ? dirname : "")... + * + * where dirname is only the directory part of each file_list entry. Our + * entries are bare filenames, so both sources resolve to + * $(BUILD_DIR)\maxminddb.obj, the makefile gets two rules for one + * target, and whichever compiles last wins. When libmaxminddb's won, + * the DLL linked without our get_module() and PHP rejected it with + * "Invalid library (maybe not a PHP library)" -- a build that succeeds + * and produces an extension that cannot load. + * + * Passing the object directory explicitly takes the branch that honours + * it, so these objects land in $(BUILD_DIR)\libmaxminddb. ADD_SOURCES + * registers that in build_dirs and configure creates it. */ + ADD_SOURCES(maxminddb_bundled + "\\src", "maxminddb.c data-pool.c", + "maxminddb", "libmaxminddb"); + } else if (CHECK_HEADER_ADD_INCLUDE("maxminddb.h", "CFLAGS_MAXMINDDB", PHP_MAXMINDDB + ";" + PHP_PHP_BUILD + "\\include\\maxminddb") && CHECK_LIB("libmaxminddb.lib;maxminddb.lib", "maxminddb", PHP_MAXMINDDB)) { EXTENSION("maxminddb", "maxminddb.c"); } else { diff --git a/ext/libmaxminddb b/ext/libmaxminddb new file mode 160000 index 00000000..09a0540f --- /dev/null +++ b/ext/libmaxminddb @@ -0,0 +1 @@ +Subproject commit 09a0540fea89a16e5c6a9e21e93ee9aece6639e3 From b10e47cdb426d67f6afa476dd45e726ae10e3454 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 4 Aug 2026 15:34:14 +0000 Subject: [PATCH 04/17] Enable Dependabot for git submodules, ignoring libmaxminddb The schedule and cooldown match the existing entries. ext/libmaxminddb is excluded. It is pinned to a release tag and compiled into the extension, and the gitsubmodule ecosystem advances a submodule to the tip of its tracked branch, which would take us off release tags. Bumping it is a deliberate action: the version has to be updated in ext/config.m4 and ext/config.w32 at the same time, which dev-bin/check-libmaxminddb-version.sh asserts. Co-Authored-By: Claude Opus 5 (1M context) --- .github/dependabot.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 4acb74a4..284e6ea5 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -21,3 +21,17 @@ updates: codeql: patterns: - github/codeql-action* + - package-ecosystem: gitsubmodule + directory: / + schedule: + interval: weekly + day: monday + time: '14:00' + cooldown: + default-days: 7 + ignore: + # libmaxminddb is pinned to release tags and compiled into the + # extension. Dependabot moves submodules to the latest commit on the + # tracked branch when no newer tag exists, which would take us off + # release tags. + - dependency-name: ext/libmaxminddb From 972858c73f2f32d49b81e0d870aef3c747b9466b Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 5 Aug 2026 18:56:06 +0000 Subject: [PATCH 05/17] Define PACKAGE_VERSION in the bundled header, not on the command line `-DPACKAGE_VERSION` on the global CFLAGS also reaches ext/maxminddb.c, which includes php.h. PHP only began stripping PACKAGE_* from its generated headers in 7.4, so on 7.2 and 7.3 this redefined PHP's own macro. It is a warning today -- visible in this branch's own green CI, php_config.h:2259 on 7.2 and :2278 on 7.3, absent on 8.4 -- but --enable-maxminddb-debug adds -Werror, and the bundled-debug job pins 8.4, so `--with-maxminddb-bundled --enable-maxminddb-debug` on 7.2 or 7.3 fails to compile with nothing in CI covering it. The define moves into ext/bundled-include/maxminddb_config.h behind `#ifndef PACKAGE_VERSION`. That header is what maxminddb.h includes unconditionally, so it reaches libmaxminddb's sources, which include no PHP headers and are where MMDB_lib_version() is compiled. ext/maxminddb.c includes php.h before maxminddb.h, so the guard finds the macro already defined and skips -- and that file never reads PACKAGE_VERSION anyway, it calls MMDB_lib_version(). config.w32 keeps passing its own /D, read out of the submodule's configure.ac, which still wins over the header default via the command line. Verified by compiling a translation unit that defines PACKAGE_VERSION before including maxminddb.h, as php.h does: with the old -D and -Werror it fails with "'PACKAGE_VERSION' redefined"; with the header it compiles clean. A standalone MMDB_lib_version() still returns 1.13.3 with no -D present. This also removes the reason config.m4 could not use PHP_NEW_EXTENSION's extra-cflags argument, and the stale reference to a version-check script that no longer exists. Co-Authored-By: Claude Opus 5 (1M context) --- ext/bundled-include/maxminddb_config.h | 38 ++++++++++++++++++++------ ext/config.m4 | 20 ++++---------- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/ext/bundled-include/maxminddb_config.h b/ext/bundled-include/maxminddb_config.h index be719637..c578a4eb 100644 --- a/ext/bundled-include/maxminddb_config.h +++ b/ext/bundled-include/maxminddb_config.h @@ -1,17 +1,37 @@ #ifndef MAXMINDDB_CONFIG_H #define MAXMINDDB_CONFIG_H -/* Intentionally empty. +/* Supplies the header libmaxminddb's maxminddb.h includes unconditionally and + * that libmaxminddb's own build system would generate. We never run that + * build. * - * libmaxminddb's maxminddb.h includes "maxminddb_config.h" unconditionally, - * and its own build system generates that header. We do not run that build, so - * this satisfies the include while ext/config.m4 and ext/config.w32 pass the - * values it would have defined on the command line -- which keeps them - * authoritative even where a generated header is also present. + * This is the canonical account of the arrangement; ext/config.m4 and + * ext/config.w32 point here rather than repeating it. * - * It is a tracked file rather than something the two build systems write at - * configure time: generating it meant writing into the submodule on Windows, - * which left it dirty after every build. + * It is a tracked file rather than something the build systems write at + * configure time: generating it meant writing inside the submodule on Windows, + * which left it dirty after every build. Nothing includes anything from the + * submodule root, so that directory is deliberately off the include path. + * + * PACKAGE_VERSION lives here rather than on the command line. It has to reach + * libmaxminddb's sources, which is what MMDB_lib_version() returns, but our own + * ext/maxminddb.c also includes php.h -- and PHP only began stripping PACKAGE_* + * from its generated headers in 7.4, so a -D on the global CFLAGS redefines + * PHP's macro on 7.2 and 7.3. That is a warning today and a build failure under + * --enable-maxminddb-debug, which adds -Werror. + * + * The #ifndef makes this a no-op wherever PACKAGE_VERSION is already defined: + * ext/maxminddb.c includes php.h before maxminddb.h, so it keeps PHP's value + * and never sees a redefinition -- harmless, because it reads MMDB_lib_version() + * rather than the macro. config.w32 passes its own /D, read out of the + * submodule's configure.ac, which likewise wins over this default. + * + * Keep the version in step with the submodule. test-bundled.yml's "Load the + * extension on its own" step compares MMDB_LIB_VERSION against configure.ac on + * every build, so a stale value here fails CI rather than shipping. */ +#ifndef PACKAGE_VERSION +#define PACKAGE_VERSION "1.13.3" +#endif #endif /* MAXMINDDB_CONFIG_H */ diff --git a/ext/config.m4 b/ext/config.m4 index 51839d9b..40032f5f 100644 --- a/ext/config.m4 +++ b/ext/config.m4 @@ -62,20 +62,12 @@ if test $PHP_MAXMINDDB != "no"; then dnl HAVE_CONFIG_H, and redefining it is a warning that becomes an error dnl under --enable-maxminddb-debug, which adds -Werror. dnl - dnl MMDB_lib_version() returns PACKAGE_VERSION, and the extension - dnl exposes that as MMDB_LIB_VERSION and through phpinfo(), so it must - dnl be kept in sync with the version of the libmaxminddb submodule. - dnl dev-bin/check-libmaxminddb-version.sh asserts that. - dnl - dnl These deliberately go to the global CFLAGS and not to - dnl PHP_NEW_EXTENSION's extra-cflags argument, which would be the - dnl tidier home for them: on PHP 7.2 and 7.3 that argument does not - dnl reach these sources, PACKAGE_VERSION is left to PHP's own empty - dnl definition, and MMDB_LIB_VERSION comes out as "". It builds and - dnl loads, so only a version assertion catches it. 7.4 and later are - dnl fine, which is what makes the breakage easy to miss. Do not move - dnl them while 7.2 and 7.3 are supported. - CFLAGS="$CFLAGS -fvisibility=hidden -UHAVE_CONFIG_H -DHAVE_CONFIG_H=0 -DMMDB_UINT128_USING_MODE=0 -DMMDB_UINT128_IS_BYTE_ARRAY=1 -DPACKAGE_VERSION='\"1.13.3\"'" + dnl PACKAGE_VERSION is deliberately absent: it is defined in + dnl ext/bundled-include/maxminddb_config.h instead, because a -D here + dnl also reaches ext/maxminddb.c, which includes php.h -- and PHP only + dnl began stripping PACKAGE_* from its generated headers in 7.4, so on + dnl 7.2 and 7.3 this redefined PHP's own macro. See that header. + CFLAGS="$CFLAGS -fvisibility=hidden -UHAVE_CONFIG_H -DHAVE_CONFIG_H=0 -DMMDB_UINT128_USING_MODE=0 -DMMDB_UINT128_IS_BYTE_ARRAY=1" maxminddb_sources="$maxminddb_sources libmaxminddb/src/maxminddb.c libmaxminddb/src/data-pool.c" else From f0690de6d426af0fa04d19f8ee1e1905cb0df841 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 5 Aug 2026 18:58:30 +0000 Subject: [PATCH 06/17] Require the phpt suite to pass, not merely to run The summary grep was satisfied by either "Tests passed" or "Tests failed", and the only numeric assertion was passed >= 1. A run where 001 and 002 pass and 003-open-basedir.phpt fails therefore reported "make test passed 2 tests." and exited 0. That is reachable on exactly the versions this script exists for: the header already explains that phpize's `test` target does not propagate run-tests.php's status on 7.2 and 7.3, and both are in the matrix. 003 is also the file this branch just designated the canary for an extension that builds but cannot load. Now reads the failed and warned counts too and requires both to be zero, with an unreadable count treated as a parse failure rather than a zero. Verified against synthetic summaries: 3/0/0 passes; 2 passed with 1 failed now fails; 0 passed with 3 skipped still fails; 1 warned fails. Co-Authored-By: Claude Opus 5 (1M context) --- dev-bin/run-ext-tests.sh | 41 ++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/dev-bin/run-ext-tests.sh b/dev-bin/run-ext-tests.sh index 91b3746b..0b10427d 100755 --- a/dev-bin/run-ext-tests.sh +++ b/dev-bin/run-ext-tests.sh @@ -1,12 +1,13 @@ #!/usr/bin/env bash # -# Run the extension's phpt suite and assert it actually ran. +# Run the extension's phpt suite and assert it ran and passed. # -# The Makefile phpize generates ends its `test` target with an echo and no -# `exit 1`, so `make test` succeeds having executed nothing when there is no -# CLI sapi. run-tests.php also counts SKIPPED as a pass, and two of the three -# phpt files skip when the extension is not loaded -- so a build that produces -# an unloadable object could otherwise go green. +# `make test` cannot be trusted on its own here. The Makefile phpize generates +# ends its `test` target with an echo and no `exit 1`, so it succeeds having +# executed nothing when there is no CLI sapi; on 7.2 and 7.3 it does not +# propagate run-tests.php's status at all. run-tests.php in turn counts SKIPPED +# as a pass, and the phpt files that check the extension skip when it is not +# loaded -- so a build producing an unloadable object could otherwise go green. # # Run from the directory holding the generated Makefile (ext/). # @@ -16,15 +17,19 @@ set -euo pipefail +fail() { + echo "::error::$*" + exit 1 +} + log="$(mktemp)" trap 'rm -f "$log"' EXIT NO_INTERACTION=1 make test 2>&1 | tee "$log" -fail() { - echo "::error::$*" - exit 1 -} +# Reads one count out of run-tests.php's summary block. Empty means the line is +# missing, which is a parse failure rather than a zero. +count() { sed -n "s/^Tests $1 *: *\([0-9]*\).*/\1/p" "$log" | tail -n1; } if grep -q "Cannot run tests without CLI sapi" "$log"; then fail "make test ran no tests: no CLI sapi was found" @@ -33,10 +38,18 @@ if ! grep -qE "Tests +(passed|failed)" "$log"; then fail "make test produced no test summary; it likely ran nothing" fi -# The summary prints even when every test skips, so read the count. -passed="$(sed -n 's/^Tests passed *: *\([0-9]*\).*/\1/p' "$log" | tail -n1)" -[ -n "$passed" ] || fail "could not read the passed count from the make test summary" +passed="$(count passed)" +failed="$(count failed)" +warned="$(count warned)" +[ -n "$passed" ] && [ -n "$failed" ] && [ -n "$warned" ] || + fail "could not read the pass/fail/warn counts from the make test summary" + +# Failures are what matters, and `make test` will not report them for us: the +# summary prints regardless, and the exit status is unreliable on 7.2 and 7.3. +[ "$failed" -eq 0 ] || fail "make test had $failed failing test(s)." +[ "$warned" -eq 0 ] || fail "make test had $warned warned test(s)." +# And a run where everything skipped is not a pass. [ "$passed" -ge 1 ] || fail "make test passed $passed tests; the extension probably did not load" -echo "make test passed $passed tests." +echo "make test passed $passed tests, $failed failed, $warned warned." From 58261920a234094a06d75d0a59fb38fa55f08a0a Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 5 Aug 2026 18:58:30 +0000 Subject: [PATCH 07/17] Make the gate self-test able to fail `refute()` inspected only the exit status and discarded the output, so any non-zero exit counted as a correct rejection -- including 126 and 127. With no gate-extension.sh present at all the suite printed "the gate rejected all four." and exited 0. A gate broken into refusing everything passed just as quietly as one working correctly, which is the opposite of the failure the file was written to catch and exactly as invisible. Three changes. A positive control runs first and requires the gate to accept a known-good object, which is what a suite of rejection cases cannot establish on its own. Every refutation now matches the specific `::error::` message the gate should print, so "the gate never ran" and "the gate refused for the wrong reason" are both distinguishable from "the gate refused". And the gate's presence and executability are checked up front rather than inferred. The case list also grows from four to eight. Three of the original four died at argument or file checks before reaching a single grep, so nothing observed the checks that matter: exported MMDB_ symbols -- the assertion that -fvisibility=hidden worked, which is the whole RTLD_GLOBAL argument -- a missing get_module, a RUNPATH, or an unexpected runtime dependency. Those now have throwaway gcc fixtures, including a locally built shared library to depend on so the test needs no development packages. Verified against four broken gates: missing, always-succeeds, always-fails, and rejects-with-the-wrong-message. The old version caught only the second. Co-Authored-By: Claude Opus 5 (1M context) --- dev-bin/test-gate-extension.sh | 106 ++++++++++++++++++++++++++------- 1 file changed, 84 insertions(+), 22 deletions(-) diff --git a/dev-bin/test-gate-extension.sh b/dev-bin/test-gate-extension.sh index ad297dc6..2220b12c 100755 --- a/dev-bin/test-gate-extension.sh +++ b/dev-bin/test-gate-extension.sh @@ -1,37 +1,99 @@ #!/usr/bin/env bash # -# Assert that gate-extension.sh rejects what it should. +# Assert that gate-extension.sh accepts a good object and rejects bad ones. # -# Every other caller runs the gate over a good object, so it is only ever -# observed succeeding. An inverted grep, a dropped `!`, or a pattern written in -# the wrong regex dialect would leave both this repository and -# maxmind/MaxMind-DB-Reader-php-ext green while the gate certified anything -- -# not hypothetical, since the ERE/BRE distinction has already bitten that way. +# Every other caller runs the gate over an object it expects to pass, so the +# gate is only ever observed succeeding. Nothing would notice it breaking into +# certifying anything -- or, just as quietly, into refusing everything, which +# a suite of rejection cases alone cannot tell apart from working correctly. # -# These are the cheap cases: a ceiling below the measured floor, an input that -# is not an object, a path that does not exist, and no argument at all. +# So: a positive control first, and every rejection matched on the message the +# gate is supposed to print rather than on a non-zero exit, since 126 and 127 +# are non-zero too and "the gate never ran" must not read as "the gate said no". # # Usage: test-gate-extension.sh set -euo pipefail +fail() { + echo "::error::$*" + exit 1 +} + +[ $# -eq 1 ] || fail "Usage: test-gate-extension.sh " + so="$1" -gate="$(dirname "${BASH_SOURCE[0]}")/gate-extension.sh" +[ -f "$so" ] || fail "$so does not exist; nothing to test the gate against." + +gate="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/gate-extension.sh" +[ -x "$gate" ] || fail "$gate is missing or not executable." + +command -v gcc >/dev/null || fail "gcc is required to build the fixtures this test refutes against." -[ -f "$so" ] || { echo "::error::$so does not exist; nothing to test the gate against."; exit 1; } +# The positive control. Without it, a gate broken into always failing satisfies +# every case below. +"$gate" "$so" >/dev/null || fail "the gate rejected the known-good object $so." -refute() { # - local what="$1" - shift - if "$@" >/dev/null 2>&1; then - echo "::error::the gate accepted $what" - exit 1 - fi +refute() { # + local want="$1" what="$2" out status + shift 2 + set +e + out="$("$@" 2>&1)" + status=$? + set -e + [ "$status" -ne 0 ] || fail "the gate accepted $what" + grep -qF "$want" <<<"$out" || fail \ + "the gate rejected $what, but not for the expected reason: wanted \"$want\", got \"$(grep -m1 '::error::' <<<"$out" || head -n1 <<<"$out")\"" } -refute "an object above its glibc ceiling" env MAX_GLIBC=2.0 "$gate" "$so" -refute "a file that is not an object" "$gate" /etc/hostname -refute "a path that does not exist" "$gate" /nonexistent/maxminddb.so -refute "a missing argument" "$gate" +# Fixtures for the checks no malformed *input* can reach -- they need an object +# that builds and loads but is wrong in one specific way. Each carries a +# get_module and a libc reference so it clears the earlier checks and reaches +# the one under test. +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT + +cat > "$work/base.c" <<'C' +#include +__attribute__((visibility("default"))) void *get_module(void) { return malloc(8); } +C +build() { # + local out="$1" src="$2" + shift 2 + cat "$work/base.c" > "$work/tmp.c" + [ -z "$src" ] || printf '%s\n' "$src" >> "$work/tmp.c" + gcc -shared -fPIC -fvisibility=hidden -o "$work/$out" "$work/tmp.c" "$@" +} + +build exports.so '__attribute__((visibility("default"))) int MMDB_open(void) { return 0; }' +build nogetmodule.so '' -Wl,--version-script=/dev/null 2>/dev/null || + gcc -shared -fPIC -fvisibility=hidden -o "$work/nogetmodule.so" \ + -xc - <<<'#include +void *get_module(void) { return malloc(8); }' +build runpath.so '' -Wl,--enable-new-dtags,-rpath,/tmp +# Something outside the C runtime to depend on, built here so the test needs no +# development packages installed. +gcc -shared -fPIC -o "$work/libunexpected.so" -xc - <<<'int unexpected(void) { return 0; }' +build extradep.so 'extern int unexpected(void); int use(void) { return unexpected(); }' \ + -L"$work" -lunexpected -Wl,-rpath-link,"$work" + +refute "Usage: gate-extension.sh" "a missing argument" "$gate" +refute "does not exist or is not a regular file" "a path that does not exist" \ + "$gate" /nonexistent/maxminddb.so +refute "above the documented maximum" "an object above its glibc ceiling" \ + env MAX_GLIBC=2.0 "$gate" "$so" +refute "exports libmaxminddb's MMDB_ symbols" "an object exporting MMDB_ symbols" \ + "$gate" "$work/exports.so" +refute "does not export get_module" "an object with get_module hidden" \ + "$gate" "$work/nogetmodule.so" +refute "carries a RUNPATH" "an object with a RUNPATH" "$gate" "$work/runpath.so" +refute "Unexpected runtime dependency" "an object with a non-libc dependency" \ + "$gate" "$work/extradep.so" + +# The one case with no ::error:: to match: readelf aborts on a non-object before +# any check runs, which is the correct outcome but not one the gate announces. +if "$gate" /etc/hostname >/dev/null 2>&1; then + fail "the gate accepted a file that is not an object" +fi -echo "the gate rejected all four." +echo "the gate accepted the good object and rejected all eight bad ones." From 68f20019d9aadadac9c6511b2dfacc3ffa4b06f7 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 5 Aug 2026 18:59:28 +0000 Subject: [PATCH 08/17] Assert the extension and gmp are loaded before trusting PHPUnit The PHPUnit step passed green when the extension did not load. A failed `extension=` is a warning rather than a fatal, PHP continues, phpunit.xml.dist bootstraps vendor/autoload.php, and the PSR-4 map resolves MaxMind\Db\Reader to the pure-PHP implementation -- which the suite is written to accept, ReaderTest.php:162 selecting its expectation on extension_loaded('maxminddb'). So a dlopen failure left every assertion testing src/ instead of the object this job just built, in the one job whose comment claims MMDB_UINT128_IS_BYTE_ARRAY and MMDB_LITTLE_ENDIAN are "caught here and nowhere else". gmp is the same shape one level down: ReaderTest's decimal uint128 cases call markTestIncomplete when it is missing, which does not fail a run, so the value assertions vanish silently. Both are now explicit preconditions. The extension path also becomes absolute, matching the standalone load step above it; a relative path resolves against the cwd rather than extension_dir and was one plausible way for the load to start failing unnoticed. Two comment corrections while here: a five-line block was duplicated verbatim, and "which this job did not run at all" was contradicted by the steps thirteen lines below it. Also drops the "system-libmaxminddb" phrasing, since test.yml builds libmaxminddb from git rather than installing a distro package. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test-bundled.yml | 53 ++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/.github/workflows/test-bundled.yml b/.github/workflows/test-bundled.yml index 8ba634f9..04145dd1 100644 --- a/.github/workflows/test-bundled.yml +++ b/.github/workflows/test-bundled.yml @@ -97,11 +97,6 @@ jobs: - name: Check that the extension is self-contained run: dev-bin/gate-extension.sh ext/modules/maxminddb.so - # A prebuilt extension is installed as a lone maxminddb.so, so check that - # the object loads with no ini file and no build tree beside it. Note - # that this is not a self-containment proof: every library present at - # build time is still installed on this machine. gate-extension.sh above - # is what establishes that. # A prebuilt extension is installed as a lone maxminddb.so, so check that # the object loads with no ini file and no build tree beside it. Note # that this is not a self-containment proof: every library present at @@ -123,28 +118,52 @@ jobs: php -n -d extension="$dir/maxminddb.so" dev-bin/verify-extension.php \ tests/data/test-data/GeoIP2-City-Test.mmdb "$version" - # ext/tests is three .phpt files that decode nothing. Every behavioural - # assertion against the C code -- all twelve MMDB data types, the - # corrupt-database paths, the closed-reader paths -- lives in the PHPUnit - # suite, which test.yml runs against a system-libmaxminddb build and - # which this job did not run at all. + # The phpt files in ext/tests decode nothing. Every behavioural assertion + # against the C code -- all twelve MMDB data types, the corrupt-database + # paths, the closed-reader paths -- lives in the PHPUnit suite, which + # test.yml runs against a libmaxminddb built from git, and which this job + # did not previously run at all. Hence the steps below. # # It matters most for uint128. MMDB_UINT128_IS_BYTE_ARRAY=1 is the path # every bundled build takes, and no other job in this repository - # exercises it: they all link a system libmaxminddb, whose own configure - # finds unsigned __int128 and compiles the other branch. ReaderTest - # asserts the exact string the extension returns for 2^120, so a swapped - # high/low word or a wrong shift is caught here and nowhere else -- it - # would otherwise produce a plausible-looking hex string and crash - # nothing. + # exercises it: they all link an external libmaxminddb, whose own + # configure finds unsigned __int128 and compiles the other branch. + # ReaderTest asserts the exact string the extension returns for 2^120, so + # a swapped high/low word or a wrong shift is caught here and nowhere + # else -- it would otherwise produce a plausible-looking hex string and + # crash nothing. # # The suite also reads floats and doubles, which are the only consumers # of MMDB_LITTLE_ENDIAN. - name: Install dependencies run: composer install --no-progress --prefer-dist --optimize-autoloader + # Both preconditions are load-bearing, and neither failure is loud on its + # own. + # + # A failed `extension=` load is a warning, not a fatal, and PHP carries + # on. phpunit.xml.dist bootstraps vendor/autoload.php, whose PSR-4 map + # resolves MaxMind\Db\Reader to the pure-PHP implementation, and the + # suite is written to pass either way -- so an extension that does not + # load leaves every assertion above testing src/ rather than the object + # this job built. + # + # gmp is what turns ReaderTest's decimal uint128 case from + # markTestIncomplete into a real assertion, and markTestIncomplete does + # not fail a run. (The hex form at ReaderTest.php:162 is gated on + # extension_loaded('maxminddb'), not on gmp, so only half the uint128 + # coverage depends on this -- but it is the half that checks the value + # rather than the format.) + - name: Check the extension and gmp are actually loaded + run: | + php -d extension="$PWD/ext/modules/maxminddb.so" -r \ + 'exit(extension_loaded("maxminddb") ? 0 : 1);' || + { echo "::error::the bundled extension did not load; PHPUnit would silently test the pure-PHP reader"; exit 1; } + php -r 'exit(extension_loaded("gmp") ? 0 : 1);' || + { echo "::error::gmp is not loaded; ReaderTest's uint128 value assertions would be skipped, not run"; exit 1; } + - name: Test with phpunit using the bundled extension - run: php -d extension=ext/modules/maxminddb.so vendor/bin/phpunit + run: php -d extension="$PWD/ext/modules/maxminddb.so" vendor/bin/phpunit bundled-debug: runs-on: ubuntu-latest From 66ad3cf731a65876a196a95c7bbbdd4b6311f8c3 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 5 Aug 2026 19:00:26 +0000 Subject: [PATCH 09/17] Stop the glibc floor swallowing a broken pipeline, and reject old-style slices Two holes in check 4, both of the kind the file's own header says it exists to prevent. The `|| true` was attached to the whole pipeline while the comment claimed it covered only the grep. With pipefail set, a sed, sort or tail failure was swallowed too. The `[ -z "$floor" ]` guard catches a total failure, but a partial one yields a floor lower than the object really requires, which then passes the ceiling check. Demonstrated with a missing sed: the old shape reaches the comparison with an empty floor and exits 0, the new one aborts. Scoping it to the grep keeps the no-match case it was added for. The macOS deployment target had the mirror-image problem. The awk only sees LC_BUILD_VERSION, and the empty-result guard fires only when every slice lacks one -- so a universal object with one slice targeting 10.13 or lower, which emits LC_VERSION_MIN_MACOSX instead, leaves that slice unmeasured while the modern slices keep all_minos non-empty. That is precisely the case the error text below it was written to describe. Now rejected outright. The nine-case gate battery is unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- dev-bin/gate-extension.sh | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/dev-bin/gate-extension.sh b/dev-bin/gate-extension.sh index fd255fb9..98491c72 100755 --- a/dev-bin/gate-extension.sh +++ b/dev-bin/gate-extension.sh @@ -121,14 +121,17 @@ Linux) esac done <<<"$needed" - # 4. Measured glibc floor must not exceed the documented maximum. `|| true` - # covers only the grep, which legitimately exits non-zero on no match. + # 4. Measured glibc floor must not exceed the documented maximum. The + # `|| true` is scoped to the grep, which legitimately exits non-zero on + # no match; wrapping the whole pipeline would swallow a sed, sort or + # tail failure too, and a partial result yields a floor lower than the + # real one, which passes the ceiling check. syms="$(objdump -T "$so")" # Unstable at any version, so an absolute bar rather than part of the floor. if grep -qE 'GLIBC_PRIVATE' <<<"$syms"; then fail "The object references GLIBC_PRIVATE, which is not a stable interface." fi - floor="$(grep -oE 'GLIBC_[0-9]+(\.[0-9]+)+' <<<"$syms" | sed 's/^GLIBC_//' | sort -uV | tail -n1 || true)" + floor="$({ grep -oE 'GLIBC_[0-9]+(\.[0-9]+)+' <<<"$syms" || true; } | sed 's/^GLIBC_//' | sort -uV | tail -n1)" if [ -z "$floor" ]; then fail "Could not measure a glibc floor for $so; the gate proved nothing." fi @@ -191,6 +194,14 @@ Darwin) # just the first: otool -l emits load commands per architecture, arm64 # has a hard 11.0 floor, and Xcode clamps minos per architecture, so an # x86_64 slice at 11.0 can hide an arm64 slice at 14.0. + # A slice targeting 10.13 or lower carries LC_VERSION_MIN_MACOSX instead, + # which the awk below cannot see. On a universal object mixing the two the + # empty-result guard never fires -- all_minos is non-empty from the modern + # slices -- and the old-style one is silently unmeasured, so reject it + # outright rather than measuring around it. + if grep -q LC_VERSION_MIN_MACOSX <<<"$loadcmds"; then + fail "$so carries LC_VERSION_MIN_MACOSX; its deployment target cannot be measured here." + fi all_minos="$(awk '/LC_BUILD_VERSION/{f=1} f && $1=="minos"{print $2; f=0}' <<<"$loadcmds")" minos="$(sort -V <<<"$all_minos" | tail -n1)" printf 'Measured minimum macOS per slice: %s (documented maximum %s)\n' \ From c0146c901f3b160448a27f816f6afc7d6718bd52 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 5 Aug 2026 19:01:35 +0000 Subject: [PATCH 10/17] Validate verify-extension.php's arguments and that the extension is loaded Two vacuous paths. $argc was never checked and $expected never validated, so an empty argument compared against an empty MMDB_LIB_VERSION succeeded and the script exited 0 having proved nothing -- and "" is exactly the value a PACKAGE_VERSION that never reached the compiler produces, which is the failure this assertion exists to catch. The workflow caller guards against an empty version, but the script is shared with maxmind/MaxMind-DB-Reader-php-ext, where the caller is out of sight. It also never checked that the class came from the extension. Safe under the documented `php -n -d extension=...` invocation, since without an autoloader a missing class is fatal, but a caller who drops -n or runs it where composer's autoloader is registered would validate the pure-PHP Reader and pass. Co-Authored-By: Claude Opus 5 (1M context) --- dev-bin/verify-extension.php | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/dev-bin/verify-extension.php b/dev-bin/verify-extension.php index a2c7bff4..8692ace1 100644 --- a/dev-bin/verify-extension.php +++ b/dev-bin/verify-extension.php @@ -13,6 +13,31 @@ // Shared with maxmind/MaxMind-DB-Reader-php-ext, which needs the same check // against the binaries it publishes. +// Validated rather than assumed: this script is shared with +// maxmind/MaxMind-DB-Reader-php-ext, where the caller is out of sight. An empty +// $expected against an empty MMDB_LIB_VERSION would compare equal and exit 0 +// having proved nothing -- and "" is exactly the value a PACKAGE_VERSION that +// never reached the compiler produces. +if ($argc !== 3) { + fwrite(\STDERR, "usage: verify-extension.php \n"); + + exit(1); +} +if ($argv[2] === '') { + fwrite(\STDERR, "the expected MMDB_LIB_VERSION must not be empty\n"); + + exit(1); +} + +// Does not rely on the caller passing -n: with an autoloader in scope the +// pure-PHP Reader would satisfy everything below and prove nothing about the +// object under test. +if (!\extension_loaded('maxminddb')) { + fwrite(\STDERR, "the maxminddb extension is not loaded\n"); + + exit(1); +} + $reader = new Reader($argv[1]); $record = $reader->get('81.2.69.160'); $city = isset($record['city']['names']['en']) From 9e4acabb530dbaece1e0cd4d6944d89b838793d0 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 5 Aug 2026 19:01:35 +0000 Subject: [PATCH 11/17] Check for the bundled source file on Windows, not just its directory config.w32 tested FolderExists on libmaxminddb\src where config.m4 tests for libmaxminddb/src/maxminddb.c. An empty or half-populated leftover src directory -- an interrupted submodule checkout, say -- passes the folder test and then fails opaquely at compile time, which is the failure mode the matching check in config.m4 was added to avoid. Co-Authored-By: Claude Opus 5 (1M context) --- ext/config.w32 | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ext/config.w32 b/ext/config.w32 index a94d32d1..07e158f8 100644 --- a/ext/config.w32 +++ b/ext/config.w32 @@ -5,7 +5,10 @@ if (PHP_MAXMINDDB == "yes") { if (PHP_MAXMINDDB_BUNDLED != "no") { var maxminddb_bundled = configure_module_dirname + "\\libmaxminddb"; - if (!FSO.FolderExists(maxminddb_bundled + "\\src")) { + /* The file, not just the directory: an empty or half-populated leftover + * src\\ passes a folder check and then fails opaquely at compile time, + * which is what the matching check in config.m4 was added to avoid. */ + if (!FSO.FileExists(maxminddb_bundled + "\\src\\maxminddb.c")) { ERROR("--with-maxminddb-bundled needs the bundled libmaxminddb sources; run \"git submodule update --init\""); } From 87e380f42d99a3cdb9cbfd1eeed1d7f178445c6a Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 5 Aug 2026 19:01:35 +0000 Subject: [PATCH 12/17] Run the phpt suite through the shared script everywhere test.yml and clang-addresssanitizer.yml still called bare `NO_INTERACTION=1 make test`, so every hazard dev-bin/run-ext-tests.sh documents applied to them unaltered: the target exiting 0 having run nothing without a CLI sapi, run-tests.php counting SKIPPED as a pass, and the failure counts going unread. Those two jobs are the large majority of the matrix. The point of extracting the script was that one implementation cannot drift. Leaving the inline copies in place was the drift. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/clang-addresssanitizer.yml | 2 +- .github/workflows/test.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/clang-addresssanitizer.yml b/.github/workflows/clang-addresssanitizer.yml index 07a3797c..a6a66540 100644 --- a/.github/workflows/clang-addresssanitizer.yml +++ b/.github/workflows/clang-addresssanitizer.yml @@ -68,7 +68,7 @@ jobs: ./configure --with-maxminddb --enable-maxminddb-debug make clean make -j2 - NO_INTERACTION=1 make test + ../dev-bin/run-ext-tests.sh env: # -isystem as otherwise Clang generates warnings for includes there. # I don't think it should, but I haven't discovered a better way to diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cd0c1d23..9398b5f7 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -58,7 +58,7 @@ jobs: ./configure --with-maxminddb --enable-maxminddb-debug make clean make - NO_INTERACTION=1 make test + ../dev-bin/run-ext-tests.sh cd .. - name: Install dependencies From 5e4a6db4db1dfc6a33617a645529cbbb26f4e815 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 5 Aug 2026 19:02:39 +0000 Subject: [PATCH 13/17] Build and load the extension on Windows in CI ext/config.w32 has never had automated coverage. There is no Windows workflow in this repository, yet the CHANGELOG advertises the bundled build "on both Unix-like systems and Windows", and the parts that are unverified include two with documented failure modes that produce a DLL which builds fine and then cannot load: the ADD_SOURCES object-directory fix and the deliberate omission of HAVE_CONFIG_H. One job: build with php/php-windows-builder in local mode -- no extension-url, so it builds this checkout rather than cloning, which is the point -- then load the DLL and run dev-bin/verify-extension.php against a real database. That covers the load path and, through MMDB_LIB_VERSION, the version config.w32 scrapes out of the submodule. The explicit gate is not redundant with the phpt run, it is the only signal. 001 and 002 skip when the extension is not loaded, and 003 now skips on Windows outright, so an unloadable DLL makes every test skip and run-tests.php exits 0. The sibling repository shipped a DLL with no get_module export in exactly that shape. 003's comment is corrected to say so rather than claiming a property it no longer has on Windows. One job rather than a matrix: config.w32 does not vary by PHP version, arch or thread safety in any way this exercises, and PHP <= 7.4 needs a toolchain windows-latest no longer carries. php/setup-php-sdk was the other candidate and is archived; its toolset mapping also misclassifies the VS2026 that windows-latest now runs. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test-bundled.yml | 63 ++++++++++++++++++++++++++++++ ext/tests/003-open-basedir.phpt | 6 ++- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-bundled.yml b/.github/workflows/test-bundled.yml index 04145dd1..c4ad950d 100644 --- a/.github/workflows/test-bundled.yml +++ b/.github/workflows/test-bundled.yml @@ -210,3 +210,66 @@ jobs: # input that is not an object at all. - name: Check that the gate rejects what it should run: dev-bin/test-gate-extension.sh ext/modules/maxminddb.so + + windows-bundled: + runs-on: windows-latest + name: "Windows bundled build (x64, nts, PHP 8.4)" + steps: + # submodules: recursive because php-windows-builder does not initialise + # them in local mode -- ext/libmaxminddb is the build input, and + # tests/data holds the database the gate below queries. + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + submodules: recursive + persist-credentials: false + + # No extension-url on purpose: with none set the action looks for a + # config.w32 in the workspace and builds a copy of this checkout rather + # than cloning, which is the point -- the thing under test is this tree's + # ext/config.w32. build-directory puts the build where the gate can find + # it; the action otherwise uses a temporary directory. + # + # One job is enough. config.w32 does not vary by PHP version, arch or + # thread safety in any way this exercises, and PHP <= 7.4 needs a + # toolchain windows-latest no longer carries. + - name: Build the extension + uses: php/php-windows-builder/extension@29352c0ef9e8ce65264ea9e287881a6f7758a953 # 1.9.0 + with: + build-directory: winbuild + php-version: '8.4' + arch: x64 + ts: nts + args: --with-maxminddb --with-maxminddb-bundled + + # Load-bearing, not belt-and-braces. 001 and 002 skip when the extension + # is not loaded and 003 skips on Windows outright, so every phpt skips + # and run-tests.php exits 0 -- a DLL that builds but cannot load is + # otherwise a green job. That is not hypothetical: a DLL with no + # get_module export shipped from the sibling repository exactly this way. + # + # Uses the php.exe the build downloaded, not the runner's: version, arch, + # thread safety and toolset all have to match. Neither is on PATH in a + # later step, so both are located by search. + - name: Gate the built DLL + shell: pwsh + run: | + $dll = @(Get-ChildItem winbuild -Recurse -File -Filter php_maxminddb.dll) + $php = @(Get-ChildItem winbuild -Recurse -File -Filter php.exe | + Where-Object { $_.Directory.Name -eq 'php-bin' }) + if ($dll.Count -ne 1) { throw "expected one php_maxminddb.dll, found $($dll.Count)" } + if ($php.Count -ne 1) { throw "expected one php-bin\php.exe, found $($php.Count)" } + + $ac = Get-Content ext/libmaxminddb/configure.ac -Raw + if ($ac -notmatch 'AC_INIT\(\[libmaxminddb\], \[([^\]]+)\]') { + throw "could not read the libmaxminddb version from ext/libmaxminddb/configure.ac" + } + $version = $Matches[1] + + # verify-extension.php asserts the extension is loaded, queries a real + # database and compares MMDB_LIB_VERSION -- which also covers the + # version config.w32 scrapes out of the submodule. + & $php[0].FullName -n -d "extension=$($dll[0].FullName)" ` + dev-bin/verify-extension.php ` + tests/data/test-data/GeoIP2-City-Test.mmdb $version + if ($LASTEXITCODE -ne 0) { throw "the DLL did not load and query cleanly" } diff --git a/ext/tests/003-open-basedir.phpt b/ext/tests/003-open-basedir.phpt index b70191bc..ddd56cf8 100644 --- a/ext/tests/003-open-basedir.phpt +++ b/ext/tests/003-open-basedir.phpt @@ -3,8 +3,10 @@ openbase_dir is followed --SKIPIF-- Date: Wed, 5 Aug 2026 19:03:21 +0000 Subject: [PATCH 14/17] Document the bundled build in the README The feature's entire user-facing documentation was one CHANGELOG line, against several hundred lines of internal commentary in the build files. README's install-from-source section still showed a bare `./configure` with no mention of --with-maxminddb-bundled and no submodule checkout, which is the step someone following it would fail on first. Also adds the previously-missing --with-maxminddb to the existing recipe and says what that path links against, so the two are distinguishable. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 12ac0d5c..b6105520 100644 --- a/README.md +++ b/README.md @@ -149,18 +149,39 @@ pecl install maxminddb ### Installing Extension from Source ### -Alternatively, you may install it from the source. To do so, run the following -commands from the top-level directory of this distribution: +Alternatively, you may install it from the source. This links against the +libmaxminddb installed on your system, so install that first as described +above. Run the following commands from the top-level directory of this +distribution: ``` cd ext phpize -./configure +./configure --with-maxminddb make make test sudo make install ``` +#### Building without a system libmaxminddb #### + +Passing `--with-maxminddb-bundled` compiles a bundled copy of libmaxminddb +into the extension instead, so the result depends on nothing but the C +runtime. This is what the precompiled builds use. The bundled sources are a +git submodule, so they have to be checked out first: + +``` +git submodule update --init +cd ext +phpize +./configure --with-maxminddb --with-maxminddb-bundled +make +make test +sudo make install +``` + +The same flag works on Windows, passed to `configure.bat`. + You then must load your extension. The recommended method is to add the following to your `php.ini` file: From 79bdc97a85ca96f6d89804185487faed0281fb65 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 5 Aug 2026 19:03:22 +0000 Subject: [PATCH 15/17] Correct comments that overstate or restate The gate-rejection step in test-bundled.yml restated test-gate-extension.sh's own header and had already fallen out of sync with it in this branch -- the copy said two cases where the script refutes eight. It now points at the script. gate-extension.sh claimed the Linux and Darwin symbol checks "have already drifted once", which is not citable from anything in this branch's history. The structural reason for sharing them stands on its own. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test-bundled.yml | 6 ++---- dev-bin/gate-extension.sh | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test-bundled.yml b/.github/workflows/test-bundled.yml index c4ad950d..db2584d6 100644 --- a/.github/workflows/test-bundled.yml +++ b/.github/workflows/test-bundled.yml @@ -204,10 +204,8 @@ jobs: run: dev-bin/gate-extension.sh ext/modules/maxminddb.so # Nothing else exercises the gate's failure paths: every other caller - # runs it over a good object, so an inverted grep or a dropped `!` would - # leave this repository and the extension repository green for good. - # These are the cheap cases -- a ceiling below the measured floor, and an - # input that is not an object at all. + # runs it over an object it expects to pass. See the script's header for + # what it refutes and why. - name: Check that the gate rejects what it should run: dev-bin/test-gate-extension.sh ext/modules/maxminddb.so diff --git a/dev-bin/gate-extension.sh b/dev-bin/gate-extension.sh index 98491c72..3c2f94e9 100755 --- a/dev-bin/gate-extension.sh +++ b/dev-bin/gate-extension.sh @@ -45,8 +45,8 @@ at_most() { # Checks 3, 3b and 3c. They differ between platforms only in the nm invocation # and Mach-O's leading underscore, so the caller sets $undefined and $defined -# and passes the prefix. Keeping one copy is not cosmetic: the two have already -# drifted once, when only the Linux greps were anchored. +# and passes the prefix. One copy rather than two because the platforms differ +# in the tool invocation, not in what is being asserted. # # 3b exists because vendoring makes libmaxminddb's API part of this object's # exports, and PHP dlopens extensions with RTLD_GLOBAL, so a process that also From d1e6cb8fb4b73b6ef10ba5650cfed35b74cd4ed0 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 5 Aug 2026 19:07:31 +0000 Subject: [PATCH 16/17] Build the Windows job from a clone, and drop a stray namespace separator The action's local mode cannot be used for an extension in a subdirectory. It selects itself by finding a config.w32 anywhere in the tree, but Get-ExtensionName then reads "config.w32" relative to the repository root, so ext/config.w32 is detected and the next step fails with "Cannot find path ...\MaxMind-DB-Reader-php\config.w32". Cloning instead also gets the action's recursive submodule init, which local mode skips, and matches the configuration maxmind/MaxMind-DB-Reader-php-ext already builds this config.w32 with. Also drops the leading backslash from extension_loaded(): php-cs-fixer wants native constants qualified and native functions not. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test-bundled.yml | 20 ++++++++++++-------- dev-bin/verify-extension.php | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/.github/workflows/test-bundled.yml b/.github/workflows/test-bundled.yml index db2584d6..3ef1b027 100644 --- a/.github/workflows/test-bundled.yml +++ b/.github/workflows/test-bundled.yml @@ -213,20 +213,22 @@ jobs: runs-on: windows-latest name: "Windows bundled build (x64, nts, PHP 8.4)" steps: - # submodules: recursive because php-windows-builder does not initialise - # them in local mode -- ext/libmaxminddb is the build input, and - # tests/data holds the database the gate below queries. + # The action clones and initialises its own copy to build from; this + # checkout supplies dev-bin/verify-extension.php and the tests/data + # database the gate below queries. - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: submodules: recursive persist-credentials: false - # No extension-url on purpose: with none set the action looks for a - # config.w32 in the workspace and builds a copy of this checkout rather - # than cloning, which is the point -- the thing under test is this tree's - # ext/config.w32. build-directory puts the build where the gate can find - # it; the action otherwise uses a temporary directory. + # extension-url rather than the action's local mode, which cannot be used + # here: local mode selects itself by finding a config.w32 anywhere in the + # tree, but Get-ExtensionName then reads "config.w32" relative to the + # repository root, so an extension living in ext/ is detected and then + # immediately fails. Cloning also gets the action's own recursive + # submodule init, which local mode does not do. This is the shape + # maxmind/MaxMind-DB-Reader-php-ext already builds this config.w32 with. # # One job is enough. config.w32 does not vary by PHP version, arch or # thread safety in any way this exercises, and PHP <= 7.4 needs a @@ -234,6 +236,8 @@ jobs: - name: Build the extension uses: php/php-windows-builder/extension@29352c0ef9e8ce65264ea9e287881a6f7758a953 # 1.9.0 with: + extension-url: https://github.com/${{ github.repository }} + extension-ref: ${{ github.event.pull_request.head.sha || github.sha }} build-directory: winbuild php-version: '8.4' arch: x64 diff --git a/dev-bin/verify-extension.php b/dev-bin/verify-extension.php index 8692ace1..b496c2b4 100644 --- a/dev-bin/verify-extension.php +++ b/dev-bin/verify-extension.php @@ -32,7 +32,7 @@ // Does not rely on the caller passing -n: with an autoloader in scope the // pure-PHP Reader would satisfy everything below and prove nothing about the // object under test. -if (!\extension_loaded('maxminddb')) { +if (!extension_loaded('maxminddb')) { fwrite(\STDERR, "the maxminddb extension is not loaded\n"); exit(1); From 22297cc36e3bec72695b32701320a47d8b7b1589 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 5 Aug 2026 19:20:04 +0000 Subject: [PATCH 17/17] Correct claims in the build files and the gate that do not hold Nine small things, all cases where a comment or a list says something the code does not support. "Every shared object needs at least libc" is false -- a trivial gcc -shared object has no DT_NEEDED at all. The check still fails safe; the premise would have misled whoever debugged an unexpected "the gate proved nothing". The musl entries in the dependency allowlist were unreachable: check 5 requires a measurable GLIBC_x.y floor and treats its absence as fatal, so a musl-linked object can never get that far. Allowlisting one only looked like support. Removed, with a note saying why rather than leaving the reader to work it out. An unset MAX_GLIBC or MACOSX_DEPLOYMENT_TARGET aborted with bash's "unbound variable" instead of this script's own annotation -- fail-closed but invisible in the Actions UI, and inconsistent with the care taken over the argument check. The numbered checks also ran 1, 2, 4a, 4; the symbol checks are now named rather than sub-numbered across the function boundary they moved into, and the rest renumber in order. In config.m4: only AC_C_BIGENDIAN's third argument defaults to an abort, so the claim about "the last two" was over-broad; -UHAVE_CONFIG_H does not come first in the flag string, it precedes -DHAVE_CONFIG_H=0, which is the actual requirement; and the submodule-check comment narrated this PR's own development rather than the failure mode. --with-maxminddb-bundled without --with-maxminddb is also no longer a silent no-op. In config.w32: the version match is captured rather than read back out of RegExp.$1, which is engine-global state shared with every other config.w32 in the same scope; the "these match config.m4 except" clause now enumerates all three divergences instead of one; and the verbatim confutils.js quote, pinned to no version, becomes prose. The nine-case gate battery and the eight-case self-test are unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- dev-bin/gate-extension.sh | 48 +++++++++++++++++++++++++-------------- ext/config.m4 | 20 ++++++++++------ ext/config.w32 | 27 ++++++++++++++-------- 3 files changed, 61 insertions(+), 34 deletions(-) diff --git a/dev-bin/gate-extension.sh b/dev-bin/gate-extension.sh index 3c2f94e9..562a02a1 100755 --- a/dev-bin/gate-extension.sh +++ b/dev-bin/gate-extension.sh @@ -13,8 +13,11 @@ # floor or minos -- is fatal rather than a pass. # # Usage: gate-extension.sh -# Reads MAX_GLIBC (Linux) or MACOSX_DEPLOYMENT_TARGET (macOS); the two callers -# set different values, so the limit belongs at the call site. +# +# Reads MAX_GLIBC on Linux. The two repositories that run this set different +# values -- they build in different containers -- so the limit belongs at the +# call site. MACOSX_DEPLOYMENT_TARGET is its macOS counterpart and is set only +# by the extension repository; nothing here reaches the Darwin branch. set -euo pipefail @@ -48,13 +51,13 @@ at_most() { # and passes the prefix. One copy rather than two because the platforms differ # in the tool invocation, not in what is being asserted. # -# 3b exists because vendoring makes libmaxminddb's API part of this object's +# The exported-symbol check exists because vendoring makes libmaxminddb's API part of this object's # exports, and PHP dlopens extensions with RTLD_GLOBAL, so a process that also # loads a system libmaxminddb could bind across the two. config.m4 passes # -fvisibility=hidden to prevent it; this asserts that worked. MSVC exports # nothing unmarked, so config.w32 needs no equivalent. # -# 3c is its counterweight: -fvisibility=hidden covers our own maxminddb.c too, +# The get_module check is its counterweight: -fvisibility=hidden covers our own maxminddb.c too, # and get_module survives only because ZEND_GET_MODULE expands through # ZEND_DLEXPORT, which carries visibility("default"). check_symbols() { @@ -81,6 +84,10 @@ file "$so" || echo "file(1) is unavailable; skipping the object summary." case "$(uname -s)" in Linux) + # Checked here rather than left to `set -u`, so a caller that forgets it + # gets this script's annotation like every other failure. + [ -n "${MAX_GLIBC:-}" ] || fail "MAX_GLIBC is not set; the glibc ceiling has no value to compare against." + dynamic="$(readelf -d "$so")" # 1. No libmaxminddb dependency -- the point of the bundled build is that @@ -88,7 +95,9 @@ Linux) # readelf prints `(NEEDED)` and llvm-readelf a bare `NEEDED`. needed="$(sed -n 's/.*(\{0,1\}NEEDED)\{0,1\}.*\[\(.*\)\]/\1/p' <<<"$dynamic")" printf 'NEEDED:\n%s\n' "$needed" - # Every shared object needs at least libc, so none means the parse failed. + # An extension object always links libc, so an empty list here means the + # parse failed rather than that there are no dependencies. (A trivial + # gcc -shared object genuinely has none, but that is not what we gate.) if [ -z "$needed" ]; then fail "Could not read any DT_NEEDED entries from $so; the gate proved nothing." fi @@ -102,26 +111,29 @@ Linux) fail "The object carries a RUNPATH/RPATH." fi - # 3, 3b, 3c -- the symbol checks. + # 3. The symbol checks: no undefined MMDB_, no exported MMDB_, get_module + # still exported. See check_symbols above. undefined="$(nm -D -u "$so")" defined="$(nm -D --defined-only "$so")" check_symbols '' - # 4a. Nothing but the C runtime. Check 1 rejects libmaxminddb by name; this - # makes any other new dependency a deliberate decision. musl spells its - # libc libc.musl-.so.1, and libatomic turns up for 64-bit atomics - # on some 32-bit architectures; neither is built here today. + # 4. Nothing but the C runtime. Check 1 rejects libmaxminddb by name; this + # makes any other new dependency a deliberate decision. libatomic turns + # up for 64-bit atomics on some 32-bit architectures. + # + # No musl spellings: check 4 below requires a measurable GLIBC_x.y floor + # and treats its absence as fatal, so a musl-linked object cannot reach + # this list. Allowlisting one would only look like support. while read -r lib; do [ -n "$lib" ] || continue case "$lib" in - libc.so.* | libc.musl-*.so.* | libm.so.* | libdl.so.* | librt.so.* | \ - libpthread.so.* | ld-linux*.so.* | ld-musl-*.so.* | libgcc_s.so.* | \ - libatomic.so.*) ;; + libc.so.* | libm.so.* | libdl.so.* | librt.so.* | libpthread.so.* | \ + ld-linux*.so.* | libgcc_s.so.* | libatomic.so.*) ;; *) fail "Unexpected runtime dependency $lib; the object should need nothing but the C runtime." ;; esac done <<<"$needed" - # 4. Measured glibc floor must not exceed the documented maximum. The + # 5. Measured glibc floor must not exceed the documented maximum. The # `|| true` is scoped to the grep, which legitimately exits non-zero on # no match; wrapping the whole pipeline would swallow a sed, sort or # tail failure too, and a partial result yields a floor lower than the @@ -149,6 +161,8 @@ Linux) fi ;; Darwin) + [ -n "${MACOSX_DEPLOYMENT_TARGET:-}" ] || fail "MACOSX_DEPLOYMENT_TARGET is not set; the deployment-target ceiling has no value to compare against." + # Reached only from the extension repository's macOS lane; the # bundled-build workflow here is Linux-only. linked="$(otool -L "$so")" @@ -168,13 +182,13 @@ Darwin) fail "The object carries an LC_RPATH." fi - # 3, 3b, 3c -- the symbol checks, with Mach-O's leading underscore. + # 3. The symbol checks, with Mach-O's leading underscore. # `nm -gU` (external only, defined only) means the same to Apple's nm and # to the llvm-nm now behind it. defined="$(nm -gU "$so")" check_symbols _ - # 4a. Nothing but libSystem, the Mach-O counterpart of the NEEDED + # 4. Nothing but libSystem, the Mach-O counterpart of the NEEDED # allowlist. Only indented lines whose first field is an absolute path # are dependencies, which skips otool's echo of the object's own path # and, on a universal binary, the architecture headers. @@ -190,7 +204,7 @@ Darwin) esac done <<<"$dylibs" - # 4. The macOS analogue of the glibc floor. Every slice is measured, not + # 5. The macOS analogue of the glibc floor. Every slice is measured, not # just the first: otool -l emits load commands per architecture, arm64 # has a hard 11.0 floor, and Xcode clamps minos per architecture, so an # x86_64 slice at 11.0 can hide an arm64 slice at 14.0. diff --git a/ext/config.m4 b/ext/config.m4 index 40032f5f..e4357e63 100644 --- a/ext/config.m4 +++ b/ext/config.m4 @@ -10,6 +10,12 @@ PHP_ARG_WITH(maxminddb-bundled, PHP_ARG_ENABLE(maxminddb-debug, for MaxMind DB debug support, [ --enable-maxminddb-debug Enable MaxMind DB debug support], no, no) +dnl --with-maxminddb-bundled on its own is otherwise a silent no-op: the whole +dnl block below is skipped, configure exits 0, and make builds nothing. +if test "$PHP_MAXMINDDB_BUNDLED" != "no" && test "$PHP_MAXMINDDB" = "no"; then + AC_MSG_ERROR([--with-maxminddb-bundled requires --with-maxminddb]) +fi + if test $PHP_MAXMINDDB != "no"; then maxminddb_sources="maxminddb.c" @@ -42,9 +48,10 @@ if test $PHP_MAXMINDDB != "no"; then dnl warning-free under it, and --enable-maxminddb-debug turns warnings dnl into errors. dnl - dnl The last two actions must be non-empty: autoconf treats an empty - dnl argument as absent and substitutes its own default, which for - dnl action-if-unknown is to abort rather than fall through. + dnl The third action must be non-empty: autoconf treats an empty + dnl argument as absent, and its default for action-if-unknown is to + dnl abort rather than fall through. The fourth is passed for symmetry; + dnl its default is a harmless AC_DEFINE libmaxminddb never reads. m4_define([_mmdb_endian_unknown], [AC_MSG_NOTICE([endianness undetermined; letting maxminddb.h derive MMDB_LITTLE_ENDIAN from __BYTE_ORDER__])]) AC_C_BIGENDIAN([CFLAGS="$CFLAGS -DMMDB_LITTLE_ENDIAN=0"], @@ -58,7 +65,7 @@ if test $PHP_MAXMINDDB != "no"; then dnl something linked against a system libmaxminddb could bind across dnl the two. dev-bin/gate-extension.sh asserts the result. dnl - dnl -UHAVE_CONFIG_H must come first: PHP's CPPFLAGS already define + dnl -UHAVE_CONFIG_H must precede -DHAVE_CONFIG_H=0: PHP's CPPFLAGS define dnl HAVE_CONFIG_H, and redefining it is a warning that becomes an error dnl under --enable-maxminddb-debug, which adds -Werror. dnl @@ -108,9 +115,8 @@ if test $PHP_MAXMINDDB != "no"; then dnl directory for the bundled sources is never created and they fail to dnl compile. if test "$PHP_MAXMINDDB_BUNDLED" != "no"; then - dnl config.w32 has had this check since it was written; config.m4 had - dnl none, so a clone without --recursive produced a stray "No such file - dnl or directory", a *successful* configure -- autoconf-generated + dnl A clone without --recursive otherwise produces a stray "No such + dnl file or directory", a *successful* configure -- autoconf-generated dnl configure does not run under set -e -- and then an opaque failure dnl much later at maxminddb.h. if test ! -f "$ext_srcdir/libmaxminddb/src/maxminddb.c"; then diff --git a/ext/config.w32 b/ext/config.w32 index 07e158f8..29b4bc54 100644 --- a/ext/config.w32 +++ b/ext/config.w32 @@ -19,10 +19,16 @@ if (PHP_MAXMINDDB == "yes") { * config.m4 keeps a literal because test-bundled.yml compares * MMDB_LIB_VERSION against configure.ac on every build. */ var maxminddb_ac = file_get_contents(maxminddb_bundled + "\\configure.ac"); - if (!maxminddb_ac || !maxminddb_ac.match(/AC_INIT\(\[libmaxminddb\], \[([^\]]+)\]/)) { + /* The match is captured rather than read back out of RegExp.$1, which is + * engine-global state shared with every other config.w32 evaluated in + * the same scope. */ + var maxminddb_match = maxminddb_ac + ? maxminddb_ac.match(/AC_INIT\(\[libmaxminddb\], \[([^\]]+)\]/) + : null; + if (!maxminddb_match) { ERROR("could not read the libmaxminddb version from ext/libmaxminddb/configure.ac"); } - var maxminddb_version = RegExp.$1; + var maxminddb_version = maxminddb_match[1]; /* ext/bundled-include supplies maxminddb_config.h, which * libmaxminddb's maxminddb.h includes unconditionally and its own build @@ -34,7 +40,12 @@ if (PHP_MAXMINDDB == "yes") { '/I "' + maxminddb_bundled + '\\include" ' + '/I "' + maxminddb_bundled + '\\src"'); - /* These match ext/config.m4, except that HAVE_CONFIG_H is left alone. + /* The uint128 defines match ext/config.m4. Three things deliberately do + * not: HAVE_CONFIG_H is left alone here (below); MMDB_LITTLE_ENDIAN is + * hardcoded because every Windows target is little-endian, where + * config.m4 derives it and may leave it undefined; and there is no + * -fvisibility=hidden counterpart, because MSVC exports nothing that is + * not marked __declspec(dllexport). * config.m4 has to define it as 0 because PHP's Unix CPPFLAGS already * define it, and libmaxminddb's maxminddb.c tests the *value* with * `#if HAVE_CONFIG_H`, so it would otherwise include the config.h it @@ -83,13 +94,9 @@ if (PHP_MAXMINDDB == "yes") { * directories and nothing would collide -- that is how ext/gd gets away * with libgd/gd.c next to gd.c. Under phpize, which is how this * extension is built for Windows, confutils.js takes a different branch - * and ignores that argument: - * - * if (MODE_PHPIZE) { - * var build_dir = (dirname ? dirname : "")... - * - * where dirname is only the directory part of each file_list entry. Our - * entries are bare filenames, so both sources resolve to + * in ADD_SOURCES that derives the build directory from the dirname of + * each file_list entry and ignores the fourth argument. Our entries are + * bare filenames, so both sources resolve to * $(BUILD_DIR)\maxminddb.obj, the makefile gets two rules for one * target, and whichever compiles last wins. When libmaxminddb's won, * the DLL linked without our get_module() and PHP rejected it with