diff --git a/.docker/README.md b/.dev/README.md similarity index 56% rename from .docker/README.md rename to .dev/README.md index d4ace260467..3fd6bcfa4cc 100644 --- a/.docker/README.md +++ b/.dev/README.md @@ -1,17 +1,56 @@ # SMF development environment -A throwaway, reproducible local stack for working on SMF 3.0. Nothing here is -part of the shipped forum — it lives in `.docker/` precisely so the CI checks -(`check-smf-index.php`, `check-smf-license.php`) skip it. +Tools for installing a throwaway SMF 3.0 forum and running the tests against it. +Nothing here is part of the shipped forum — it lives in a dot directory +precisely so the CI checks (`check-smf-index.php`, `check-smf-license.php`) skip +it, and so it is never served by a real install. -Both database engines SMF supports are in the stack. **MySQL is the default.** +Both database engines SMF supports are covered. **MySQL is the default.** + +## Two ways to run it + +Every script here works either against the PHP and database on this machine, or +against the Docker stack in `.docker/`. `SMF_RUNNER` decides, and it defaults to +**local**: + +```sh +.dev/install-forum.sh --engine mysql # this machine +.dev/install-forum.sh --docker --engine mysql # the stack +``` + +Local is the default because it needs nothing installed beyond what SMF itself +requires, and because it is the only one available on a machine without Docker. +It is also what CI uses. + +**If you work in the Docker stack, put this in `.env` once** and every command +in this file works exactly as written: + +```sh +SMF_RUNNER=docker +``` + +The two differ in one way worth knowing. Under Docker the forum is served by +Apache, the way it is in production. Locally it is served by PHP's own built-in +server, which is enough for the tests — SMF routes on the query string and on +`PATH_INFO`, and there is no `.htaccess` at the root, so nothing here wants +mod_rewrite — but it is not Apache. When a bug might be about the web server, +reach for the stack. ## Requirements -Docker Desktop (Linux containers). Nothing else — no local PHP, Composer, MySQL -or PostgreSQL install is needed. +**Locally:** PHP with the extensions SMF needs (`mysqli` or `pgsql` for the +engine you pick, plus `mbstring`, `fileinfo` and `curl`), Composer, and a MySQL +or PostgreSQL server you can already reach. The scripts expect a database and a +user matching `DB_NAME`, `DB_USER` and `DB_PASSWORD` — `smf` / `smf` / `smf` by +default — and never create or drop the database itself, only the tables in it. +Point them elsewhere with `SMF_MYSQL_SERVER`, `SMF_MYSQL_PORT` and the +PostgreSQL equivalents. If `php` is not on `PATH`, set `PHP_BIN`. + +Create the MySQL database as `utf8mb4`; nothing here alters it afterwards. + +**With Docker:** Docker Desktop (Linux containers), and nothing else. -## Start +## The Docker stack ```sh docker compose up -d --build @@ -31,8 +70,19 @@ and wait for the `[smf-dev] ready` line. Credentials are `smf` / `smf` / database `smf` on both engines. +Those two published ports are also how a *local* run can borrow the stack's +databases without installing a server, which is a useful halfway house on a +machine that has PHP but no MySQL: + +```sh +SMF_MYSQL_PORT=3307 SMF_POSTGRES_PORT=5433 .dev/test.sh --engine both +``` + ## Choosing the engine +This section is about the stack only; the scripts take `--engine` and are not +affected by it. + Both database services always start. `SMF_DB_TYPE` decides which one the forum is pointed at, and it defaults to `mysql`: @@ -54,9 +104,9 @@ forum. ## Installing the forum ```sh -.docker/install-forum.sh --engine mysql -.docker/install-forum.sh --engine postgresql -.docker/install-forum.sh --engine both +.dev/install-forum.sh --engine mysql +.dev/install-forum.sh --engine postgresql +.dev/install-forum.sh --engine both ``` That resets the engine's database and installs a forum into it, with no browser @@ -98,12 +148,12 @@ ever be live in a process. Both installs are kept. Switch between them with: ```sh -.docker/use-engine.sh postgresql +.dev/use-engine.sh postgresql ``` That puts the saved `Settings.php` back and clears `cache/`. No restart is needed — the entrypoint only writes `Settings.php` when there is not one, so it -leaves whatever is in place alone. The copies live in `.docker/settings/` and +leaves whatever is in place alone. The copies live in `.dev/settings/` and are gitignored. `reset.sh` is the other half: it empties one engine's database and restages the @@ -116,9 +166,9 @@ Two forums, each with its own administrator, and a password chosen months ago is a recipe for an afternoon of hand written SQL. `user.sh` is there so it is not: ```sh -.docker/user.sh list -.docker/user.sh check admin 'password' -.docker/user.sh reset admin 'a new password' +.dev/user.sh list +.dev/user.sh check admin 'password' +.dev/user.sh reset admin 'a new password' ``` `check` exits 0 when SMF would accept the password and 1 when it would not, so @@ -130,7 +180,7 @@ exactly like a wrong one. engine, so the *other* forum can be inspected without switching to it: ```sh -.docker/user.sh check admin 'password' --engine mysql +.dev/user.sh check admin 'password' --engine mysql ``` The hashing goes through SMF's own `Security` class rather than being written @@ -168,9 +218,9 @@ exists, `Settings.php` redirects every request back into the installer. ## Running the tests ```sh -.docker/test.sh # both engines -.docker/test.sh --engine postgresql -.docker/test.sh --engine both --filter ModSettings +.dev/test.sh # both engines +.dev/test.sh --engine postgresql +.dev/test.sh --engine both --filter ModSettings ``` Anything it does not recognise is passed on to PHPUnit. It installs a forum for @@ -183,15 +233,193 @@ bug still in place** and only fails on PostgreSQL, because MySQL coerces text to a number where PostgreSQL refuses. A suite that only ever sees one engine proves considerably less than it looks like it does. -The unit suite needs none of this — `composer test` runs everything, and the -integration tests skip themselves when there is no forum to talk to. +The unit suite needs none of this — `composer test-unit` runs it on its own, and +`composer test` runs everything, with the integration tests skipping themselves +when there is no forum to talk to. + +That skip is a convenience, and in CI it would be a lie: a job wired to the wrong +database would report a green run having tested nothing. So the workflow passes +`--fail-on-skipped`, and anything worth doing by hand before pushing should too: + +```sh +.dev/test.sh --engine both --testsuite integration --fail-on-skipped +``` + +Some of the tests sign in, so they need to know the administrator. They default +to what `install-forum.sh` creates (`admin` / `password`); if your forum has +different credentials, export them: + +```sh +SMF_ADMIN_USER=admin SMF_ADMIN_PASS='…' .dev/test.sh +``` + +Getting that wrong makes those tests **skip**, with a message saying so, rather +than fail — a password the suite does not know is a misconfigured forum, not a +regression. `user.sh check admin '…'` settles which it is, and +`user.sh reset admin password` puts a forum installed some other way back on the +credentials the suite expects. + +## Writing a test + +### Which suite + +Three of them, and picking the wrong one is the usual reason a test is harder to +write than it should be: + +| Suite | Has | Use it for | +| ------------------------ | -------------------------------------- | ----------------------------- | +| `tests/Unit` | nothing — no database, no request | pure functions, value objects | +| `tests/Integration` | `Db::$db`, `$modSettings`, `User::$me` | anything needing real data | +| `tests/Integration/Http` | all of that, plus a real request | proving a *page* works | + +Work down the list and stop at the first that can hold the test. An HTTP test +costs about a second and cannot be rolled back; a unit test costs nothing. The +limits of the unit suite are spelled out in `AGENTS.md`. + +Reach for HTTP only when the thing worth proving is in the parts nothing else +touches: the session, the cookies, the security token, the theme and the +templates. `User::setMe()` skips every one of them, which is exactly why the +plain integration tests are cheap. + +### The shape of an HTTP test + +Four beats: fetch a page, submit a form on it, assert on what came back, assert +nothing was logged. + +```php +#[CoversNothing] +class ProfileTest extends HttpTestCase +{ + public function testAMemberCanChangeTheirSignature(): void + { + $this->signInAsAdmin(); + + $form = $this->fetch('?action=profile;area=forumprofile'); + + $response = $this->submitForm($form, [ + 'signature' => 'Set by the integration suite.', + 'save' => 'Change profile', // the button + ], '//form[contains(@action, "area=forumprofile")]'); + + $this->assertLessThan(400, $response->status, $response->errorText()); + $this->assertNoErrorsLogged('saving a signature logged something.' . "\n"); + } +} +``` + +`#[CoversNothing]` is not optional. These cross dozens of classes, so naming one +would be untrue, and `failOnRisky` wants an attribute either way. + +Four things that are easy to get wrong: + +- **Submit through `submitForm()`, not `HttpClient::submit()`.** Only the former + waits out flood control. `Security::spamProtection()` gives a moderator two + seconds between posts, per IP, and the tests all arrive from the same one far + faster than a person would; without the wait you get a suite that fails about + one run in five for no reproducible reason. +- **Name the button you are pressing.** `formFields()` leaves every button out on + purpose, because the posting form carries both `preview` and `post` and sending + the pair means preview quietly wins — no post, and a perfectly good 200 to show + for it. +- **Clean up whatever you write.** There is no transaction here; see + `HttpTestCase::usesTransaction()` for why one would not help. `PostingTest` + deletes through `Topic::remove()` rather than by hand, so the board and member + counters go back as well. +- **`assertNoErrorsLogged()` is the point of the test**, not a formality. A page + can return exactly the right HTML while logging an undefined index, and that is + the failure mode this whole suite exists to catch. + +One thing to rule out before believing a failure: if `install.php` is still in +the board root, SMF puts a "MAJOR SECURITY RISK" box on every page it shows an +administrator. That is an `errorbox`, so it fails `assertLooksLikeAForumPage()` +and turns up in `errorText()` in front of whatever the test was actually looking +at. `install-forum.sh` removes the file once it is done; a forum installed +through the browser needs it deleting by hand. + +### Who the request is + +The identity of an HTTP request is the cookie jar and nothing else. There are two +states out of the box: a guest, which is what `setUp()` leaves you, and the +administrator, through `signInAsAdmin()`. + +**`actingAs()` does not work here.** It is inherited from `IntegrationTestCase` +and it repoints `User::$me` in the PHPUnit process — but the request is handled +by Apache in a different process, which knows only the cookie. Calling it in an +HTTP test changes nothing about the request and leaves the assertions describing +a guest, confidently. + +So: + +- **Two users at once** means two `HttpClient` instances. Each opens its own + cookie jar, so they are independent browsers — which is how to test one member + sending another a PM. +- **Back to being a guest** is `$this->http->forgetCookies()`, then + `$this->http->get('')` to pick up a fresh session. +- **A member who is not the administrator** has to be made first, through + `Register2::registerMember()` with `interface => 'admin'` (which needs + `actingAs($this->adminId())` first, as it checks `moderate_forum`). That member + outlives the test, so delete it in `tearDown()`. + +### Finding the endpoint and the field names + +Endpoints are looked up; field names are not. + +`Forum::$actions` in `Sources/Forum.php` is the authoritative list of every +`?action=` the forum answers and the class behind it. Sub-actions — the `;area=` +and `;sa=` parts — are a `$subactions` property on that class. So +`?action=profile;area=forumprofile` resolves as `$actions['profile']` → +`Actions\Profile\Main` → its `$subactions`. That is quicker and more reliable +than reading templates. + +Field names you are deliberately not meant to know. `HttpClient::submit()` +scrapes every input, textarea and select out of the form it was handed and sends +them back, the way a browser does. That is what carries the session check and the +security token, both named unpredictably per session and neither hardcodable. All +a test supplies is the few values it is choosing, plus the button. + +When you do need to see them, ask the page rather than the template: + +```sh +docker compose exec web php -r ' + require "tests/bootstrap.php"; + $c = new SMF\Tests\Support\HttpClient(); + $c->get(""); + $p = $c->get("?action=login"); + print_r($p->formFields("//form[contains(@action, \"login2\")]"));' +``` + +``` +Array +( + [user] => + [passwrd] => + [d0004e1655] => b2f5189a9b3014cadee7bcb0b8d697f2 + [b8ae8fd32d] => 8f6026ed9a1b91bf6fec315801a2a93c +) +``` + +Two named fields, which are the ones a test writes, and two whose names are +different for every session — the session check and the security token, and +running the command twice gives two different pairs. That is what +`submit()` is for, and why a test that builds its own POST body by hand gets a +403 it cannot fix. + +The paths are relative because the container's working directory is +`/var/www/html` already. Spelling them absolutely also works, but not from Git +Bash on Windows, which rewrites anything that looks like a Unix path before +Docker sees it. + +Note the throwaway `get("")` before the form is fetched. The very first request +of a new session regenerates it, so a token minted on the first page a visitor +ever sees is bound to a session that no longer exists by the time it comes back. +The symptom is a 403 about the token, when the token was never the problem. ## Running CI locally ```sh -.docker/ci.sh # everything CI checks -.docker/ci.sh --full # style check over the whole tree, not just changes -.docker/ci.sh --fix # apply the style fixes rather than reporting them +.dev/ci.sh # everything CI checks +.dev/ci.sh --full # style check over the whole tree, not just changes +.dev/ci.sh --fix # apply the style fixes rather than reporting them ``` Mirrors `php.yml` (sign-off, the four file integrity checks, phplint) and @@ -207,10 +435,12 @@ before you push rather than after. Two things it cannot do for you: -- **The other PHP version.** CI lints and tests on 8.4 *and* 8.5; the container - is whichever built it. To cover the other: +- **The other PHP version.** CI lints and tests on 8.4 *and* 8.5; this runs on + whichever `php` it found, or whichever `PHP_VERSION` built the container. To + cover the other, point `PHP_BIN` at it, or `PHP_VERSION=8.5 docker compose up -d --build web`. -- **The integration tests on both engines.** Use `.docker/test.sh` for that. +- **Both engines at once.** It runs the suite against whichever forum is live. + Use `.dev/test.sh --engine both` for the pair. ## Hardening the upgrade @@ -223,8 +453,8 @@ off half way. ```sh BASE=../SMF-2.1/.docker/baseline/artifacts/2.1.7-1/small/mysql.sql -.docker/rerun-upgrade.sh --engine mysql --baseline "$BASE" -.docker/interrupt-upgrade.sh --engine mysql --baseline "$BASE" +.dev/rerun-upgrade.sh --engine mysql --baseline "$BASE" +.dev/interrupt-upgrade.sh --engine mysql --baseline "$BASE" ``` Both rebuild the database for the engine they are given, so anything installed @@ -289,7 +519,7 @@ The repository is bind-mounted at `/var/www/html`, so edits on the host are live on the next request. Opcache is on but revalidates every request, so you never need to restart for a PHP change. -To reinstall from scratch: `.docker/install-forum.sh --engine mysql --force`. +To reinstall from scratch: `.dev/install-forum.sh --engine mysql --force`. To wipe everything including the volumes: `docker compose down -v`. ## Comparing an upgrade against a fresh install @@ -300,7 +530,7 @@ whatever 2.1 left behind. They are meant to converge, and nothing checks that they do: ```bash -.docker/compare-upgrade.sh --engine mysql --baseline path/to/a-2.1-dump.sql +.dev/compare-upgrade.sh --engine mysql --baseline path/to/a-2.1-dump.sql ``` That empties the database, loads the dump, upgrades it, reads the schema, @@ -341,10 +571,10 @@ same engine — two forums you already have, or the same forum before and after something you are testing: ```bash -docker compose exec web php .docker/schema-tool.php dump --engine mysql --db smf > before.json +docker compose exec web php .dev/schema-tool.php dump --engine mysql --db smf > before.json # ... do the thing ... -docker compose exec web php .docker/schema-tool.php dump --engine mysql --db smf > after.json -docker compose exec web php .docker/schema-tool.php diff before.json after.json +docker compose exec web php .dev/schema-tool.php dump --engine mysql --db smf > after.json +docker compose exec web php .dev/schema-tool.php diff before.json after.json ``` It talks to the database directly rather than through SMF, so it works on a @@ -418,16 +648,25 @@ compose.yaml the stack .docker/mysql/init/10-smf.sh runs once on first mysql database creation .docker/postgres/init/10-smf.sh runs once on first postgres database creation .docker/env.example optional overrides -.docker/lib.sh paths, credentials and engine names, shared -.docker/install-forum.sh install a forum with no browser involved -.docker/reset.sh empty one engine and restage the installer -.docker/use-engine.sh switch which installed forum is live -.docker/user.sh inspect accounts, check and reset passwords -.docker/test.sh run the test suites against an installed forum - -.docker/upgrade-readings.sh shared: driving upgrade.php, reading a database -.docker/rerun-upgrade.sh upgrade twice, report what the second run changed -.docker/interrupt-upgrade.sh kill an upgrade part way, report what recovery left -.docker/compare-upgrade.sh upgrade a 2.1 dump, install 3.0, diff the two -.docker/schema-tool.php read a database's shape, and compare readings + +.dev/lib.sh paths, credentials, engine names, the runner +.dev/db.php one SQL statement, without a database client +.dev/install-forum.sh install a forum with no browser involved +.dev/reset.sh empty one engine and restage the installer +.dev/use-engine.sh switch which installed forum is live +.dev/user.sh inspect accounts, check and reset passwords +.dev/test.sh run the suite against a real forum +.dev/ci.sh run what CI runs, before pushing + +.dev/upgrade-readings.sh shared: driving upgrade.php, reading a database +.dev/rerun-upgrade.sh upgrade twice, report what the second run changed +.dev/interrupt-upgrade.sh kill an upgrade part way, report what recovery left +.dev/compare-upgrade.sh upgrade a 2.1 dump, install 3.0, diff the two +.dev/schema-tool.php read a database's shape, and compare readings ``` + +`.dev/db.php` exists because the two runners reach the database differently. +Under Docker the `mysql` and `psql` clients are already inside the container; +locally they would be a dependency nothing else here needs, while `mysqli` and +`pgsql` are ones SMF has anyway. So any machine that can serve the forum can run +these scripts, with no database client installed. diff --git a/.docker/ci.sh b/.dev/ci.sh similarity index 71% rename from .docker/ci.sh rename to .dev/ci.sh index 101c5a5ee18..6b3aace35f0 100755 --- a/.docker/ci.sh +++ b/.dev/ci.sh @@ -1,9 +1,10 @@ #!/usr/bin/env bash # Runs what CI runs, before you push instead of after. # -# .docker/ci.sh every check -# .docker/ci.sh --full style check over the whole tree, not just changes -# .docker/ci.sh --fix apply the style fixes rather than reporting them +# .dev/ci.sh every check +# .dev/ci.sh --full style check over the whole tree, not just changes +# .dev/ci.sh --fix apply the style fixes rather than reporting them +# .dev/ci.sh --docker run the checks in the web container # # The workflows this mirrors are php.yml (sign-off, the file integrity checks, # phplint) and php-cs-fixer.yml. phpunit.yml is included when the branch has a @@ -12,9 +13,10 @@ # Every check runs even after one fails, because finding out about the second # problem on the next push is the thing this script exists to stop. # -# One difference worth knowing: CI lints and tests on PHP 8.4 *and* 8.5, and the -# web container is whichever PHP_VERSION built it (8.4 by default). To cover the -# other one, rebuild against it: +# One difference worth knowing: CI lints and tests on PHP 8.4 *and* 8.5, while +# this runs on one of them -- whichever php is on PATH, or whichever PHP_VERSION +# built the web container. To cover the other one, point PHP_BIN at it, or +# rebuild the image against it: # # PHP_VERSION=8.5 docker compose up -d --build web # @@ -23,6 +25,8 @@ set -uo pipefail . "$(dirname -- "${BASH_SOURCE[0]}")/lib.sh" +parse_runner_args "$@" + FULL=0 FIX=0 @@ -30,7 +34,8 @@ while [ $# -gt 0 ]; do case "$1" in --full) FULL=1; shift ;; --fix) FIX=1; shift ;; - -h|--help) sed -n '2,21p' "${BASH_SOURCE[0]}"; exit 0 ;; + --docker|--local) shift ;; + -h|--help) sed -n '2,22p' "${BASH_SOURCE[0]}"; exit 0 ;; *) die "unknown argument: $1" ;; esac done @@ -39,19 +44,23 @@ done # check does not stop the ones after it. That means cd has to be checked. cd "$BOARD_DIR" || die "cannot enter $BOARD_DIR" -docker compose ps --status running --services 2>/dev/null | grep -qx web \ - || die 'the web container is not running -- docker compose up -d' +if is_docker; then + docker compose ps --status running --services 2>/dev/null | grep -qx web \ + || die 'the web container is not running -- docker compose up -d' +else + require_local_deps +fi FAILED='' -# $1 label, rest: the command to run in the web container. +# $1 label, rest: the command to run where the forum lives. check() { local label="$1" shift printf '\n[smf-dev] --- %s ---\n' "$label" - if docker compose exec -T web "$@"; then + if run_cmd "$@"; then return 0 fi @@ -72,8 +81,8 @@ check 'file integrity' sh -c ' echo "all four integrity checks passed" ' -check "syntax ($(docker compose exec -T web php -r 'echo PHP_MAJOR_VERSION . "." . PHP_MINOR_VERSION;' 2>/dev/null))" \ - vendor/bin/phplint --no-progress --exclude .git --exclude vendor . +check "syntax ($(run_cmd php -r 'echo PHP_MAJOR_VERSION . "." . PHP_MINOR_VERSION;' 2>/dev/null))" \ + php vendor/bin/phplint --no-progress --exclude .git --exclude vendor . # ------------------------------------------------------------ php-cs-fixer.yml # CI checks only the files a pull request changed, and switches to the whole @@ -90,7 +99,7 @@ else fi if [ "$FULL" -eq 1 ]; then - check 'code style (whole tree)' vendor/bin/php-cs-fixer "$FIXER_MODE" "${FIXER_ARGS[@]}" + check 'code style (whole tree)' php vendor/bin/php-cs-fixer "$FIXER_MODE" "${FIXER_ARGS[@]}" else # Same intersection CI builds, from the files this branch actually touches: # committed since release-3.0, staged, unstaged, and - the one CI never has @@ -107,13 +116,13 @@ else printf '\n[smf-dev] --- code style --- no changed PHP files\n' else # shellcheck disable=SC2086 - check 'code style (changed files)' vendor/bin/php-cs-fixer "$FIXER_MODE" "${FIXER_ARGS[@]}" --path-mode=intersection $CHANGED + check 'code style (changed files)' php vendor/bin/php-cs-fixer "$FIXER_MODE" "${FIXER_ARGS[@]}" --path-mode=intersection $CHANGED fi fi # ---------------------------------------------------------------- phpunit.yml if [ -f phpunit.xml.dist ]; then - check 'tests' vendor/bin/phpunit --no-coverage --colors=always + check 'tests' php vendor/bin/phpunit --no-coverage --colors=always else printf '\n[smf-dev] --- tests --- no phpunit.xml.dist on this branch, skipping\n' fi diff --git a/.docker/compare-upgrade.sh b/.dev/compare-upgrade.sh similarity index 88% rename from .docker/compare-upgrade.sh rename to .dev/compare-upgrade.sh index 185d07b21a0..1a9ebf803d2 100755 --- a/.docker/compare-upgrade.sh +++ b/.dev/compare-upgrade.sh @@ -2,8 +2,8 @@ # Upgrades a 2.1 database to 3.0, installs 3.0 from scratch, and reports where # the two schemas disagree. # -# .docker/compare-upgrade.sh --engine mysql --baseline ../SMF-2.1/.docker/baseline/artifacts/2.1.7-1/small/mysql.sql -# .docker/compare-upgrade.sh --engine postgresql --baseline ../SMF-2.1/.docker/baseline/artifacts/2.1.7-1/small/postgres.sql +# .dev/compare-upgrade.sh --engine mysql --baseline ../SMF-2.1/.docker/baseline/artifacts/2.1.7-1/small/mysql.sql +# .dev/compare-upgrade.sh --engine postgresql --baseline ../SMF-2.1/.docker/baseline/artifacts/2.1.7-1/small/postgres.sql # # The installer builds the schema from Sources/Db/Schema/v3_0/ in one go. The # upgrader arrives at the same place through a hundred-odd migrations applied @@ -24,11 +24,19 @@ # Runs on the host. Expect five to ten minutes per engine. set -euo pipefail +# Orchestrating containers is all this can mean, so it says so rather than +# inheriting the default and failing further in. Read by lib.sh, which is +# sourced below and which a linter reading this file alone cannot see. +# shellcheck disable=SC2034 +SMF_RUNNER=docker + . "$(dirname -- "${BASH_SOURCE[0]}")/lib.sh" +require_docker + ENGINE='' BASELINE='' -OUT="$DOCKER_DIR/compare" +OUT="$DEV_DIR/compare" while [ $# -gt 0 ]; do case "$1" in @@ -65,7 +73,7 @@ esac snapshot() { local smf_type="$1" label="$2" file="$3" - docker compose exec -T web php .docker/schema-tool.php dump \ + docker compose exec -T web php .dev/schema-tool.php dump \ --engine "$smf_type" \ --db "$DB_NAME" \ --prefix "$DB_PREFIX" \ @@ -106,7 +114,7 @@ compare_one() { # ---------------------------------------------------------- the upgrade log "${smf_type}: emptying the database" - "$DOCKER_DIR/reset.sh" --engine "$smf_type" >/dev/null + "$DEV_DIR/reset.sh" --docker --engine "$smf_type" >/dev/null log "${smf_type}: loading ${BASELINE##*/}" load_baseline "$smf_type" @@ -163,14 +171,14 @@ compare_one() { # --force because there is an installed forum now, and install-forum.sh # leaves one alone unless told otherwise. log "${smf_type}: installing from scratch" - "$DOCKER_DIR/install-forum.sh" --engine "$smf_type" --force >/dev/null + "$DEV_DIR/install-forum.sh" --docker --engine "$smf_type" --force >/dev/null snapshot "$smf_type" fresh "$OUT/fresh-${smf_type}.json" # ------------------------------------------------------------ the report local status=0 - docker compose exec -T web php .docker/schema-tool.php diff \ + docker compose exec -T web php .dev/schema-tool.php diff \ "${OUT_REL}/fresh-${smf_type}.json" \ "${OUT_REL}/upgraded-${smf_type}.json" \ > "$OUT/report-${smf_type}.txt" || status=$? diff --git a/.dev/db.php b/.dev/db.php new file mode 100644 index 00000000000..e721783d1ae --- /dev/null +++ b/.dev/db.php @@ -0,0 +1,169 @@ + (string) $value, $row)), "\n"; + } +} + +if ($engine === 'mysql') { + if (!extension_loaded('mysqli')) { + missing('mysqli'); + } + + // Off, so a failure arrives as a return value to report rather than as an + // exception with a stack trace nobody reading a shell script wants. + mysqli_report(MYSQLI_REPORT_OFF); + + // An empty database name is how the callers connect with no database yet, + // which is what dropping and recreating one needs. + $connection = @new mysqli($server, $user, $password, $database, $port); + + if ($connection->connect_errno !== 0) { + fwrite(STDERR, "db.php: could not connect to mysql at {$server}:{$port} as {$user}: " . $connection->connect_error . "\n"); + + exit(1); + } + + // The callers send several statements at once when they reset a database, + // so this is multi_query rather than query. + if ($connection->multi_query($sql) === false) { + fwrite(STDERR, 'db.php: ' . $connection->error . "\n"); + + exit(1); + } + + while (true) { + $result = $connection->store_result(); + + if ($result instanceof mysqli_result) { + emit($result->fetch_all(MYSQLI_NUM)); + $result->free(); + } + + // An error can surface on any statement after the first, where + // multi_query() has already said yes. It arrives two ways: as an errno + // on the current statement, or as next_result() refusing to move on. + // Testing only the first, or folding next_result() into a loop + // condition, is how a batch that failed halfway reports success. + if ($connection->errno !== 0) { + fwrite(STDERR, 'db.php: ' . $connection->error . "\n"); + + exit(1); + } + + if (!$connection->more_results()) { + break; + } + + if (!$connection->next_result()) { + fwrite(STDERR, 'db.php: ' . $connection->error . "\n"); + + exit(1); + } + } + + exit(0); +} + +if (!function_exists('pg_connect')) { + missing('pgsql'); +} + +$dsn = sprintf( + "host='%s' port=%d dbname='%s' user='%s' password='%s'", + addslashes($server), + $port, + addslashes($database), + addslashes($user), + addslashes($password), +); + +$connection = @pg_connect($dsn); + +if ($connection === false) { + fwrite(STDERR, "db.php: could not connect to postgresql at {$server}:{$port} as {$user}\n"); + + exit(1); +} + +$result = @pg_query($connection, $sql); + +if ($result === false) { + fwrite(STDERR, 'db.php: ' . pg_last_error($connection) . "\n"); + + exit(1); +} + +// pg_num_fields() is 0 for a statement that returns no rows, such as the DDL +// the reset uses, and fetch_all() on that emits a warning rather than nothing. +if (pg_num_fields($result) > 0) { + emit(pg_fetch_all($result, PGSQL_NUM)); +} + +exit(0); diff --git a/.docker/install-forum.sh b/.dev/install-forum.sh similarity index 74% rename from .docker/install-forum.sh rename to .dev/install-forum.sh index 8bc55700270..c39c32c8e83 100755 --- a/.docker/install-forum.sh +++ b/.dev/install-forum.sh @@ -1,9 +1,10 @@ #!/usr/bin/env bash # Installs the forum without a browser. # -# .docker/install-forum.sh --engine mysql -# .docker/install-forum.sh --engine postgresql -# .docker/install-forum.sh --engine both +# .dev/install-forum.sh --engine mysql +# .dev/install-forum.sh --engine postgresql +# .dev/install-forum.sh --engine both +# .dev/install-forum.sh --docker --engine mysql # # SMF 3.0's installer is CLI-native: Maintenance::parseCliArguments() turns # --name=value into $_POST, and Maintenance::execute() then runs every step in @@ -30,6 +31,8 @@ set -euo pipefail . "$(dirname -- "${BASH_SOURCE[0]}")/lib.sh" +parse_runner_args "$@" + ENGINE='' PIN_SECRETS=0 FORCE=0 @@ -40,7 +43,8 @@ while [ $# -gt 0 ]; do --engine=*) ENGINE="${1#*=}"; shift ;; --pin-secrets) PIN_SECRETS=1; shift ;; --force) FORCE=1; shift ;; - -h|--help) sed -n '2,27p' "${BASH_SOURCE[0]}"; exit 0 ;; + --docker|--local) shift ;; + -h|--help) sed -n '2,28p' "${BASH_SOURCE[0]}"; exit 0 ;; *) die "unknown argument: $1" ;; esac done @@ -48,6 +52,10 @@ done [ -n "$ENGINE" ] || die 'need --engine mysql|postgresql|both' ENGINES=$(engine_list "$ENGINE") || die "unknown engine: $ENGINE" +for smf_type in $ENGINES; do + require_local_deps "$smf_type" +done + cd "$BOARD_DIR" # The installer's own name for each engine, which is the key of the array it @@ -76,7 +84,7 @@ install_one() { fi log "${smf_type}: resetting" - "$DOCKER_DIR/reset.sh" --engine "$smf_type" >/dev/null + "$DEV_DIR/reset.sh" "--${SMF_RUNNER}" --engine "$smf_type" >/dev/null args=( --contbutt=1 @@ -96,18 +104,18 @@ install_one() { --password2="$SMF_ADMIN_PASS" ) - # reset.sh does not return until the entrypoint has staged this, so its - # absence means something went wrong there rather than here. Worth saying so: - # without it php reports "Could not open input file: install.php", which reads - # like a broken script rather than a forum that was never made installable. - docker compose exec -T web test -f install.php \ - || die "${smf_type}: install.php is not staged, so there is nothing to run (docker compose logs web)" + # reset.sh does not return until this is staged, so its absence means + # something went wrong there rather than here. Worth saying so: without it + # php reports "Could not open input file: install.php", which reads like a + # broken script rather than a forum that was never made installable. + [ -f "$BOARD_DIR/install.php" ] \ + || die "${smf_type}: install.php is not staged, so there is nothing to run" log "${smf_type}: building the schema" - docker compose exec -T web php install.php "${args[@]}" >/dev/null + run_php install.php "${args[@]}" >/dev/null log "${smf_type}: creating the administrator and finalising" - docker compose exec -T web php install.php "${args[@]}" --pop_done=1 >/dev/null + run_php install.php "${args[@]}" --pop_done=1 >/dev/null local version version=$(installed_version "$smf_type" || true) @@ -121,8 +129,8 @@ install_one() { # not removed install.php" box on every page it shows an administrator. # # Safe to delete even though a reinstall needs it again: install_one() always - # calls reset.sh first, and reset.sh clears Settings.php and waits for the - # entrypoint to put a fresh copy back before returning. + # calls reset.sh first, and reset.sh does not return until a fresh copy is + # staged alongside a fresh Settings.php. rm -f install.php log "${smf_type}: installed SMF ${version}" @@ -148,23 +156,30 @@ install_one() { pin_secrets() { log 'pinning auth_secret and image_proxy_secret' - # The values have to be handed over with -e. Exporting them on the host does - # nothing: docker compose exec starts a fresh environment, so getenv() came - # back empty and this wrote two empty secrets over the generated ones. - docker compose exec -T \ - -e PIN_AUTH_SECRET="$PIN_AUTH_SECRET" \ - -e PIN_IMAGE_PROXY_SECRET="$PIN_IMAGE_PROXY_SECRET" \ - web php -r ' + # The values have to be handed over rather than exported. docker compose exec + # starts a fresh environment, so getenv() came back empty and this wrote two + # empty secrets over the generated ones. The board directory travels the same + # way, because it is not the same path on both sides of that boundary. + # The $board, $auth and $proxy below belong to the PHP, not to the shell, so + # the quotes around it have to stay single. + # shellcheck disable=SC2016 + run_php_env \ + PIN_AUTH_SECRET="$PIN_AUTH_SECRET" \ + PIN_IMAGE_PROXY_SECRET="$PIN_IMAGE_PROXY_SECRET" \ + PIN_BOARD_DIR="$(run_board_dir)" \ + -- -r ' + $board = (string) getenv("PIN_BOARD_DIR"); + define("SMF", 1); - define("SMF_SETTINGS_FILE", "/var/www/html/Settings.php"); - define("SMF_SETTINGS_BACKUP_FILE", "/var/www/html/Settings_bak.php"); - require_once "/var/www/html/index.php"; + define("SMF_SETTINGS_FILE", $board . "/Settings.php"); + define("SMF_SETTINGS_BACKUP_FILE", $board . "/Settings_bak.php"); + require_once $board . "/index.php"; $auth = (string) getenv("PIN_AUTH_SECRET"); $proxy = (string) getenv("PIN_IMAGE_PROXY_SECRET"); if ($auth === "" || $proxy === "") { - fwrite(STDERR, "pin-secrets: the secrets did not reach the container\n"); + fwrite(STDERR, "pin-secrets: the secrets did not reach the php that had to write them\n"); exit(1); } @@ -184,7 +199,7 @@ save_settings() { cp Settings.php "$SETTINGS_DIR/Settings.${smf_type}.php" cp Settings_bak.php "$SETTINGS_DIR/Settings_bak.${smf_type}.php" - log "${smf_type}: settings saved to .docker/settings/" + log "${smf_type}: settings saved to .dev/settings/" } PIN_AUTH_SECRET="${PIN_AUTH_SECRET:-0b6e5f3c1a94d27e8f5b0c3a76d1e94f2b8c5a03e7d146f9b2c8a501d3e7f4c69}" @@ -200,6 +215,6 @@ done # Leave the first engine of a "both" run active rather than whichever happened # to go last, so the result does not depend on the order. FIRST_ENGINE="${ENGINES%% *}" -"$DOCKER_DIR/use-engine.sh" "$FIRST_ENGINE" >/dev/null +"$DEV_DIR/use-engine.sh" "--${SMF_RUNNER}" "$FIRST_ENGINE" >/dev/null log "active engine: ${FIRST_ENGINE} -- ${SMF_BOARDURL} (${SMF_ADMIN_USER} / ${SMF_ADMIN_PASS})" diff --git a/.docker/interrupt-upgrade.sh b/.dev/interrupt-upgrade.sh similarity index 93% rename from .docker/interrupt-upgrade.sh rename to .dev/interrupt-upgrade.sh index 3e700ed9461..a52508b8851 100644 --- a/.docker/interrupt-upgrade.sh +++ b/.dev/interrupt-upgrade.sh @@ -2,10 +2,10 @@ # Kills an upgrade part way through, starts it again, and reports whether the # forum it ends up with is the one an uninterrupted upgrade would have built. # -# .docker/interrupt-upgrade.sh --engine mysql --baseline ../SMF-2.1/.docker/baseline/artifacts/2.1.7-1/small/mysql.sql -# .docker/interrupt-upgrade.sh --engine mysql --baseline ... --at 40 -# .docker/interrupt-upgrade.sh --engine mysql --baseline ... --points 10,25,50,75,90 -# .docker/interrupt-upgrade.sh --engine mysql --baseline ... --backup +# .dev/interrupt-upgrade.sh --engine mysql --baseline ../SMF-2.1/.docker/baseline/artifacts/2.1.7-1/small/mysql.sql +# .dev/interrupt-upgrade.sh --engine mysql --baseline ... --at 40 +# .dev/interrupt-upgrade.sh --engine mysql --baseline ... --points 10,25,50,75,90 +# .dev/interrupt-upgrade.sh --engine mysql --baseline ... --backup # # --backup asks the upgrader for the backup step, which the command line skips # unless something does. It is worth turning on precisely because a retry is @@ -42,11 +42,19 @@ # points is the better part of an hour. set -euo pipefail +# Orchestrating containers is all this can mean, so it says so rather than +# inheriting the default and failing further in. Read by lib.sh, which is +# sourced below and which a linter reading this file alone cannot see. +# shellcheck disable=SC2034 +SMF_RUNNER=docker + . "$(dirname -- "${BASH_SOURCE[0]}")/lib.sh" +require_docker + ENGINE='' BASELINE='' -OUT="$DOCKER_DIR/interrupt" +OUT="$DEV_DIR/interrupt" POINTS='10,25,50,75,90' AT='' # How many times the upgrader may be started again after a kill before we call @@ -105,7 +113,7 @@ finished_upgrade_log() { # Everything the readings and the run share with rerun-upgrade.sh lives here # rather than being written twice. -. "$DOCKER_DIR/upgrade-readings.sh" +. "$DEV_DIR/upgrade-readings.sh" # How many substeps the log has seen. Each one is announced before it runs, so # this counts substeps started, not substeps finished -- which is what we want, @@ -181,7 +189,7 @@ interrupt_one_point() { local smf_type="$1" want="$2" round=0 version log "${smf_type}: emptying the database" - "$DOCKER_DIR/reset.sh" --engine "$smf_type" >/dev/null + "$DEV_DIR/reset.sh" --docker --engine "$smf_type" >/dev/null load_baseline "$smf_type" "$BASELINE" KILLED_AFTER='' @@ -277,7 +285,7 @@ interrupt_one_engine() { # ------------------------------------------------------- the reference log "${smf_type}: emptying the database" - "$DOCKER_DIR/reset.sh" --engine "$smf_type" >/dev/null + "$DEV_DIR/reset.sh" --docker --engine "$smf_type" >/dev/null load_baseline "$smf_type" "$BASELINE" [ -n "$(installed_version "$smf_type" || true)" ] \ diff --git a/.dev/lib.sh b/.dev/lib.sh new file mode 100644 index 00000000000..3b4c48cbbd6 --- /dev/null +++ b/.dev/lib.sh @@ -0,0 +1,482 @@ +#!/usr/bin/env bash +# Shared settings and helpers for the .dev scripts. Sourced, never run. +# +# Host-side scripts (reset.sh, install-forum.sh, use-engine.sh) source this from +# wherever the caller happens to be standing; everything below resolves paths +# for itself rather than assuming a working directory. +# +# Every script that sources this can work two ways, and $SMF_RUNNER decides +# which. See "the runner" below. +# +# Everything defined here is consumed by the scripts that source this file, and +# a linter reading it on its own cannot see any of those uses -- hence the +# blanket disable below. Keep it on its own, with nothing after it that starts +# with the linter's name, or the following line gets parsed as a directive too. +# +# shellcheck disable=SC2034 + +# Git Bash on Windows rewrites anything that looks like a Unix path before +# handing it to a program, so a container-side path like /var/www/html/... is +# silently turned into C:/Program Files/Git/var/www/html/... and the command +# fails with "Could not open input file". These two switch that off. They mean +# nothing on Linux and macOS. +export MSYS_NO_PATHCONV=1 +export MSYS2_ARG_CONV_EXCL='*' + +# Repository root, regardless of where the caller was standing. +DEV_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) +BOARD_DIR=$(cd -- "$DEV_DIR/.." && pwd) + +# Where use-engine.sh keeps each engine's Settings.php. Gitignored: these hold +# generated secrets and a machine-specific board URL. +SETTINGS_DIR="$DEV_DIR/settings" + +# ------------------------------------------------------------------ the runner +# Where the forum actually runs. 'local' uses the PHP on this machine and a +# database it can already reach; 'docker' uses the compose stack in .docker/. +# +# Local is the default because it is the one that needs nothing installed beyond +# what SMF itself requires, and because it is the only one available on a +# machine without Docker. Pass --docker, or set this in .env, for the stack. +SMF_RUNNER="${SMF_RUNNER:-local}" + +# Reads the --docker/--local flags every script accepts. Call it with "$@" +# before the script's own option loop, which then ignores them. +parse_runner_args() { + local arg + + for arg in "$@"; do + case "$arg" in + --docker) SMF_RUNNER='docker' ;; + --local) SMF_RUNNER='local' ;; + esac + done + + case "$SMF_RUNNER" in + docker|local) ;; + *) die "SMF_RUNNER must be 'local' or 'docker', not '${SMF_RUNNER}'" ;; + esac +} + +# Strips those flags out of a script's arguments, so an option loop that does +# not know about them does not have to. +without_runner_args() { + local arg + + for arg in "$@"; do + case "$arg" in + --docker|--local) ;; + *) echo "$arg" ;; + esac + done +} + +is_docker() { [ "$SMF_RUNNER" = 'docker' ]; } + +# For the scripts that orchestrate containers and cannot mean anything else. +# They set SMF_RUNNER=docker themselves, so this only fires if someone overrode +# it in the environment. +require_docker() { + is_docker || die "$(basename -- "$0") only works against the compose stack; unset SMF_RUNNER or set it to 'docker'" +} + +# ---------------------------------------------------------------- credentials +# These match compose.yaml's defaults. Override them in the environment if you +# changed them in .env. +DB_NAME="${DB_NAME:-smf}" +DB_USER="${DB_USER:-smf}" +DB_PASSWORD="${DB_PASSWORD:-smf}" +DB_ROOT_PASSWORD="${DB_ROOT_PASSWORD:-smf}" +DB_PREFIX="${DB_PREFIX:-smf_}" + +WEB_PORT="${WEB_PORT:-8080}" +SMF_BOARDURL="${SMF_BOARDURL:-http://localhost:${WEB_PORT}}" +SMF_MBNAME="${SMF_MBNAME:-SMF Dev}" + +# The administrator the installer creates. Dev-only values for a throwaway +# forum; never reuse them anywhere real. +SMF_ADMIN_USER="${SMF_ADMIN_USER:-admin}" +SMF_ADMIN_PASS="${SMF_ADMIN_PASS:-password}" +# example.com is reserved by RFC 2606, so this can never reach a real inbox. +# SMF's validator rejects dotless domains, so 'admin@localhost' is not an option. +SMF_ADMIN_EMAIL="${SMF_ADMIN_EMAIL:-admin@example.com}" + +# --------------------------------------------------------------------- output +log() { printf '[smf-dev] %s\n' "$*"; } +warn() { printf '[smf-dev] %s\n' "$*" >&2; } +die() { printf '[smf-dev] error: %s\n' "$*" >&2; exit 1; } + +# Engine name normalisation. Everything downstream uses either the SMF type +# ('mysql' / 'postgresql') or the compose service name ('mysql' / 'postgres'), +# and mixing them up is an easy way to waste an afternoon. +engine_smf_type() { + case "$1" in + mysql|mysqli|mariadb) echo 'mysql' ;; + postgres|postgresql|pgsql) echo 'postgresql' ;; + *) return 1 ;; + esac +} + +engine_service() { + case "$1" in + mysql|mysqli|mariadb) echo 'mysql' ;; + postgres|postgresql|pgsql) echo 'postgres' ;; + *) return 1 ;; + esac +} + +# Where an engine answers, as Settings.php has to spell it. Under docker that is +# the compose service on the container network, not the host-side ports in +# compose.yaml; locally it is the loopback address. Both are overridable, which +# is how a database somewhere else gets named. +engine_server() { + local fallback_mysql='127.0.0.1' fallback_postgres='127.0.0.1' + + if is_docker; then + fallback_mysql='mysql' + fallback_postgres='postgres' + fi + + case "$(engine_smf_type "$1")" in + mysql) echo "${SMF_MYSQL_SERVER:-$fallback_mysql}" ;; + postgresql) echo "${SMF_POSTGRES_SERVER:-$fallback_postgres}" ;; + *) return 1 ;; + esac +} + +# The same either way: the container network and a stock local install both use +# the default ports. The published 3307 and 5433 in compose.yaml are only how +# the host reaches the containers, which is not what goes in Settings.php. +engine_port() { + case "$(engine_smf_type "$1")" in + mysql) echo "${SMF_MYSQL_PORT:-3306}" ;; + postgresql) echo "${SMF_POSTGRES_PORT:-5432}" ;; + *) return 1 ;; + esac +} + +# Expands "both" into the engines to act on, in the order they run. Only one +# engine can be live at a time -- Settings.php pins $db_type and Db::load() +# early-returns once the connection exists -- so "both" is a sequential chain, +# never two connections. +engine_list() { + case "$1" in + both|all) echo 'mysql postgresql' ;; + *) engine_smf_type "$1" ;; + esac +} + +# The installed version for one engine, empty if the forum is not installed. +# Asks the database directly rather than trusting the presence of a file: +# Settings.php exists from the moment the entrypoint writes it, long before +# there is a forum behind it. +installed_version() { + db_sql "$1" "SELECT value FROM ${DB_PREFIX}settings WHERE variable = 'smfVersion';" 2>/dev/null +} + +# ----------------------------------------------------------------- php and sql +# Which PHP to run things with locally. Not used under docker, where it is +# whatever the image ships. +PHP_BIN="${PHP_BIN:-php}" + +# Runs php with some environment set, as `run_php_env VAR=value -- script args`. +# +# The variables have to be handed over rather than exported: docker compose exec +# starts a fresh environment, so an exported value never reaches the container +# and the script on the other end reads an empty string. +run_php_env() { + local -a envs=() flags=() + local entry + + while [ $# -gt 0 ] && [ "$1" != '--' ]; do + envs+=("$1") + shift + done + + shift + + if is_docker; then + for entry in ${envs[@]+"${envs[@]}"}; do + flags+=(-e "$entry") + done + + docker compose exec -T ${flags[@]+"${flags[@]}"} web php "$@" + + return + fi + + # Every script here is written against the board directory, and the local + # runner has no working directory of its own to inherit. + ( cd "$BOARD_DIR" && env ${envs[@]+"${envs[@]}"} "$PHP_BIN" "$@" ) +} + +run_php() { + run_php_env -- "$@" +} + +# Runs an arbitrary command where the forum lives, rather than only php. +# +# A leading `php` becomes $PHP_BIN locally, so a call site reads the same in +# both modes. It also means the vendor binaries can be invoked as +# `run_cmd php vendor/bin/whatever`, which works on Windows, where their +# shebang line does not. +run_cmd() { + if is_docker; then + docker compose exec -T web "$@" + + return + fi + + if [ "${1:-}" = 'php' ]; then + shift + + ( cd "$BOARD_DIR" && "$PHP_BIN" "$@" ) + + return + fi + + ( cd "$BOARD_DIR" && "$@" ) +} + +# The board directory as the PHP started by run_php sees it, which is not +# $BOARD_DIR under docker. Anything handing an absolute path to that PHP has to +# go through this. +run_board_dir() { + if is_docker; then + echo '/var/www/html' + else + echo "$BOARD_DIR" + fi +} + +# Runs SQL as the forum's own database user and prints any rows, tab separated. +# +# Under docker that is the client inside the database container. Locally it is +# .dev/db.php, which goes through mysqli or pgsql: those are what SMF itself +# connects with, so a machine that can run the forum can already do this, while +# the command line clients would be a dependency nothing else here needs. +db_sql() { + local engine service sql + engine=$(engine_smf_type "$1") || return 1 + service=$(engine_service "$1") + sql="$2" + + if is_docker; then + if [ "$engine" = 'mysql' ]; then + docker compose exec -T -e MYSQL_PWD="$DB_PASSWORD" "$service" \ + mysql -u"$DB_USER" -D "$DB_NAME" -N -B -e "$sql" + else + docker compose exec -T "$service" \ + psql -U "$DB_USER" -d "$DB_NAME" -tAX -c "$sql" + fi + + return + fi + + ( cd "$BOARD_DIR" && "$PHP_BIN" .dev/db.php \ + --engine "$engine" \ + --server "$(engine_server "$engine")" \ + --port "$(engine_port "$engine")" \ + --database "$DB_NAME" \ + --user "$DB_USER" \ + --password "$DB_PASSWORD" \ + --sql "$sql" ) +} + +# Empties an engine's database, which is the one thing the two runners cannot do +# the same way. +# +# Under docker the database is ours to destroy, so MySQL gets a real DROP +# DATABASE as root, which restores the character set along with everything else. +# Locally there is no reason to assume a root account exists or that the forum's +# user may create databases, so the tables go and the database itself stays. +# PostgreSQL has always worked the second way: dropping the schema takes the +# tables, sequences and functions with it, and smf owns it. +db_empty() { + local engine service + engine=$(engine_smf_type "$1") || return 1 + service=$(engine_service "$1") + + if [ "$engine" = 'postgresql' ]; then + if is_docker; then + docker compose exec -T "$service" psql -v ON_ERROR_STOP=1 -q -U "$DB_USER" -d "$DB_NAME" -c ' + DROP SCHEMA IF EXISTS public CASCADE; + CREATE SCHEMA public; + ' >/dev/null + else + db_sql "$engine" 'DROP SCHEMA IF EXISTS public CASCADE; CREATE SCHEMA public;' >/dev/null + fi + + return + fi + + if is_docker; then + # As root: the smf user has rights on the smf database but cannot drop + # and recreate it. utf8mb4 matches what compose.yaml asks the server for + # and what SMF's own DDL emits. + docker compose exec -T -e MYSQL_PWD="$DB_ROOT_PASSWORD" "$service" mysql -uroot -e " + DROP DATABASE IF EXISTS \`${DB_NAME}\`; + CREATE DATABASE \`${DB_NAME}\` CHARACTER SET utf8mb4; + GRANT ALL ON \`${DB_NAME}\`.* TO '${DB_USER}'@'%'; + " + + return + fi + + # The names are collected first and the DROP is built from them here, rather + # than in SQL: doing it in one statement means GROUP_CONCAT, whose result is + # cut off at group_concat_max_len -- 1024 bytes by default, which SMF's 70-odd + # tables pass comfortably. That truncation is silent, so it produces a reset + # that drops nothing and reports success. + local tables + tables=$(db_sql "$engine" " + SELECT CONCAT('\`', table_name, '\`') FROM information_schema.tables + WHERE table_schema = DATABASE(); + " | tr -d '\r' | paste -sd, -) + + [ -n "$tables" ] || return 0 + + # Foreign keys are off for the duration: the order the tables come back in is + # not an order they can be dropped in. + db_sql "$engine" " + SET FOREIGN_KEY_CHECKS = 0; + DROP TABLE ${tables}; + SET FOREIGN_KEY_CHECKS = 1; + " >/dev/null +} + +# ------------------------------------------------------- the local environment +# Says what is missing before something else fails in a way that reads like a +# broken script rather than a machine that is not set up for this. +# +# The engine is optional: without one this only checks that there is a php to +# run, which is all a caller that has not chosen an engine yet can know. +require_local_deps() { + local engine extension + + is_docker && return 0 + + command -v "$PHP_BIN" >/dev/null 2>&1 \ + || die "no php on PATH. Set PHP_BIN to one, or pass --docker to use the compose stack" + + [ -n "${1:-}" ] || return 0 + + engine=$(engine_smf_type "$1") || return 1 + + if [ "$engine" = 'mysql' ]; then + extension='mysqli' + else + extension='pgsql' + fi + + "$PHP_BIN" -r "exit(extension_loaded('${extension}') ? 0 : 1);" \ + || die "${PHP_BIN} has no ${extension} extension, which SMF needs for ${engine}. Enable it, or pass --docker to use the compose stack" +} + +# Puts a Settings.php and an install.php in place, so the installer has somewhere +# to write and something to run. +# +# Under docker the entrypoint does this on boot, so the job is to restart the web +# container pointed at the right engine and wait for it. Locally there is no +# entrypoint, so the same work happens here. Keep the two in step: +# .docker/php/entrypoint.sh, under "installer bits". +stage_installer() { + local smf_type path waited + + smf_type=$(engine_smf_type "$1") || return 1 + + if is_docker; then + SMF_DB_TYPE="$smf_type" docker compose up -d web >/dev/null + + # The entrypoint waits for the database before it writes anything, so + # give it a moment to get there rather than racing whatever runs next. + for waited in $(seq 1 60); do + if docker compose exec -T web test -f install.php 2>/dev/null; then + return 0 + fi + + sleep 1 + done + + die 'timed out waiting for the entrypoint to stage install.php (docker compose logs web)' + fi + + sed \ + -e "s|^\$db_type = 'mysql';|\$db_type = '${smf_type}';|" \ + -e "s|^\$db_port = 0;|\$db_port = $(engine_port "$smf_type");|" \ + -e "s|^\$db_server = 'localhost';|\$db_server = '$(engine_server "$smf_type")';|" \ + -e "s|^\$db_name = 'smf';|\$db_name = '${DB_NAME}';|" \ + -e "s|^\$db_user = 'root';|\$db_user = '${DB_USER}';|" \ + -e "s|^\$db_passwd = '';|\$db_passwd = '${DB_PASSWORD}';|" \ + -e "s|^\$boardurl = 'http://127.0.0.1/smf';|\$boardurl = '${SMF_BOARDURL}';|" \ + "$BOARD_DIR/other/Settings.php" > "$BOARD_DIR/Settings.php" + + cp "$BOARD_DIR/other/Settings_bak.php" "$BOARD_DIR/Settings_bak.php" + cp "$BOARD_DIR/other/install.php" "$BOARD_DIR/install.php" + + # The installer refuses to continue unless all of these are writable, and the + # forum needs them at runtime too. They are all in the checkout already, + # except the cache's copy of db_last_error.php. + for path in attachments avatars custom_avatar cache Packages Smileys Themes Languages; do + [ -e "$BOARD_DIR/$path" ] || mkdir -p "$BOARD_DIR/$path" + done + + [ -f "$BOARD_DIR/cache/db_last_error.php" ] || cp "$BOARD_DIR/db_last_error.php" "$BOARD_DIR/cache/db_last_error.php" 2>/dev/null || true +} + +# ----------------------------------------------------------------- the server +# Under docker Apache is already serving the checkout. Locally the HTTP tests +# need something answering on the board URL, and PHP's own server is enough: SMF +# routes on the query string and on PATH_INFO, and there is no .htaccess at the +# root, so nothing here wants mod_rewrite. It is not Apache, though, which is +# why the compose stack stays the closer thing to production. +SERVE_PID='' + +serve_start() { + local host port waited + + is_docker && return 0 + + # Idempotent, so a caller looping over both engines can ask on each pass + # without having to remember whether it already has one. + [ -z "$SERVE_PID" ] || return 0 + + host=$(printf '%s' "$SMF_BOARDURL" | sed -E 's|^[a-z]+://||; s|[:/].*$||') + port=$(printf '%s' "$SMF_BOARDURL" | sed -nE 's|^[a-z]+://[^:/]+:([0-9]+).*$|\1|p') + port="${port:-80}" + + # A single-threaded server deadlocks on a page that asks itself for + # something. Nothing in the suite does that, but the cost of not finding out + # the hard way is one variable. Windows has no forking and ignores it. + PHP_CLI_SERVER_WORKERS=4 "$PHP_BIN" -d memory_limit=512M \ + -S "${host}:${port}" -t "$BOARD_DIR" >/dev/null 2>&1 & + + SERVE_PID=$! + + for waited in $(seq 1 30); do + # Asked through PHP rather than curl, which is not a binary every machine + # that can run the forum has. + if "$PHP_BIN" -r "exit(@fsockopen('${host}', ${port}, \$e, \$s, 2) ? 0 : 1);" 2>/dev/null; then + log "serving ${BOARD_DIR} at ${SMF_BOARDURL} (pid ${SERVE_PID})" + + return 0 + fi + + # Nothing is going to start listening if the process has already gone. + kill -0 "$SERVE_PID" 2>/dev/null \ + || die "the built-in server exited immediately; is ${host}:${port} already in use?" + + sleep 1 + done + + die "the built-in server did not answer on ${SMF_BOARDURL} after ${waited}s" +} + +serve_stop() { + [ -n "$SERVE_PID" ] || return 0 + + kill "$SERVE_PID" 2>/dev/null || true + wait "$SERVE_PID" 2>/dev/null || true + + SERVE_PID='' +} diff --git a/.docker/rerun-upgrade.sh b/.dev/rerun-upgrade.sh similarity index 85% rename from .docker/rerun-upgrade.sh rename to .dev/rerun-upgrade.sh index 7c5bfcf9d60..7cf502ac9f6 100644 --- a/.docker/rerun-upgrade.sh +++ b/.dev/rerun-upgrade.sh @@ -2,8 +2,8 @@ # Upgrades a 2.1 database to 3.0, then upgrades it again, and reports where the # second run changed anything. # -# .docker/rerun-upgrade.sh --engine mysql --baseline ../SMF-2.1/.docker/baseline/artifacts/2.1.7-1/small/mysql.sql -# .docker/rerun-upgrade.sh --engine postgresql --baseline ../SMF-2.1/.docker/baseline/artifacts/2.1.7-1/small/postgres.sql +# .dev/rerun-upgrade.sh --engine mysql --baseline ../SMF-2.1/.docker/baseline/artifacts/2.1.7-1/small/mysql.sql +# .dev/rerun-upgrade.sh --engine postgresql --baseline ../SMF-2.1/.docker/baseline/artifacts/2.1.7-1/small/postgres.sql # # Running the upgrader twice is not an unusual thing to do. It is what happens # when an admin refreshes a page that timed out, when a run dies part way and is @@ -30,11 +30,19 @@ # Runs on the host. Expect five to ten minutes per engine. set -euo pipefail +# Orchestrating containers is all this can mean, so it says so rather than +# inheriting the default and failing further in. Read by lib.sh, which is +# sourced below and which a linter reading this file alone cannot see. +# shellcheck disable=SC2034 +SMF_RUNNER=docker + . "$(dirname -- "${BASH_SOURCE[0]}")/lib.sh" +require_docker + ENGINE='' BASELINE='' -OUT="$DOCKER_DIR/rerun" +OUT="$DEV_DIR/rerun" while [ $# -gt 0 ]; do case "$1" in @@ -58,7 +66,7 @@ cd "$BOARD_DIR" mkdir -p "$OUT" OUT=$(cd -- "$OUT" && pwd) -. "$DOCKER_DIR/upgrade-readings.sh" +. "$DEV_DIR/upgrade-readings.sh" rerun_one() { local smf_type="$1" version status=0 @@ -66,7 +74,7 @@ rerun_one() { : > "$OUT/report-${smf_type}.txt" log "${smf_type}: emptying the database" - "$DOCKER_DIR/reset.sh" --engine "$smf_type" >/dev/null + "$DEV_DIR/reset.sh" --docker --engine "$smf_type" >/dev/null log "${smf_type}: loading ${BASELINE##*/}" load_baseline "$smf_type" "$BASELINE" diff --git a/.dev/reset.sh b/.dev/reset.sh new file mode 100755 index 00000000000..7381e759eb6 --- /dev/null +++ b/.dev/reset.sh @@ -0,0 +1,81 @@ +#!/usr/bin/env bash +# Returns things to "installable": no forum, an empty database for the chosen +# engine, and a Settings.php regenerated for it. +# +# .dev/reset.sh --engine mysql +# .dev/reset.sh --engine postgresql +# .dev/reset.sh --docker --engine mysql +# +# This is also how you move an install between engines. Settings.php pins one +# engine and wins over SMF_DB_TYPE, so switching means throwing it away and +# writing a new one. To keep an install rather than discard it, use +# use-engine.sh instead. +# +# Only the chosen engine's database is touched, so a MySQL reset can never +# disturb a PostgreSQL install or vice versa. +# +# Runs on the host. +set -euo pipefail + +. "$(dirname -- "${BASH_SOURCE[0]}")/lib.sh" + +parse_runner_args "$@" + +ENGINE='' +KEEP_FILES=0 + +while [ $# -gt 0 ]; do + case "$1" in + --engine) ENGINE="$2"; shift 2 ;; + --engine=*) ENGINE="${1#*=}"; shift ;; + --keep-files) KEEP_FILES=1; shift ;; + --docker|--local) shift ;; + -h|--help) sed -n '2,17p' "${BASH_SOURCE[0]}"; exit 0 ;; + *) die "unknown argument: $1" ;; + esac +done + +[ -n "$ENGINE" ] || die 'need --engine mysql|postgresql' +SERVICE=$(engine_service "$ENGINE") || die "unknown engine: $ENGINE" +SMF_TYPE=$(engine_smf_type "$ENGINE") + +require_local_deps "$SMF_TYPE" + +cd "$BOARD_DIR" + +log "resetting for ${SMF_TYPE}" + +# ------------------------------------------------------------------ the forum +# Stop the web container first: Apache holding a half-installed forum open while +# its database vanishes underneath produces confusing errors in the log. The +# local server belongs to whoever started it and is left alone. +if is_docker; then + docker compose stop web >/dev/null 2>&1 || true +fi + +rm -f Settings.php Settings_bak.php install.php upgrade.php + +# SMF's cache holds a serialised copy of $modSettings, which would otherwise +# outlive the database it describes. +find cache -type f ! -name 'index.php' ! -name '.htaccess' -delete 2>/dev/null || true + +if [ "$KEEP_FILES" -eq 0 ]; then + for dir in attachments custom_avatar; do + find "$dir" -type f ! -name 'index.php' ! -name '.htaccess' ! -name 'blank.png' -delete 2>/dev/null || true + done + rm -f Packages/installed.list +fi + +# --------------------------------------------------------------- the database +if is_docker; then + docker compose up -d "$SERVICE" >/dev/null +fi + +db_empty "$SMF_TYPE" + +log "${SMF_TYPE} database ${DB_NAME} is empty" + +# ------------------------------------------------------------------ the installer +stage_installer "$SMF_TYPE" + +log 'installer staged, ready to install' diff --git a/.docker/schema-tool.php b/.dev/schema-tool.php similarity index 98% rename from .docker/schema-tool.php rename to .dev/schema-tool.php index 9cca9cc3cfb..5865d1965b0 100644 --- a/.docker/schema-tool.php +++ b/.dev/schema-tool.php @@ -13,8 +13,8 @@ * Two modes, and they are separate on purpose: only one engine can be live at * a time, so the two readings cannot be taken in one process. * - * php .docker/schema-tool.php dump --engine mysql --db smf > fresh.json - * php .docker/schema-tool.php diff fresh.json upgraded.json + * php .dev/schema-tool.php dump --engine mysql --db smf > fresh.json + * php .dev/schema-tool.php diff fresh.json upgraded.json * * Runs inside the web container, and talks to the database directly rather * than through SMF. Nothing here loads Settings.php or boots the forum: the @@ -54,8 +54,8 @@ function usage(): int fwrite(STDERR, <<<'TEXT' Reads the shape of an SMF database, and compares two of those readings. - php .docker/schema-tool.php dump --engine mysql --db smf > fresh.json - php .docker/schema-tool.php diff fresh.json upgraded.json + php .dev/schema-tool.php dump --engine mysql --db smf > fresh.json + php .dev/schema-tool.php diff fresh.json upgraded.json dump options, with the defaults compose.yaml gives: diff --git a/.docker/test.sh b/.dev/test.sh similarity index 51% rename from .docker/test.sh rename to .dev/test.sh index 7ea104629e9..50145274082 100755 --- a/.docker/test.sh +++ b/.dev/test.sh @@ -1,15 +1,16 @@ #!/usr/bin/env bash # Runs the test suite against a real forum, on one engine or on both. # -# .docker/test.sh both engines, whole suite -# .docker/test.sh --engine postgresql -# .docker/test.sh --engine both --filter ModSettings +# .dev/test.sh both engines, whole suite +# .dev/test.sh --engine postgresql +# .dev/test.sh --engine both --filter ModSettings +# .dev/test.sh --docker --engine mysql # # Anything after the recognised options is handed straight to PHPUnit, so # --filter, --testsuite and friends work as usual. # # Installs a forum for an engine that has not got one yet. Use -# .docker/install-forum.sh --force to start any of them over. +# .dev/install-forum.sh --force to start any of them over. # # Running on both engines is the point rather than a thoroughness exercise: the # two disagree often enough that a suite which only ever sees one of them @@ -22,6 +23,8 @@ set -euo pipefail . "$(dirname -- "${BASH_SOURCE[0]}")/lib.sh" +parse_runner_args "$@" + ENGINE='both' PHPUNIT_ARGS=() @@ -29,13 +32,18 @@ while [ $# -gt 0 ]; do case "$1" in --engine) ENGINE="$2"; shift 2 ;; --engine=*) ENGINE="${1#*=}"; shift ;; - -h|--help) sed -n '2,19p' "${BASH_SOURCE[0]}"; exit 0 ;; + --docker|--local) shift ;; + -h|--help) sed -n '2,20p' "${BASH_SOURCE[0]}"; exit 0 ;; *) PHPUNIT_ARGS+=("$1"); shift ;; esac done ENGINES=$(engine_list "$ENGINE") || die "unknown engine: $ENGINE" +for smf_type in $ENGINES; do + require_local_deps "$smf_type" +done + cd "$BOARD_DIR" # Remember what was active, and put it back afterwards however this ends. A test @@ -46,27 +54,52 @@ if [ -f Settings.php ]; then ORIGINAL=$(sed -n "s|^\$db_type = '\([^']*\)';.*|\1|p" Settings.php | head -n 1 | tr '[:upper:]' '[:lower:]') fi -restore_engine() { +cleanup() { + serve_stop + if [ -n "$ORIGINAL" ] && [ -f "$SETTINGS_DIR/Settings.$(engine_smf_type "$ORIGINAL").php" ]; then - "$DOCKER_DIR/use-engine.sh" "$ORIGINAL" >/dev/null 2>&1 || true + "$DEV_DIR/use-engine.sh" "--${SMF_RUNNER}" "$ORIGINAL" >/dev/null 2>&1 || true fi } -trap restore_engine EXIT +trap cleanup EXIT FAILED='' for smf_type in $ENGINES; do if [ -z "$(installed_version "$smf_type" || true)" ]; then log "${smf_type}: no forum yet, installing one" - "$DOCKER_DIR/install-forum.sh" --engine "$smf_type" >/dev/null + "$DEV_DIR/install-forum.sh" "--${SMF_RUNNER}" --engine "$smf_type" >/dev/null fi - "$DOCKER_DIR/use-engine.sh" "$smf_type" >/dev/null + "$DEV_DIR/use-engine.sh" "--${SMF_RUNNER}" "$smf_type" >/dev/null + + # One server serves both engines: it reads Settings.php per request, and + # use-engine.sh has just rewritten it. Started after the first install rather + # than before it, so it never has a half-built forum underneath it. + serve_start log "${smf_type}: running the tests" - if docker compose exec -T web vendor/bin/phpunit --no-coverage --colors=always "${PHPUNIT_ARGS[@]+"${PHPUNIT_ARGS[@]}"}"; then + # The HTTP tests sign in, so they need to be told who the administrator is. + # These default to what install-forum.sh created; export them to point the + # suite at a forum that was set up some other way. + # + # The base URL is only named locally. Under docker the suite runs inside the + # web container, where the forum answers on port 80 rather than on the + # published port the board URL names, and HttpClient works that out for + # itself. + TEST_ENV=( + SMF_ADMIN_USER="$SMF_ADMIN_USER" + SMF_ADMIN_PASS="$SMF_ADMIN_PASS" + ) + + if ! is_docker; then + TEST_ENV+=(SMF_TESTS_BASE_URL="$SMF_BOARDURL") + fi + + if run_php_env "${TEST_ENV[@]}" -- vendor/bin/phpunit \ + --no-coverage --colors=always "${PHPUNIT_ARGS[@]+"${PHPUNIT_ARGS[@]}"}"; then log "${smf_type}: passed" else warn "${smf_type}: FAILED" diff --git a/.docker/upgrade-readings.sh b/.dev/upgrade-readings.sh similarity index 100% rename from .docker/upgrade-readings.sh rename to .dev/upgrade-readings.sh diff --git a/.docker/use-engine.sh b/.dev/use-engine.sh similarity index 75% rename from .docker/use-engine.sh rename to .dev/use-engine.sh index 5fd2b7a96b5..f0676a70103 100755 --- a/.docker/use-engine.sh +++ b/.dev/use-engine.sh @@ -1,8 +1,8 @@ #!/usr/bin/env bash # Switches which installed forum is live, without reinstalling either. # -# .docker/use-engine.sh mysql -# .docker/use-engine.sh postgresql +# .dev/use-engine.sh mysql +# .dev/use-engine.sh postgresql # # Both database services always run, on separate volumes, so each keeps its own # forum. What decides which one you get is Settings.php: it pins $db_type, and @@ -19,18 +19,24 @@ set -euo pipefail . "$(dirname -- "${BASH_SOURCE[0]}")/lib.sh" -[ $# -eq 1 ] || die 'usage: use-engine.sh mysql|postgresql' +parse_runner_args "$@" -case "$1" in +# Nothing here talks to a container, but installed_version() below does, so the +# runner still has to be settled before the engine name is read. +mapfile -t ARGS < <(without_runner_args "$@") + +[ "${#ARGS[@]}" -eq 1 ] || die 'usage: use-engine.sh [--docker] mysql|postgresql' + +case "${ARGS[0]}" in -h|--help) sed -n '2,16p' "${BASH_SOURCE[0]}"; exit 0 ;; esac -SMF_TYPE=$(engine_smf_type "$1") || die "unknown engine: $1" +SMF_TYPE=$(engine_smf_type "${ARGS[0]}") || die "unknown engine: ${ARGS[0]}" SAVED="$SETTINGS_DIR/Settings.${SMF_TYPE}.php" cd "$BOARD_DIR" -[ -f "$SAVED" ] || die "no saved settings for ${SMF_TYPE} -- run .docker/install-forum.sh --engine ${SMF_TYPE}" +[ -f "$SAVED" ] || die "no saved settings for ${SMF_TYPE} -- run .dev/install-forum.sh --engine ${SMF_TYPE}" cp "$SAVED" Settings.php cp "$SETTINGS_DIR/Settings_bak.${SMF_TYPE}.php" Settings_bak.php diff --git a/.docker/user.sh b/.dev/user.sh similarity index 83% rename from .docker/user.sh rename to .dev/user.sh index eebf4d1a8e5..f93c20ca480 100755 --- a/.docker/user.sh +++ b/.dev/user.sh @@ -2,10 +2,10 @@ # Looks at forum accounts and fixes their passwords, so "which password did this # forum end up with?" does not turn into a session of hand written SQL. # -# .docker/user.sh list -# .docker/user.sh check admin 'password' -# .docker/user.sh reset admin 'a new password' -# .docker/user.sh check admin 'password' --engine postgresql +# .dev/user.sh list +# .dev/user.sh check admin 'password' +# .dev/user.sh reset admin 'a new password' +# .dev/user.sh check admin 'password' --engine postgresql # # check exits 0 when SMF would accept the password and 1 when it would not, so # it is usable in a conditional as well as by eye. @@ -24,6 +24,8 @@ set -euo pipefail . "$(dirname -- "${BASH_SOURCE[0]}")/lib.sh" +parse_runner_args "$@" + ENGINE='' ACTION='' NAME='' @@ -34,6 +36,7 @@ while [ $# -gt 0 ]; do case "$1" in --engine) ENGINE="$2"; shift 2 ;; --engine=*) ENGINE="${1#*=}"; shift ;; + --docker|--local) shift ;; -h|--help) sed -n '2,22p' "${BASH_SOURCE[0]}"; exit 0 ;; -*) die "unknown argument: $1" ;; *) POSITIONAL+=("$1"); shift ;; @@ -55,17 +58,20 @@ case "$ACTION" in *) die "unknown action: ${ACTION} (expected list, check or reset)" ;; esac -# The settings file to read, as the container sees it. Empty means "whichever -# forum is live", which is the common case and needs no explanation in the log. -SETTINGS='/var/www/html/Settings.php' +require_local_deps "$ENGINE" + +# The settings file to read, spelled the way the php that reads it will see it. +# Whichever forum is live is the common case and needs no explanation in the log. +RUN_BOARD_DIR=$(run_board_dir) +SETTINGS="${RUN_BOARD_DIR}/Settings.php" if [ -n "$ENGINE" ]; then SMF_TYPE=$(engine_smf_type "$ENGINE") || die "unknown engine: $ENGINE" - SAVED="$DOCKER_DIR/settings/Settings.${SMF_TYPE}.php" + SAVED="$SETTINGS_DIR/Settings.${SMF_TYPE}.php" [ -f "$SAVED" ] || die "no saved settings for ${SMF_TYPE}; install it first with install-forum.sh --engine ${SMF_TYPE}" - SETTINGS="/var/www/html/.docker/settings/Settings.${SMF_TYPE}.php" + SETTINGS="${RUN_BOARD_DIR}/.dev/settings/Settings.${SMF_TYPE}.php" fi cd "$BOARD_DIR" @@ -73,18 +79,19 @@ cd "$BOARD_DIR" # The password goes through the environment rather than the argument list: # arguments are visible to anything that can read the process table, and a # password typed at a shell is quite enough exposure already. -docker compose exec -T \ - -e SMF_USER_ACTION="$ACTION" \ - -e SMF_USER_NAME="$NAME" \ - -e SMF_USER_PASSWORD="$PASSWORD" \ - -e SMF_USER_SETTINGS="$SETTINGS" \ - web php <<-'PHP' +run_php_env \ + SMF_USER_ACTION="$ACTION" \ + SMF_USER_NAME="$NAME" \ + SMF_USER_PASSWORD="$PASSWORD" \ + SMF_USER_SETTINGS="$SETTINGS" \ + SMF_USER_BOARD_DIR="$RUN_BOARD_DIR" \ + -- <<-'PHP' &2; } -die() { printf '[smf-dev] error: %s\n' "$*" >&2; exit 1; } - -# Engine name normalisation. Everything downstream uses either the SMF type -# ('mysql' / 'postgresql') or the compose service name ('mysql' / 'postgres'), -# and mixing them up is an easy way to waste an afternoon. -engine_smf_type() { - case "$1" in - mysql|mysqli|mariadb) echo 'mysql' ;; - postgres|postgresql|pgsql) echo 'postgresql' ;; - *) return 1 ;; - esac -} - -engine_service() { - case "$1" in - mysql|mysqli|mariadb) echo 'mysql' ;; - postgres|postgresql|pgsql) echo 'postgres' ;; - *) return 1 ;; - esac -} - -# Container-internal host and port for an engine. Not the host-side ports in -# compose.yaml: these are what Settings.php has to contain. -engine_server() { - case "$(engine_smf_type "$1")" in - mysql) echo "${SMF_MYSQL_SERVER:-mysql}" ;; - postgresql) echo "${SMF_POSTGRES_SERVER:-postgres}" ;; - *) return 1 ;; - esac -} - -engine_port() { - case "$(engine_smf_type "$1")" in - mysql) echo "${SMF_MYSQL_PORT:-3306}" ;; - postgresql) echo "${SMF_POSTGRES_PORT:-5432}" ;; - *) return 1 ;; - esac -} - -# Expands "both" into the engines to act on, in the order they run. Only one -# engine can be live at a time -- Settings.php pins $db_type and Db::load() -# early-returns once the connection exists -- so "both" is a sequential chain, -# never two connections. -engine_list() { - case "$1" in - both|all) echo 'mysql postgresql' ;; - *) engine_smf_type "$1" ;; - esac -} - -# The installed version for one engine, empty if the forum is not installed. -# Asks the database directly rather than trusting the presence of a file: -# Settings.php exists from the moment the entrypoint writes it, long before -# there is a forum behind it. -installed_version() { - local engine service - engine=$(engine_smf_type "$1") || return 1 - service=$(engine_service "$1") - - if [ "$engine" = 'mysql' ]; then - docker compose exec -T -e MYSQL_PWD="$DB_PASSWORD" "$service" \ - mysql -u"$DB_USER" -D "$DB_NAME" -N -B -e \ - "SELECT value FROM ${DB_PREFIX}settings WHERE variable = 'smfVersion';" 2>/dev/null - else - docker compose exec -T "$service" \ - psql -U "$DB_USER" -d "$DB_NAME" -tAX -c \ - "SELECT value FROM ${DB_PREFIX}settings WHERE variable = 'smfVersion';" 2>/dev/null - fi -} diff --git a/.docker/reset.sh b/.docker/reset.sh deleted file mode 100755 index 00f2de5a404..00000000000 --- a/.docker/reset.sh +++ /dev/null @@ -1,99 +0,0 @@ -#!/usr/bin/env bash -# Returns the stack to "installable": no forum, an empty database for the chosen -# engine, and a Settings.php regenerated for it. -# -# .docker/reset.sh --engine mysql -# .docker/reset.sh --engine postgresql -# -# This is also how you move an install between engines. Settings.php pins one -# engine and wins over SMF_DB_TYPE, so switching means throwing it away and -# letting the entrypoint write a new one. To keep an install rather than -# discard it, use use-engine.sh instead. -# -# Only the chosen engine's database is touched. The two engines keep separate -# volumes, so a MySQL reset can never disturb a PostgreSQL install or vice -# versa. -# -# Runs on the host. -set -euo pipefail - -. "$(dirname -- "${BASH_SOURCE[0]}")/lib.sh" - -ENGINE='' -KEEP_FILES=0 - -while [ $# -gt 0 ]; do - case "$1" in - --engine) ENGINE="$2"; shift 2 ;; - --engine=*) ENGINE="${1#*=}"; shift ;; - --keep-files) KEEP_FILES=1; shift ;; - -h|--help) sed -n '2,17p' "${BASH_SOURCE[0]}"; exit 0 ;; - *) die "unknown argument: $1" ;; - esac -done - -[ -n "$ENGINE" ] || die 'need --engine mysql|postgresql' -SERVICE=$(engine_service "$ENGINE") || die "unknown engine: $ENGINE" -SMF_TYPE=$(engine_smf_type "$ENGINE") - -cd "$BOARD_DIR" - -log "resetting for ${SMF_TYPE}" - -# ------------------------------------------------------------------ the forum -# Stop the web container first: Apache holding a half-installed forum open while -# its database vanishes underneath produces confusing errors in the log. -docker compose stop web >/dev/null 2>&1 || true - -rm -f Settings.php Settings_bak.php install.php upgrade.php - -# SMF's cache holds a serialised copy of $modSettings, which would otherwise -# outlive the database it describes. -find cache -type f ! -name 'index.php' ! -name '.htaccess' -delete 2>/dev/null || true - -if [ "$KEEP_FILES" -eq 0 ]; then - for dir in attachments custom_avatar; do - find "$dir" -type f ! -name 'index.php' ! -name '.htaccess' ! -name 'blank.png' -delete 2>/dev/null || true - done - rm -f Packages/installed.list -fi - -# --------------------------------------------------------------- the database -docker compose up -d "$SERVICE" >/dev/null - -if [ "$SMF_TYPE" = 'mysql' ]; then - # As root: the smf user has rights on the smf database but cannot drop and - # recreate it. utf8mb4 matches what compose.yaml asks the server for and - # what SMF's own DDL emits. - docker compose exec -T -e MYSQL_PWD="$DB_ROOT_PASSWORD" "$SERVICE" mysql -uroot -e " - DROP DATABASE IF EXISTS \`${DB_NAME}\`; - CREATE DATABASE \`${DB_NAME}\` CHARACTER SET utf8mb4; - GRANT ALL ON \`${DB_NAME}\`.* TO '${DB_USER}'@'%'; - " -else - # The database itself cannot be dropped while we are connected to it, and - # dropping the schema is enough: it takes the tables, sequences, functions - # and operators with it. smf owns the database, so it may recreate public. - docker compose exec -T "$SERVICE" psql -v ON_ERROR_STOP=1 -q -U "$DB_USER" -d "$DB_NAME" -c ' - DROP SCHEMA IF EXISTS public CASCADE; - CREATE SCHEMA public; - ' >/dev/null -fi - -log "${SMF_TYPE} database ${DB_NAME} is empty" - -# Bring web back up so the entrypoint regenerates Settings.php for this engine -# and stages the installer. -SMF_DB_TYPE="$SMF_TYPE" docker compose up -d web >/dev/null - -# The entrypoint waits for the database before it writes anything, so give it a -# moment to get there rather than racing whatever runs next. -for _ in $(seq 1 60); do - if docker compose exec -T web test -f install.php 2>/dev/null; then - log 'installer staged, ready to install' - exit 0 - fi - sleep 1 -done - -die 'timed out waiting for the entrypoint to stage install.php (docker compose logs web)' diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index fa91b421f9b..61a97758b51 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -42,5 +42,104 @@ jobs: if: steps.composer-cache.outputs.cache-hit != 'true' run: composer install --prefer-dist --no-progress --ansi + # Only the unit suite, so this job reports what it ran. The integration + # tests skip themselves when there is no forum, and counting those skips + # as part of a passing run reads as coverage that is not there. - name: Run the unit tests - run: composer test -- --colors=always + run: composer test-unit -- --colors=always + + integration: + name: Integration tests (${{ matrix.engine }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + # Both, because the two engines disagree. The counter regression in + # tests/Integration/ModSettingsTest.php passes on MySQL with the bug + # still in place and fails on PostgreSQL, so one engine would have + # proved nothing there. + engine: [ mysql, postgresql ] + + # Linux containers, so this job cannot also run on windows-latest the way + # the unit job does. Windows is covered there, and .dev/test.sh is what runs + # these by hand on a machine without Docker. + services: + mysql: + image: mysql:8.4 + env: + MYSQL_DATABASE: smf + MYSQL_USER: smf + MYSQL_PASSWORD: smf + MYSQL_ROOT_PASSWORD: smf + ports: + - 3306:3306 + options: >- + --health-cmd="mysqladmin ping -h 127.0.0.1 --silent" + --health-interval=3s + --health-timeout=5s + --health-retries=40 + --health-start-period=20s + + postgres: + image: postgres:17-alpine + env: + POSTGRES_DB: smf + POSTGRES_USER: smf + POSTGRES_PASSWORD: smf + ports: + - 5432:5432 + options: >- + --health-cmd="pg_isready -U smf -d smf" + --health-interval=3s + --health-timeout=3s + --health-retries=20 + + env: + # What .dev/lib.sh defaults to anyway, named here because the forum is + # installed with this board URL baked into it and the built-in server has + # to answer on the same port. + WEB_PORT: 8080 + + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 #7.0.1 + + - name: Setup PHP + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 #2.37.2 + with: + php-version: '8.4' + # What .docker/php/Dockerfile installs, so the two environments agree + # about what the forum has available. mysqli and pgsql are what + # .dev/db.php connects with; curl is what the HTTP tests drive. + extensions: mysqli, pgsql, mbstring, fileinfo, gd, intl, curl, exif, xsl, zip + coverage: none + + - name: Cache Composer packages + id: composer-cache + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 #6.1.0 + with: + path: vendor + key: ${{ runner.os }}-php8.4-${{ hashFiles('**/composer.lock') }} + restore-keys: ${{ runner.os }}-php8.4- + + - name: Install dependencies + if: steps.composer-cache.outputs.cache-hit != 'true' + run: composer install --prefer-dist --no-progress --ansi + + - name: Install a forum on ${{ matrix.engine }} + run: .dev/install-forum.sh --engine ${{ matrix.engine }} + + # --fail-on-skipped is the point of the job. Both the missing-forum skip in + # IntegrationTestCase and the wrong-password skip in HttpTestCase are + # there so the suite stays usable on a machine with no forum; in CI they + # would turn a job that tested nothing into a green tick. + - name: Run the integration tests + run: .dev/test.sh --engine ${{ matrix.engine }} --testsuite integration --fail-on-skipped + + # SMF records most of what goes wrong here rather than showing it, and a + # failure that never reached an assertion leaves its explanation in this + # table and nowhere else. + - name: Show the forum's error log + if: failure() + run: | + php .dev/db.php --engine ${{ matrix.engine }} --sql \ + "SELECT id_error, error_type, url, message FROM smf_log_errors ORDER BY id_error DESC LIMIT 20;" || true diff --git a/.gitignore b/.gitignore index d68f074d24d..13cf9506e40 100644 --- a/.gitignore +++ b/.gitignore @@ -81,12 +81,12 @@ Thumbs.db # One saved Settings.php per engine, so use-engine.sh can switch between two # installs without reinstalling. Generated secrets and a machine-specific # board URL: local to whoever ran the installer. -/.docker/settings/ -/.docker/rerun/ -/.docker/interrupt/ +/.dev/settings/ +/.dev/rerun/ +/.dev/interrupt/ # Schema readings and reports from compare-upgrade.sh. They describe one # machine's databases at one moment, and are rewritten on every run. -/.docker/compare/ +/.dev/compare/ # Test / Private files # ######################## diff --git a/AGENTS.md b/AGENTS.md index 06d399ffc8b..ea3b0c5ad5e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -171,13 +171,20 @@ failure, not a pass. things above: `Db::$db`, `Config::$modSettings` as the database holds it, and `User::$me`. ```bash -.docker/test.sh # both engines -.docker/test.sh --engine postgresql +.dev/test.sh # both engines +.dev/test.sh --engine postgresql +.dev/test.sh --docker # in the compose stack rather than on this machine ``` `composer test` still runs everything. When there is no forum to talk to the integration -tests **skip** rather than fail, so it stays useful on a machine with no Docker. To get -one: `.docker/install-forum.sh --engine mysql`. +tests **skip** rather than fail, so it stays useful on a machine with nothing installed. +To get one: `.dev/install-forum.sh --engine mysql`. + +**CI runs this suite on both engines, with `--fail-on-skipped`.** So a skip that is a +convenience locally is a failure there, and a change that breaks a page will turn a pull +request red rather than sliding through on a green unit run. `.github/workflows/phpunit.yml` +installs a forum on the runner the same way you would, which is why the scripts in `.dev/` +work with or without Docker: `SMF_RUNNER` picks, and it defaults to `local`. Extend `SMF\Tests\Integration\IntegrationTestCase`, which gives you: @@ -197,6 +204,29 @@ Two things the rollback does not cover: **DDL**, since MySQL commits implicitly `CREATE`/`ALTER`/`DROP`; and anything happening in another process, such as a request made over HTTP, which runs on its own connection. +#### HTTP tests + +`tests/Integration/Http/` drives the forum over the wire, through `HttpTestCase`. Use it +when the thing worth proving is that a *page* works: the session, the cookies, the theme +and the templates are all in the path, and none of them are otherwise reachable. + +They cannot use a transaction and do not try to - see `HttpTestCase::usesTransaction()` - +so a test that writes cleans up after itself. Four things about SMF make writing them +harder than it looks, all of them handled in the base class: + +- **Arrive at the forum before submitting anything.** The first request of a new session + regenerates it, so a security token minted on the very first page a visitor sees can + never be validated. The symptom is a 403 "Token verification failed" that looks like a + broken token rather than a replaced session. +- **Send the button you mean to press.** `HttpResponse::formFields()` deliberately leaves + buttons out. The posting form has both `preview` and `post`; submitting the pair means + preview wins, the post is never made, and the response is a perfectly ordinary 200. +- **Flood control will hit you.** `Security::spamProtection()` allows a moderator one + login or post every two seconds per IP, and tests are far faster than people. + `submitForm()` waits it out once rather than failing at random. +- **Quote `errorText()` in failure messages, not the body.** A fatal error in SMF is a + normal page, and its first few hundred characters are the menu. + **Run both engines.** This is not thoroughness for its own sake — the two disagree often enough to matter. `ModSettingsTest` pins a bug that *passes on MySQL with the bug still in place*, because MySQL silently coerces text to a number where PostgreSQL refuses. @@ -253,10 +283,11 @@ behaviour. What to watch for: ### Running the forum -The rest of CI only proves the code parses (`phplint` on 8.4 and 8.5) and is formatted. -So a fully green PR still tells you very little about whether a change works. Verify by -running the forum. The repository ships a Docker environment, documented in full in -`.docker/README.md`: +The rest of CI proves the code parses (`phplint` on 8.4 and 8.5), is formatted, and that +the pages the integration suite reaches still work on both engines. That is a good deal +more than it used to be, and still nowhere near the whole forum: most of it has no test +touching it, so a green PR is not evidence that a change works. Verify by running the +forum. The repository ships a Docker environment, documented in full in `.dev/README.md`: ```bash docker compose up -d --build diff --git a/tests/Integration/Http/GuestPagesTest.php b/tests/Integration/Http/GuestPagesTest.php new file mode 100644 index 00000000000..cf3841bacff --- /dev/null +++ b/tests/Integration/Http/GuestPagesTest.php @@ -0,0 +1,168 @@ +fetch($path); + + $this->assertLooksLikeAForumPage($response, $name); + $this->assertNoErrorsLogged($name . ' (' . $path . ') logged something.' . "\n"); + } + + public function testTheBoardIndexListsAtLeastOneBoard(): void + { + $response = $this->fetch(''); + + $this->assertGreaterThan( + 0, + $response->xpath('//a[contains(@href, "board=")]')->length, + 'the board index links to no boards, so a fresh install has nothing in it', + ); + + $this->assertNoErrorsLogged(); + } + + /** + * The one page here that is not HTML. It is worth its place because the feed + * is built by hand rather than by the template layer, so nothing else in this + * file would notice it breaking. + */ + public function testTheFeedIsXmlAndParses(): void + { + $response = $this->fetch('?action=.xml;type=rss2'); + + $this->assertStringContainsString( + 'xml', + strtolower($response->headers['content-type'] ?? ''), + 'the feed did not come back as XML', + ); + + $previous = libxml_use_internal_errors(true); + $parsed = simplexml_load_string($response->body); + libxml_clear_errors(); + libxml_use_internal_errors($previous); + + $this->assertNotFalse($parsed, 'the feed is not well formed XML'); + $this->assertNoErrorsLogged('the feed logged something.' . "\n"); + } + + /** + * An action that does not exist should be a 404, not a 200 with an apology + * and not a 500. + */ + public function testAnUnknownActionIsNotFound(): void + { + $this->fetch('?action=smf_tests_no_such_action', 404); + } + + /** + * Registration refuses to start for a visitor who sends no cookies at all, + * because a registration that cannot keep a session cannot be completed. + * + * It is in its own test rather than the sweep above because it is the one + * page here that a cold request is genuinely not allowed to reach, and the + * difference between the two halves is worth stating: arriving at the forum + * first is what makes it work, and that is what a browser does. + */ + public function testRegistrationNeedsASessionFirst(): void + { + $this->http->forgetCookies(); + + $cold = $this->http->get('?action=signup'); + + $this->assertSame(403, $cold->status, 'a cookieless visitor was allowed into registration'); + + // Arrive at the forum the way a person would, which sets the session + // cookie, and then go to register. + $this->fetch(''); + + $agreement = $this->fetch('?action=signup'); + + $this->assertLooksLikeAForumPage($agreement, 'the registration agreement'); + + // requireAgreement is on by default, so step one is the agreement rather + // than the form. Note the form has to be named: the first form on any SMF + // page is the search box in the header. + $registration_form = '//form[contains(@action, "action=signup")]'; + + $this->assertGreaterThan( + 0, + $agreement->xpath($registration_form . '//input[@name="accept_agreement"]')->length, + 'registration did not start at the agreement', + ); + + // Buttons are not submitted unless named, so say which one we press. + $form = $this->http->submit($agreement, [ + 'accept_agreement' => 'I accept the terms of the agreement.', + ], $registration_form); + + $this->assertSame(200, $form->status, 'accepting the agreement returned ' . $form->status); + + $this->assertGreaterThan( + 0, + $form->xpath('//input[@name="user"]')->length, + 'accepting the agreement did not lead to the registration form', + ); + + $this->assertNoErrorsLogged('registering logged something.' . "\n"); + } + + /*********************** + * Public static methods + ***********************/ + + /** + * The pages a guest can reach on a stock install. + * + * Deliberately only actions that a fresh forum can serve without any content + * having been created and without being logged in, so this stays green on a + * forum straight out of .dev/install-forum.sh. + * + * @return array The cases, path and a readable name. + */ + public static function guestPages(): array + { + return [ + 'board index' => ['', 'the board index'], + 'help' => ['?action=help', 'help'], + 'login form' => ['?action=login', 'the login form'], + 'recent posts' => ['?action=recent', 'recent posts'], + 'unread' => ['?action=unread', 'unread posts'], + 'search form' => ['?action=search', 'the search form'], + 'member list' => ['?action=mlist', 'the member list'], + 'statistics' => ['?action=stats', 'the statistics page'], + 'credits' => ['?action=credits', 'the credits page'], + 'who is online' => ['?action=who', 'who is online'], + 'agreement' => ['?action=agreement', 'the registration agreement'], + 'first board' => ['?board=1.0', 'the first board'], + ]; + } +} diff --git a/tests/Integration/Http/HttpTestCase.php b/tests/Integration/Http/HttpTestCase.php new file mode 100644 index 00000000000..82623bfca62 --- /dev/null +++ b/tests/Integration/Http/HttpTestCase.php @@ -0,0 +1,274 @@ +http = new HttpClient(); + + // Arrive at the forum before doing anything else, which is what a person + // does and what the tests below depend on. + // + // The very first request of a new session regenerates it - SMF sets a + // guest login cookie, and Cookie::setLoginCookie() throws the session + // away and starts another whenever that value changes. Anything minted + // earlier in that same request is minted against the session that just + // went away, so a security token taken from the first page a visitor + // ever sees can never be validated. Posting that form comes back 403, + // "Token verification failed", with nothing to suggest the token was + // fine and the session underneath it was not. + $this->http->get(''); + } + + /** + * Signs in as the forum administrator. + * + * The credentials are the ones .dev/install-forum.sh uses, overridable + * through the environment for a forum that was set up some other way. + * + * @return HttpResponse The response to the login post. + */ + protected function signInAsAdmin(): HttpResponse + { + $response = $this->attemptSignIn(); + + if (self::isThrottled($response)) { + sleep(self::FLOOD_WAIT); + + $response = $this->attemptSignIn(); + } + + // A password that does not match is a misconfigured forum rather than a + // regression, and failing every test in the file over it would say + // nothing useful. Skipping names the variable to set. + if (str_contains($response->text(), 'username or password you entered is incorrect')) { + self::markTestSkipped( + 'cannot sign in as "' . self::adminName() . '". Set SMF_ADMIN_USER and ' + . 'SMF_ADMIN_PASS to this forum\'s administrator, or reinstall with ' + . '.dev/install-forum.sh --engine mysql --force', + ); + } + + $this->assertLessThan( + 400, + $response->status, + 'logging in returned ' . $response->status . ': ' . $response->errorText(), + ); + + return $response; + } + + /** + * Submits a form, waiting out flood control if it gets in the way. + * + * Tests do in half a second what a person would take a minute over, so they + * trip SMF's flood protection routinely. That is the forum working, not a + * regression, and the difference between a suite people trust and one that + * fails now and then for reasons nobody can reproduce. + * + * @param HttpResponse $page The page holding the form. + * @param array $overrides Values to change or add, including the button. + * @param string $xpath Which form. + * @return HttpResponse The response. + */ + protected function submitForm(HttpResponse $page, array $overrides, string $xpath): HttpResponse + { + $response = $this->http->submit($page, $overrides, $xpath); + + if (!self::isThrottled($response)) { + return $response; + } + + sleep(self::FLOOD_WAIT); + + // The page has to be fetched again rather than resubmitted: its security + // token was spent on the attempt that just bounced. + return $this->http->submit($this->http->get($page->url), $overrides, $xpath); + } + + /** + * Asserts the client is, or is not, signed in. + * + * Uses the logout link, which the theme only renders for a member. + * + * @param bool $expected Whether we should be signed in. + * @param string $message What was being checked. + */ + protected function assertSignedIn(bool $expected, string $message = ''): void + { + $signed_in = $this->fetch('')->xpath('//a[contains(@href, "action=logout")]')->length > 0; + + $this->assertSame($expected, $signed_in, $message !== '' ? $message : ($expected ? 'not signed in' : 'still signed in')); + } + + /** + * Fetches a page and asserts it came back whole. + * + * @param string $path Where to go, as HttpClient::get() takes it. + * @param int $expected The status it should return. + * @return HttpResponse The response, for further assertions. + */ + protected function fetch(string $path, int $expected = 200): HttpResponse + { + $response = $this->http->get($path); + + $this->assertSame( + $expected, + $response->status, + $path . ' returned ' . $response->status . ' from ' . $this->http->base_url, + ); + + return $response; + } + + /** + * Asserts a page is a real forum page rather than an error SMF rendered + * with a 200. + * + * A fatal error in SMF is a normal page with an apologetic message in it, so + * the status code alone proves very little. + * + * @param HttpResponse $response The response to check. + * @param string $where What was being fetched, for the failure message. + */ + protected function assertLooksLikeAForumPage(HttpResponse $response, string $where): void + { + $this->assertNotSame('', $response->title(), $where . ' has no '); + + $this->assertGreaterThan( + 0, + $response->xpath('//div[@id="footer"] | //footer | //*[@id="bot"]')->length, + $where . ' has no footer, so the template did not finish rendering', + ); + + $this->assertSame( + 0, + $response->xpath('//*[contains(@class, "errorbox")]')->length, + $where . ' rendered an error box: ' . $response->errorText(), + ); + } + + /** + * One go at the login form. + * + * @return HttpResponse The response to the post. + */ + private function attemptSignIn(): HttpResponse + { + $form = $this->fetch('?action=login'); + + return $this->http->submit($form, [ + 'user' => self::adminName(), + 'passwrd' => self::adminPassword(), + ], '//form[contains(@action, "action=login2")]'); + } + + /************************* + * Internal static methods + *************************/ + + /** + * Whether a response is SMF turning us away for going too fast. + * + * @param HttpResponse $response The response to look at. + * @return bool Whether flood control rejected it. + */ + protected static function isThrottled(HttpResponse $response): bool + { + $error = $response->errorText(); + + return str_contains($error, 'You will have to wait') + || str_contains($error, 'The last posting from your IP'); + } +} diff --git a/tests/Integration/Http/LoginTest.php b/tests/Integration/Http/LoginTest.php new file mode 100644 index 00000000000..48b7ba77555 --- /dev/null +++ b/tests/Integration/Http/LoginTest.php @@ -0,0 +1,99 @@ +<?php + +declare(strict_types=1); + +namespace SMF\Tests\Integration\Http; + +use PHPUnit\Framework\Attributes\CoversNothing; +use SMF\Config; + +/** + * Signing in over HTTP. + * + * Worth testing this way rather than through User::setMe(), which is what the + * rest of the integration suite uses: the parts most likely to break are exactly + * the ones setMe() skips. The session has to survive between requests, the + * security token minted with the form has to still be valid when it comes back, + * the cookie has to be signed with $auth_secret, and a post that arrives without + * a session check has to be turned away. + */ +#[CoversNothing] +class LoginTest extends HttpTestCase +{ + /**************** + * Public methods + ****************/ + + public function testTheAdministratorCanSignIn(): void + { + $this->signInAsAdmin(); + + $this->assertSignedIn(true, 'the session did not survive the login redirect'); + $this->assertNoErrorsLogged('signing in logged something.' . "\n"); + } + + /** + * The cookie is what carries the login between requests, so it is worth + * checking it was issued rather than inferring it from the page changing. + */ + public function testSigningInIssuesTheForumCookie(): void + { + $response = $this->signInAsAdmin(); + + $this->assertNotEmpty($response->set_cookies, 'logging in set no cookie at all'); + + // Note the plural: the response carries the session cookie as well, and + // keeping only the last one would test whichever happened to come second. + $this->assertStringContainsString( + (string) Config::$cookiename, + implode("\n", $response->set_cookies), + 'the forum cookie was not among those set: ' . implode(' | ', $response->set_cookies), + ); + } + + public function testTheWrongPasswordDoesNotSignAnyoneIn(): void + { + $form = $this->fetch('?action=login'); + + $this->http->submit($form, [ + 'user' => self::adminName(), + 'passwrd' => 'definitely not the password', + ], '//form[contains(@action, "action=login2")]'); + + $this->assertSignedIn(false, 'a wrong password signed us in anyway'); + } + + /** + * A post carrying no session check should be turned away. This is the guard + * that stops another site from posting to the forum on a visitor's behalf, + * and nothing that does not go over HTTP can exercise it. + */ + public function testAPostWithoutTheSessionCheckIsRejected(): void + { + // Hand built rather than submitted from the form, so none of the session + // fields the form carries are included. + $this->http->post('?action=login2', [ + 'user' => self::adminName(), + 'passwrd' => self::adminPassword(), + ]); + + $this->assertSignedIn(false, 'a login with no session check was accepted'); + } + + public function testSigningOutEndsTheSession(): void + { + $this->signInAsAdmin(); + $this->assertSignedIn(true); + + $page = $this->fetch(''); + $logout = $page->xpath('//a[contains(@href, "action=logout")]')->item(0); + + $this->assertNotNull($logout, 'no logout link to follow'); + + // The link carries its own session check in the query string. + $this->http->get((string) $logout?->attributes?->getNamedItem('href')?->nodeValue); + + $this->assertSignedIn(false, 'still signed in after logging out'); + $this->assertNoErrorsLogged('logging out logged something.' . "\n"); + } +} diff --git a/tests/Integration/Http/PostingTest.php b/tests/Integration/Http/PostingTest.php new file mode 100644 index 00000000000..8240481b32c --- /dev/null +++ b/tests/Integration/Http/PostingTest.php @@ -0,0 +1,222 @@ +<?php + +declare(strict_types=1); + +namespace SMF\Tests\Integration\Http; + +use PHPUnit\Framework\Attributes\CoversNothing; +use SMF\Topic; + +/** + * Starting a topic and replying to it, over HTTP. + * + * This is the journey the forum exists for, and the one with the most behind it: + * the editor, the session check, the security token, permissions, the post + * itself, and then every counter and index SMF updates afterwards. Nothing short + * of a real request covers that. + * + * These tests write, and an HTTP test cannot be rolled back - the request runs in + * the web server's process on its own connection. So whatever they create, they + * remove again in tearDown through SMF's own Topic::remove(), which puts the + * board and member counts back the way deleting a topic in the browser would. + */ +#[CoversNothing] +class PostingTest extends HttpTestCase +{ + /********************* + * Internal properties + *********************/ + + /** + * @var array Topics this test created, to be removed afterwards. + */ + private array $created_topics = []; + + /**************** + * Public methods + ****************/ + + public function testStartingATopicAndReplyingToIt(): void + { + $this->signInAsAdmin(); + + $subject = 'Integration test topic ' . bin2hex(random_bytes(6)); + $body = 'Posted by the integration suite at ' . date('c') . '.'; + + $topic_id = $this->startTopic($subject, $body); + + $topic = $this->fetch('?topic=' . $topic_id . '.0'); + + $this->assertStringContainsString( + $subject, + $topic->text(), + 'the new topic does not show its own subject', + ); + + $this->assertStringContainsString($body, $topic->text(), 'the post body is missing'); + + // And now a reply, which goes through a different form on a different + // page and updates a different set of counters. + $reply = 'A reply from the integration suite.'; + + $form = $this->fetch('?action=post;topic=' . $topic_id . '.0'); + + $posted = $this->submitForm($form, [ + 'subject' => 'Re: ' . $subject, + 'message' => $reply, + 'post' => 'Post', + ], '//form[contains(@action, "action=post2")]'); + + $this->assertLessThan( + 400, + $posted->status, + 'replying returned ' . $posted->status . ': ' . $posted->errorText(), + ); + + $after = $this->fetch('?topic=' . $topic_id . '.0'); + + $this->assertStringContainsString($reply, $after->text(), 'the reply is not on the topic'); + + $this->assertSame( + 2, + $this->countMessages($topic_id), + 'the topic should hold the first post and the reply', + ); + + $this->assertNoErrorsLogged('posting logged something.' . "\n"); + } + + /** + * A guest cannot post on a stock install, and the forum should say so rather + * than accept it. + */ + public function testAGuestCannotStartATopic(): void + { + $before = $this->countTopicsInBoard(1); + + $this->http->post('?action=post2;board=1', [ + 'subject' => 'Integration test guest post', + 'message' => 'This should not be accepted.', + ]); + + $this->assertSame( + $before, + $this->countTopicsInBoard(1), + 'a guest with no session check managed to start a topic', + ); + } + + /****************** + * Internal methods + ******************/ + + protected function tearDown(): void + { + // Before the parent runs, while the connection is still ours. + if ($this->created_topics !== []) { + Topic::remove($this->created_topics); + + $this->created_topics = []; + } + + parent::tearDown(); + } + + /** + * Starts a topic in the first board and returns its id. + * + * @param string $subject The subject. + * @param string $body The message. + * @return int The new topic's id. + */ + private function startTopic(string $subject, string $body): int + { + $before = $this->latestTopicId(); + + $form = $this->fetch('?action=post;board=1.0'); + + $this->assertGreaterThan( + 0, + $form->xpath('//form[contains(@action, "action=post2")]')->length, + 'there is no posting form on the new topic page', + ); + + $posted = $this->submitForm($form, [ + 'subject' => $subject, + 'message' => $body, + // The button we are pressing. Without it the form's other button, + // "preview", is the one SMF acts on. + 'post' => 'Post', + ], '//form[contains(@action, "action=post2")]'); + + $this->assertLessThan( + 400, + $posted->status, + 'posting returned ' . $posted->status . ': ' . $posted->errorText(), + ); + + $topic_id = $this->latestTopicId(); + + // Quote the page when this fails. SMF answers a rejected post with a + // perfectly ordinary 200 and the reason in a box, so without this the + // only evidence is a topic id that did not move. + $this->assertGreaterThan( + $before, + $topic_id, + 'no new topic appeared after posting. The forum said: ' + . ($posted->errorText() !== '' ? $posted->errorText() : '(nothing) - page title "' . $posted->title() . '"'), + ); + + $this->created_topics[] = $topic_id; + + return $topic_id; + } + + /** + * The highest topic id in the forum. + * + * @return int The id, or 0 when there are no topics. + */ + private function latestTopicId(): int + { + $row = $this->queryRow('SELECT COALESCE(MAX(id_topic), 0) AS id FROM {db_prefix}topics'); + + return (int) ($row['id'] ?? 0); + } + + /** + * How many messages a topic holds. + * + * @param int $topic_id The topic. + * @return int The number of messages. + */ + private function countMessages(int $topic_id): int + { + $row = $this->queryRow( + 'SELECT COUNT(*) AS total + FROM {db_prefix}messages + WHERE id_topic = {int:topic}', + ['topic' => $topic_id], + ); + + return (int) ($row['total'] ?? 0); + } + + /** + * How many topics a board holds. + * + * @param int $board_id The board. + * @return int The number of topics. + */ + private function countTopicsInBoard(int $board_id): int + { + $row = $this->queryRow( + 'SELECT COUNT(*) AS total + FROM {db_prefix}topics + WHERE id_board = {int:board}', + ['board' => $board_id], + ); + + return (int) ($row['total'] ?? 0); + } +} diff --git a/tests/Integration/Http/index.php b/tests/Integration/Http/index.php new file mode 100644 index 00000000000..2844a3b9e7b --- /dev/null +++ b/tests/Integration/Http/index.php @@ -0,0 +1,8 @@ +<?php + +// Try to handle it with the upper level index.php. (it should know what to do.) +if (file_exists(dirname(__DIR__) . DIRECTORY_SEPARATOR . 'index.php')) { + include dirname(__DIR__) . DIRECTORY_SEPARATOR . 'index.php'; +} else { + exit; +} diff --git a/tests/Integration/Installation.php b/tests/Integration/Installation.php index 4505b2675f8..8fd4c2ba950 100644 --- a/tests/Integration/Installation.php +++ b/tests/Integration/Installation.php @@ -19,7 +19,7 @@ * It never throws. When there is nothing to test against it says why, and * IntegrationTestCase turns that into a skip rather than a failure. * - * To get a forum: .docker/install-forum.sh --engine mysql + * To get a forum: .dev/install-forum.sh --engine mysql */ final class Installation { diff --git a/tests/Integration/IntegrationTestCase.php b/tests/Integration/IntegrationTestCase.php index 3475c80dd33..7e3a54478f0 100644 --- a/tests/Integration/IntegrationTestCase.php +++ b/tests/Integration/IntegrationTestCase.php @@ -42,31 +42,51 @@ abstract class IntegrationTestCase extends TestCase */ private int $error_watermark = 0; - /*********************** - * Public static methods - ***********************/ + /****************** + * Internal methods + ******************/ + + /** + * Whether to wrap the test in a transaction that is rolled back afterwards. + * + * Override and return false when the test causes work to happen in another + * process - a request made over HTTP, say. That runs on its own connection, + * so the transaction cannot undo it, and on MySQL, whose default isolation + * level is REPEATABLE READ, this connection would go on reading the snapshot + * it took before the request and never see what the request wrote. + * + * @return bool True to use a transaction, which is what most tests want. + */ + protected function usesTransaction(): bool + { + return true; + } - public static function setUpBeforeClass(): void + protected function setUp(): void { + parent::setUp(); + + // Before anything reaches for the connection below. + // + // Deliberately here rather than in setUpBeforeClass(), which is the + // obvious home for a check that gives the same answer for every test in + // the class. A skip raised there marks the class as skipped rather than + // its tests, and --fail-on-skipped only counts skipped tests -- so a CI + // job that could not find the forum would report a green run having + // tested nothing at all. Installation memoises its answer, so asking + // once per test costs a function call. $reason = Installation::unavailableReason(); if ($reason !== '') { self::markTestSkipped( 'no forum to test against: ' . $reason - . '. Run .docker/install-forum.sh --engine mysql', + . '. Run .dev/install-forum.sh --engine mysql', ); } - } - - /****************** - * Internal methods - ******************/ - protected function setUp(): void - { - parent::setUp(); - - Db::$db->transaction('begin'); + if ($this->usesTransaction()) { + Db::$db->transaction('begin'); + } // $modSettings is a plain static array, so a test that calls // updateModSettings() changes it for everything that runs after it. The @@ -78,7 +98,13 @@ protected function setUp(): void protected function tearDown(): void { - Db::$db->transaction('rollback'); + // PHPUnit runs this even for a test setUp() skipped, and there is nothing + // to undo in that case: no transaction was opened, and on a machine with + // no forum there is no connection to ask. Without this, a run that should + // read as "43 skipped, no forum" reads as 18 errors instead. + if ($this->usesTransaction() && isset(Db::$db)) { + Db::$db->transaction('rollback'); + } Config::$modSettings = $this->mod_settings_backup; diff --git a/tests/Support/HttpClient.php b/tests/Support/HttpClient.php new file mode 100644 index 00000000000..be674df3ca3 --- /dev/null +++ b/tests/Support/HttpClient.php @@ -0,0 +1,361 @@ +<?php + +declare(strict_types=1); + +namespace SMF\Tests\Support; + +use SMF\Config; + +/** + * A very small browser, for driving the forum over HTTP. + * + * Requests have to be real ones. Utils::obExit(), redirectexit(), + * serverResponse() and ErrorHandler::fatal*() all end in exit, and Db::$db, + * ActionTrait::$obj, Theme::$loaded and User::$loaded have no way to be reset, so + * a test process can carry out exactly one request in itself and no more. Going + * over the wire sidesteps all of that and exercises the same path a visitor does, + * including the session, the cookies and the theme. + * + * Uses curl through the extension the forum already requires, so it costs no new + * dependency. + */ +final class HttpClient +{ + /******************* + * Public properties + *******************/ + + /** + * @var string Where requests go. Public so a test can report it on failure. + */ + public readonly string $base_url; + + /********************* + * Internal properties + *********************/ + + /** + * @var \CurlHandle One handle for the life of the client. + * + * Reused rather than opened per request, and that is load bearing. curl only + * writes cookies that carry an expiry to the jar file; a session cookie has + * none, so closing the handle between requests threw the session away and + * every request arrived as a brand new visitor. The symptom is not an obvious + * one - pages still render, but any POST is rejected because the session + * check and the security token it carries were issued to a session that no + * longer exists. + */ + private \CurlHandle $handle; + + /** + * @var string Path to this client's cookie jar. + */ + private string $jar; + + /** + * @var HttpResponse|null The most recent response. + */ + private ?HttpResponse $last = null; + + /**************** + * Public methods + ****************/ + + /** + * @param string|null $base_url Override the forum URL to talk to. + */ + public function __construct(?string $base_url = null) + { + $this->base_url = rtrim($base_url ?? self::detectBaseUrl(), '/'); + + $this->jar = (string) tempnam(sys_get_temp_dir(), 'smf_tests_cookies_'); + + $handle = curl_init(); + + if (!$handle instanceof \CurlHandle) { + throw new \RuntimeException('could not start curl'); + } + + $this->handle = $handle; + } + + public function __destruct() + { + curl_close($this->handle); + + if ($this->jar !== '' && is_file($this->jar)) { + @unlink($this->jar); + } + } + + /** + * Fetches a page. + * + * @param string $path Either a full URL or something to hang off the board + * URL, with or without a leading slash. '?action=login' is typical. + * @return HttpResponse The response. + */ + public function get(string $path = ''): HttpResponse + { + return $this->request($this->url($path), null); + } + + /** + * Posts to a page. + * + * @param string $path Where to post, as for get(). + * @param array $fields The form fields. + * @return HttpResponse The response. + */ + public function post(string $path, array $fields): HttpResponse + { + return $this->request($this->url($path), $fields); + } + + /** + * Submits a form on a page the client has already fetched. + * + * This is the method to reach for. SMF forms carry a session check that + * User::checkSession() rejects the request without, and often a SecurityToken + * as well, both named unpredictably per session; resubmitting every field the + * page offered is what a browser does and saves the test knowing about any of + * it. + * + * @param HttpResponse $page The page holding the form. + * @param array $overrides Values to change or add. + * @param string $xpath Which form. Defaults to the first on the page. + * @return HttpResponse The response. + */ + public function submit(HttpResponse $page, array $overrides = [], string $xpath = '//form'): HttpResponse + { + return $this->request( + $this->url($page->formAction($xpath)), + array_merge($page->formFields($xpath), $overrides), + ); + } + + /** + * The most recent response, for reporting on a failure. + * + * @return HttpResponse|null The response, or null if nothing has been sent. + */ + public function lastResponse(): ?HttpResponse + { + return $this->last; + } + + /** + * Throws away this client's cookies, making it a fresh visitor. + */ + public function forgetCookies(): void + { + // In memory, not in the file: session cookies never reach the file. + curl_setopt($this->handle, CURLOPT_COOKIELIST, 'ALL'); + + if (is_file($this->jar)) { + file_put_contents($this->jar, ''); + } + } + + /****************** + * Internal methods + ******************/ + + /** + * Turns whatever a caller passed into an absolute URL. + * + * @param string $path A full URL, a query string, or a path. + * @return string An absolute URL. + */ + private function url(string $path): string + { + if ($path === '') { + return $this->base_url . '/'; + } + + // A form action is usually an absolute URL built from Config::$boardurl, + // which is not necessarily the host we are talking to - inside the + // container the forum answers on port 80 while boardurl names 8080. Keep + // the path and query, drop the rest. + if (preg_match('~^https?://~i', $path)) { + $parts = parse_url($path); + + $path = ($parts['path'] ?? '/') + . (isset($parts['query']) ? '?' . $parts['query'] : '') + . (isset($parts['fragment']) ? '#' . $parts['fragment'] : ''); + } + + if (str_starts_with($path, '?')) { + return $this->base_url . '/index.php' . $path; + } + + return $this->base_url . '/' . ltrim($path, '/'); + } + + /** + * Sends one request. + * + * @param string $url The absolute URL. + * @param array|null $fields POST fields, or null for a GET. + * @return HttpResponse The response. + */ + private function request(string $url, ?array $fields): HttpResponse + { + $handle = $this->handle; + + $options = [ + CURLOPT_URL => $url, + CURLOPT_RETURNTRANSFER => true, + CURLOPT_HEADER => true, + // Off on purpose. A redirect is frequently the thing under test - + // posting a reply is a success only if it sends you somewhere - and + // following it silently would hide both the status and the location. + CURLOPT_FOLLOWLOCATION => false, + CURLOPT_COOKIEJAR => $this->jar, + CURLOPT_COOKIEFILE => $this->jar, + CURLOPT_CONNECTTIMEOUT => 5, + CURLOPT_TIMEOUT => 30, + CURLOPT_USERAGENT => 'SMF test suite', + ]; + + if ($fields !== null) { + $options[CURLOPT_POST] = true; + $options[CURLOPT_POSTFIELDS] = http_build_query($fields); + } else { + // The handle is reused, so a GET after a POST has to say so or it + // would repeat the previous body. + $options[CURLOPT_HTTPGET] = true; + } + + curl_setopt_array($handle, $options); + + $raw = curl_exec($handle); + + if ($raw === false) { + throw new \RuntimeException('request to ' . $url . ' failed: ' . curl_error($handle)); + } + + $status = (int) curl_getinfo($handle, CURLINFO_HTTP_CODE); + $header_size = (int) curl_getinfo($handle, CURLINFO_HEADER_SIZE); + + $raw = (string) $raw; + + [$headers, $set_cookies] = self::parseHeaders(substr($raw, 0, $header_size)); + + return $this->last = new HttpResponse( + $status, + substr($raw, $header_size), + $headers, + $url, + $set_cookies, + ); + } + + /************************* + * Internal static methods + *************************/ + + /** + * Works out which URL the forum answers on. + * + * SMF_TESTS_BASE_URL wins. Otherwise it is Config::$boardurl, unless nothing + * is listening there - which is the normal case when the tests run inside the + * web container, where the forum is on port 80 and boardurl names whatever + * port the host publishes. + * + * @return string The base URL. + */ + private static function detectBaseUrl(): string + { + $override = (string) getenv('SMF_TESTS_BASE_URL'); + + if ($override !== '') { + return $override; + } + + $boardurl = (string) (Config::$boardurl ?? ''); + + if ($boardurl !== '' && self::listening($boardurl)) { + return $boardurl; + } + + $parts = parse_url($boardurl) ?: []; + + return ($parts['scheme'] ?? 'http') . '://localhost' . ($parts['path'] ?? ''); + } + + /** + * Whether anything answers on the host and port of a URL. + * + * @param string $url The URL to try. + * @return bool Whether a connection could be opened. + */ + private static function listening(string $url): bool + { + $parts = parse_url($url); + + if (!isset($parts['host'])) { + return false; + } + + $port = $parts['port'] ?? (($parts['scheme'] ?? 'http') === 'https' ? 443 : 80); + + $socket = @fsockopen($parts['host'], (int) $port, $errno, $errstr, 2); + + if ($socket === false) { + return false; + } + + fclose($socket); + + return true; + } + + /** + * Splits a raw header block into name => value. + * + * Only the last set is kept, which matters because CURLOPT_HEADER includes + * every hop when a proxy or a 100-continue is involved. + * + * @param string $raw The raw headers. + * @return array Two items: the headers as lowercased name => value, and + * every Set-Cookie value in order. + */ + private static function parseHeaders(string $raw): array + { + $headers = []; + $cookies = []; + + foreach (preg_split('~\R~', $raw) ?: [] as $line) { + $line = trim($line); + + if ($line === '') { + continue; + } + + if (stripos($line, 'HTTP/') === 0) { + $headers = []; + $cookies = []; + + continue; + } + + $colon = strpos($line, ':'); + + if ($colon === false) { + continue; + } + + $name = strtolower(substr($line, 0, $colon)); + $value = trim(substr($line, $colon + 1)); + + $headers[$name] = $value; + + if ($name === 'set-cookie') { + $cookies[] = $value; + } + } + + return [$headers, $cookies]; + } +} diff --git a/tests/Support/HttpResponse.php b/tests/Support/HttpResponse.php new file mode 100644 index 00000000000..15b844577e6 --- /dev/null +++ b/tests/Support/HttpResponse.php @@ -0,0 +1,259 @@ +<?php + +declare(strict_types=1); + +namespace SMF\Tests\Support; + +/** + * One response from HttpClient. + * + * Parsing is deliberately DOM based rather than string matching. The theme moves + * around a lot, so a test that greps for a phrase fails the next time somebody + * reflows the markup, which teaches everyone to ignore it. + */ +final class HttpResponse +{ + /********************* + * Internal properties + *********************/ + + /** + * @var \DOMDocument|null The parsed body, once something has asked for it. + */ + private ?\DOMDocument $dom = null; + + /**************** + * Public methods + ****************/ + + /** + * @param int $status The HTTP status. + * @param string $body The response body. + * @param array $headers Headers, lowercased name => value. Where a header + * appeared more than once only the last is here; Set-Cookie is the one + * that routinely does, so it has its own list below. + * @param string $url The URL that was requested. + * @param array $set_cookies Every Set-Cookie header, in the order sent. + * Logging in sends two - the session and the forum's own - and keeping + * only one of them loses whichever the test cares about. + */ + public function __construct( + public readonly int $status, + public readonly string $body, + public readonly array $headers, + public readonly string $url, + public readonly array $set_cookies = [], + ) {} + + /** + * The response body as a DOM document. + * + * SMF emits HTML5, which DOMDocument grumbles about; the warnings are not + * interesting and would fail the test under failOnWarning, so they are + * collected and discarded rather than raised. + * + * Parsed once per response. Note the cache has to be a property: a static + * inside this method is shared by every instance of the class, so the first + * page fetched would be handed back for every page after it. + * + * @return \DOMDocument The parsed body. + */ + public function dom(): \DOMDocument + { + if ($this->dom instanceof \DOMDocument) { + return $this->dom; + } + + $dom = new \DOMDocument(); + + $previous = libxml_use_internal_errors(true); + $dom->loadHTML('<?xml encoding="UTF-8">' . $this->body, LIBXML_NOWARNING | LIBXML_NOERROR); + libxml_clear_errors(); + libxml_use_internal_errors($previous); + + return $this->dom = $dom; + } + + /** + * Runs an XPath query against the body. + * + * @param string $expression The XPath expression. + * @return \DOMNodeList The matching nodes. + */ + public function xpath(string $expression): \DOMNodeList + { + $result = (new \DOMXPath($this->dom()))->query($expression); + + return $result === false ? new \DOMNodeList() : $result; + } + + /** + * The page title, without the forum name SMF appends to it. + * + * @return string The title, or an empty string when there is none. + */ + public function title(): string + { + $titles = $this->xpath('//title'); + + return $titles->length === 0 ? '' : trim((string) $titles->item(0)?->textContent); + } + + /** + * All visible text, with runs of whitespace collapsed. + * + * For assertions where the structure genuinely does not matter, such as + * checking an error message reached the page at all. + * + * @return string The text content of the body. + */ + public function text(): string + { + $body = $this->xpath('//body'); + $text = $body->length === 0 ? $this->body : (string) $body->item(0)?->textContent; + + return trim((string) preg_replace('~\s+~u', ' ', $text)); + } + + /** + * Whatever the page is complaining about. + * + * SMF renders a fatal error as an ordinary page with a box on it, and the + * surrounding menus and news run to several hundred characters, so quoting + * the start of the body in a failure message reliably shows everything + * except the reason. This picks out the reason. + * + * @return string The error text, or an empty string when there is none. + */ + public function errorText(): string + { + $found = []; + + foreach ($this->xpath('//*[contains(@class, "errorbox")] | //*[contains(@class, "error_message")] | //*[@id="fatal_error"]') as $node) { + $text = trim((string) preg_replace('~\s+~u', ' ', $node->textContent)); + + if ($text !== '') { + $found[] = $text; + } + } + + return implode(' / ', array_unique($found)); + } + + /** + * Every field a browser would submit for the given form. + * + * SMF puts more than one hidden field in its forms - the session check that + * User::checkSession() insists on, and often a SecurityToken as well - and + * the names of both are generated per session. Collecting them all is both + * simpler and more honest than knowing which is which. + * + * Buttons are left out, because a browser submits only the one that was + * clicked and SMF branches on which that was. The posting form offers both + * "preview" and "post"; sending the pair means the preview wins and the reply + * is silently never made, with a perfectly good 200 to show for it. Pass the + * button you mean to press as an override. + * + * @param string $xpath Which form. Defaults to the first one on the page. + * @return array The fields, name => value. + */ + public function formFields(string $xpath = '//form'): array + { + $form = $this->xpath($xpath)->item(0); + + if (!$form instanceof \DOMElement) { + return []; + } + + $fields = []; + $finder = new \DOMXPath($this->dom()); + + foreach ($finder->query('.//input | .//textarea | .//select', $form) ?: [] as $input) { + if (!$input instanceof \DOMElement) { + continue; + } + + $name = $input->getAttribute('name'); + + if ($name === '') { + continue; + } + + $type = strtolower($input->getAttribute('type')); + + // A browser only submits these when they are ticked, and submitting + // an unticked one turns every checkbox on the page into a yes. + if (\in_array($type, ['checkbox', 'radio'], true) && !$input->hasAttribute('checked')) { + continue; + } + + // Only the button that was clicked gets submitted. See above. + if (\in_array($type, ['submit', 'button', 'reset', 'image'], true)) { + continue; + } + + $fields[$name] = match ($input->nodeName) { + 'textarea' => $input->textContent, + 'select' => $this->selectedOption($finder, $input), + default => $input->getAttribute('value'), + }; + } + + return $fields; + } + + /** + * Where the given form posts to. + * + * @param string $xpath Which form. Defaults to the first one on the page. + * @return string The action attribute, or the current URL when it has none. + */ + public function formAction(string $xpath = '//form'): string + { + $form = $this->xpath($xpath)->item(0); + + if (!$form instanceof \DOMElement) { + return $this->url; + } + + $action = $form->getAttribute('action'); + + return $action === '' ? $this->url : $action; + } + + /****************** + * Internal methods + ******************/ + + /** + * The value a browser would submit for a select element. + * + * @param \DOMXPath $finder An XPath instance for this document. + * @param \DOMElement $select The select element. + * @return string The selected value, or the first option's. + */ + private function selectedOption(\DOMXPath $finder, \DOMElement $select): string + { + $options = $finder->query('.//option', $select) ?: new \DOMNodeList(); + + $first = ''; + + foreach ($options as $option) { + if (!$option instanceof \DOMElement) { + continue; + } + + $value = $option->hasAttribute('value') ? $option->getAttribute('value') : $option->textContent; + + if ($first === '') { + $first = $value; + } + + if ($option->hasAttribute('selected')) { + return $value; + } + } + + return $first; + } +} diff --git a/tests/Support/index.php b/tests/Support/index.php new file mode 100644 index 00000000000..2844a3b9e7b --- /dev/null +++ b/tests/Support/index.php @@ -0,0 +1,8 @@ +<?php + +// Try to handle it with the upper level index.php. (it should know what to do.) +if (file_exists(dirname(__DIR__) . DIRECTORY_SEPARATOR . 'index.php')) { + include dirname(__DIR__) . DIRECTORY_SEPARATOR . 'index.php'; +} else { + exit; +}