Openverse: Strip only the leading subpath when building the redirect target.

.github/unit-tests-suites.yml

@@ -99,3 +99,11 @@ suites:
     script: make:test
     paths:
       - 'environments/make/**'
+
+  openverse:
+    type: wordpress
+    name: Openverse Theme
+    script: openverse:test
+    paths:
+      - 'environments/openverse/**'
+      - 'wordpress.org/public_html/wp-content/themes/pub/wporg-openverse/**'

environments/openverse/.wp-env.test.json

@@ -0,0 +1,12 @@
+{
+	"core": "WordPress/WordPress#master",
+	"phpVersion": "8.4",
+	"testsEnvironment": false,
+	"plugins": [],
+	"themes": [
+		"../wordpress.org/public_html/wp-content/themes/pub/wporg-openverse"
+	],
+	"lifecycleScripts": {
+		"afterStart": "bash openverse/bin/after-start-test.sh"
+	}
+}

environments/openverse/bin/after-start-test.sh

@@ -0,0 +1,14 @@
+#!/bin/bash
+#
+# Runs after wp-env start for the test environment.
+# Installs PHPUnit 11 and Yoast polyfills in the test container.
+#
+
+set -euo pipefail
+
+CONFIG="--config openverse/.wp-env.test.json"
+RUN="npx wp-env $CONFIG run cli"
+
+echo "Installing PHPUnit 11 and polyfills..."
+$RUN composer global require -W phpunit/phpunit:^11.0 2>&1
+$RUN composer require --dev yoast/phpunit-polyfills:^4.0 --working-dir=/wordpress-phpunit 2>&1

environments/package.json

@@ -25,6 +25,8 @@
 		"make:test": "npm run make:test:env -- start && npm run make:test:posting-access && npm run make:test:trac-watcher && npm run make:test:cli",
 		"jobs:env": "wp-env --config jobs/.wp-env.json",
 		"browsehappy:env": "wp-env --config browsehappy/.wp-env.json",
+		"openverse:test:env": "wp-env --config openverse/.wp-env.test.json",
+		"openverse:test": "npm run openverse:test:env -- start && npm run openverse:test:env -- run cli --env-cwd=wp-content/themes/wporg-openverse phpunit",
 		"translate:env": "wp-env --config translate/.wp-env.json",
 		"translate:import": "npm run translate:env -- run cli -- wp eval-file wp-content/env-bin/import-from-wporg.php",
 		"translate:refresh": "npm run translate:env -- run cli -- wp option delete wporg_translate_env_seeded"

wordpress.org/public_html/wp-content/themes/pub/wporg-openverse/functions.php

@@ -255,25 +255,97 @@ function wporg_ov_customizer( $wp_customize ) {
  * Examples:
  * - https://ru.wordpress.org/openverse → {ov_redirect_url}/ru/
  * - https://wordpress.org/openverse/search/?q=dog → {ov_redirect_url}/search/?q=dog
+ *
+ * The returned URL always carries at least a trailing path separator, so a bare
+ * `/openverse` resolves to `{ov_redirect_url}/` rather than to the origin alone.
+ *
+ * @return string
  */
 function get_target_url() {
-	$target_url = get_theme_mod( 'ov_redirect_url', OPENVERSE_STANDALONE_URL );
+	$target_url = get_standalone_origin();
 
 	$curr_locale = get_locale();
 	$locale = get_locale_slug( $curr_locale );
 	if ( $locale !== '' ) {
 		$target_url .= '/' . $locale;
 	}
 
-	$path = $_SERVER['REQUEST_URI'];
-	if ( $path ) {
-		$count = 1; // Only replace the leading Openverse subpath.
-		$target_url .= str_replace( OPENVERSE_SUBPATH, '', $path, $count );
+	// Sanitising is required by WordPress.Security.ValidatedSanitizedInput.
+	// Not `sanitize_text_field()`, which strips percent-encoded octets and
+	// would turn `?q=cat%20dog` into `?q=catdog`.
+	$path = isset( $_SERVER['REQUEST_URI'] ) ? esc_url_raw( wp_unslash( $_SERVER['REQUEST_URI'] ) ) : '';
+
+	// Only a leading, whole-segment subpath is removed. `str_replace()` could
+	// express neither constraint: it replaced every occurrence, including one
+	// in a later segment such as `/image/openverse-logo/`, one in the query
+	// string, and the `/openverse` inside a longer segment like
+	// `/openverse-search`.
+	if ( OPENVERSE_SUBPATH === $path
+		|| str_starts_with( $path, OPENVERSE_SUBPATH . '/' )
+		|| str_starts_with( $path, OPENVERSE_SUBPATH . '?' )
+	) {
+		$path = substr( $path, strlen( OPENVERSE_SUBPATH ) );
 	}
 
+	// The origin has no trailing slash, so the path must supply the separator.
+	// Prepending it is also what keeps the remainder in the path rather than
+	// the authority; `ltrim()` only collapses a doubled slash.
+	$target_url .= '/' . ltrim( $path, '/' );
+
 	return $target_url;
 }
 
+/**
+ * The origin the standalone Openverse site is served from.
+ *
+ * The Customizer's `sanitize_callback` drops a trailing slash and
+ * `wp theme mod set` does not, so it is dropped here too.
+ *
+ * @return string
+ */
+function get_standalone_origin() {
+	return untrailingslashit( get_theme_mod( 'ov_redirect_url', OPENVERSE_STANDALONE_URL ) );
+}
+
+/**
+ * Whether the redirect to the standalone site is switched on.
+ *
+ * `wp theme mod set` stores the string it is handed, so the setting can hold
+ * `'false'`, which PHP reads as true. `wp_validate_boolean()` reads it the way
+ * whoever typed it meant it.
+ *
+ * @return bool
+ */
+function is_redirect_enabled() {
+	return wp_validate_boolean( get_theme_mod( 'ov_is_redirect_enabled', false ) );
+}
+
+/**
+ * Whether a redirect target is usable.
+ *
+ * The path is appended straight after the origin, so a target's authority is
+ * always the one an administrator configured. Comparing the two makes that an
+ * enforced property rather than an assumption, and it is why this does not need
+ * `wp_safe_redirect()`: no request can reach a host the origin did not supply,
+ * and the allow-list that would require widens redirects for the whole site.
+ *
+ * @param string $target_url URL the theme intends to redirect to.
+ * @return bool
+ */
+function is_valid_target_url( $target_url ) {
+	$parts = wp_parse_url( $target_url );
+
+	if ( empty( $parts['host'] ) || empty( $parts['scheme'] ) ) {
+		return false;
+	}
+
+	if ( ! in_array( strtolower( $parts['scheme'] ), array( 'http', 'https' ), true ) ) {
+		return false;
+	}
+
+	return wp_parse_url( get_standalone_origin(), PHP_URL_HOST ) === $parts['host'];
+}
+
 /**
 * Provide configuration for the theme to redirect to the given standalone
 * Openverse site. The destination URL can be configured and the behaviour can

wordpress.org/public_html/wp-content/themes/pub/wporg-openverse/index.php

@@ -18,16 +18,19 @@
 	If the theme mod `ov_is_redirect_enabled` is set to `true`, redirect to the
 	standalone site and exit immediately. If not, print what would have been the
 	redirect URL to the HTML as a comment.
+
+	The target is checked before redirecting. A misconfigured `ov_redirect_url`
+	would otherwise send a permanent redirect that sits in visitors' caches long
+	after the setting was corrected. Rendering the page is the safer failure.
  */
 
-$is_redirect_enabled = get_theme_mod( 'ov_is_redirect_enabled' );
 $target_url = get_target_url();
 
-if ( $is_redirect_enabled ) {
+if ( is_redirect_enabled() && is_valid_target_url( $target_url ) ) {
 	wp_redirect( $target_url, 301 );
 	exit;
 } else {
-	echo "<!-- " . $target_url . " -->";
+	echo '<!-- ' . esc_html( $target_url ) . ' -->';
 }
 
 get_header();

wordpress.org/public_html/wp-content/themes/pub/wporg-openverse/phpunit.xml.dist

@@ -0,0 +1,13 @@
+<phpunit
+	bootstrap="tests/bootstrap.php"
+	backupGlobals="false"
+	colors="true"
+	>
+	<testsuites>
+		<testsuite name="wporg-openverse">
+			<directory suffix=".php">tests/</directory>
+			<exclude>tests/bootstrap.php</exclude>
+			<exclude>tests/locales-stub.php</exclude>
+		</testsuite>
+	</testsuites>
+</phpunit>