fix[faustwp]: (#1872) use home_url() in handle_generate_endpoint#2339
Merged
josephfusco merged 5 commits intoMay 13, 2026
Merged
Conversation
On Bedrock-style installs where WordPress core lives under /wp/,
site_url('/generate') returns https://example.com/wp/generate but
$_SERVER['REQUEST_URI'] is still /generate, so the preg_match in
handle_generate_endpoint() never fires and the auth-code redirect
silently breaks.
home_url() returns the public-facing URL (what REQUEST_URI actually
contains), which matches on both standard and Bedrock installs.
Per @josephfusco's guidance on wpengine#2315, which was closed in favor of
this simpler fix — a new filter is not needed, just the correct URL
helper.
|
Currently, we do not support the creation of preview deployments based on changes coming from forked repositories. |
🦋 Changeset detectedLatest commit: 4d19bbd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
… case (wpengine#1872) Adds AuthCallbacksTests.php covering handle_generate_endpoint() with four scenarios: - Standard install reaches wp_safe_redirect (login redirect path). - Bedrock layout (site_url=/wp/x, home_url=/x) still matches REQUEST_URI=/x. This is the regression guard against future reverts to site_url(): verified to fail when callbacks.php is reverted to site_url() and pass with home_url(). - Unrelated REQUEST_URI early-returns without redirecting. - /generate without redirect_uri early-returns. Uses Patchwork to redefine wp_safe_redirect so the bare 'exit;' is bypassed and the captured redirect target can be asserted. Patchwork is already loaded via phpunit.xml.dist's LOAD_PATCHWORK=1. Also adds tests/mu-plugins/mu-bedrock-simulator.php -- a tiny mu-plugin that makes site_url() return /wp/<path> while home_url() stays at /<path>, for manual browser reproduction inside the docker-compose stack without requiring a full roots/bedrock install.
Peer-review polish on the earlier test commit:
- Use update_option('siteurl', ...) instead of an add_filter('site_url')
to drive Bedrock-style URL divergence. This matches what real Bedrock
configures via WP_SITEURL in wp-config.php, so every consumer of
get_option('siteurl') and site_url() sees the divergent value, not just
callers that go through the site_url filter. Removes the remove_all_filters()
tearDown that nuked unrelated core filters.
- Replace the RuntimeException + magic-string catch trick with a dedicated
RedirectAttempted exception class. Eliminates the risk of accidentally
swallowing unrelated runtime errors during the redirect-capture flow.
- Tighten assertions on the Bedrock-divergence test: now also asserts the
captured redirect contains 'wp-login.php' (matching the standard-install
test) and adds two sanity-check assertions that confirm the simulated
divergence is actually in place before the handler is invoked. Total
assertions in the suite increase from 5 to 8.
- Document the simulator mu-plugin's narrower scope vs the test's full
fidelity: the mu-plugin filters site_url() output (sufficient for browser
reproduction), while the PHPUnit suite updates the option directly.
Verified via temporary revert of callbacks.php to site_url(): the Bedrock
divergence test now fails with the polished message; restoring home_url()
greens the suite on both single-site and multisite.
…ilter caveat Three small additions on top of the regression-test commits: 1. Assert that the redirect_uri query arg is preserved through to the wp-login redirect_to param, in both the standard and Bedrock cases. A future change that successfully matched the request but mangled or dropped redirect_uri would previously slip through. 2. Add test_wp_in_subdirectory_install_matches_with_home_url for the Codex-documented 'Giving WordPress its own directory' pattern -- WP core in /wordpress, public site at root. Different prefix from Bedrock, same shape of divergence. Proves the fix is not a /wp-special case. Renames set_bedrock_siteurl() to set_split_install_siteurl($suffix) so both tests share the helper. 3. Document the home_url() filter dependency in handle_generate_endpoint()'s docblock. Multilingual plugins (WPML, Polylang, TranslatePress) that filter home_url() to prepend locale paths can still cause this match to miss for non-locale-prefixed frontend callers. Notes the likely follow-up direction (REST route migration) so the next person to touch this knows what they're inheriting. Suite is now 5 tests / 14 assertions. Verified red-on-revert: both the Bedrock and WP-in-subdirectory tests fail cleanly when callbacks.php is reverted to site_url(); restoring home_url() greens both single-site and multisite suites.
…est classes
phpunit.xml.dist sets backupGlobals="false", so anything mutated on
$_SERVER and $_GET inside one test class persists to the next. Our
tearDown was doing 'unset($_SERVER[REQUEST_URI])' as cleanup, which
poisoned the cron path that runs during TelemetryCallbacksTests::setUp()
(do_action('muplugins_loaded') -> wp-cron -> reads REQUEST_URI). With
convertNoticesToExceptions=true, the missing key turned into 9 hard
errors on the next test class.
Local --filter=AuthCallbacks didn't surface this because TelemetryCallbacks
was excluded. CI runs the full suite and caught it on WP 6.5 and Nightly.
Fix: snapshot $_SERVER['REQUEST_URI'] and $_GET in setUp; restore them in
tearDown. The world is left as we found it, regardless of what the WP test
bootstrap or earlier test classes had configured.
Verified locally:
- Full suite (composer test, single-site + multisite): 126 tests, 291
assertions, all pass.
- Regression guard preserved: reverting callbacks.php to site_url() still
fails the Bedrock + WP-in-subdirectory tests cleanly.
- Without this fix, the same full suite errors 9 times in
TelemetryCallbacksTests with 'Undefined array key REQUEST_URI' --
matching the CI failure exactly.
josephfusco
approved these changes
May 13, 2026
|
Currently, we do not support the creation of preview deployments based on changes coming from forked repositories. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Bedrock-style installs where WordPress core lives under
/wp/,site_url('/generate', 'relative')returns/wp/generatebut$_SERVER['REQUEST_URI']is still/generate. Thepreg_matchinhandle_generate_endpoint()never fires and the auth-code redirect silently breaks.home_url()returns the public-facing URL — which is whatREQUEST_URIactually contains. Same value assite_url()for standard installs (no observable change there), and the correct value for Bedrock + other split-install layouts (WP_HOME≠WP_SITEURL).Related Issue
Closes #1872
Refiled per @josephfusco's guidance on closed PR #2315 — a new filter is not needed, just the correct URL helper.
Confirm the issue (on canary)
Confirm the fix (this branch)
Run the test suite (corrected from earlier draft)
The new test file is
plugins/faustwp/tests/integration/AuthCallbacksTests.php. It includes aset_bedrock_layout()helper that filterssite_urlto inject/wp/(both relative and absolute URL forms) so the divergence is reproducible without a real Bedrock install. Thetest_bedrock_divergence_matches_with_home_urlcase is the regression guard — verified to fail whencallbacks.phpis reverted tosite_url(), pass with the fix in place.Manual Bedrock reproduction
A small mu-plugin lives at
plugins/faustwp/tests/mu-plugins/mu-bedrock-simulator.php. Drop it into the running container'smu-plugins/to makesite_url()return/wp/<path>whilehome_url()stays at/<path>:Verify the divergence:
Then visit
http://localhost:8080/generate?redirect_uri=https://example.test/— handler matches and redirects towp-login.php.Checklist
canary@faustwp/wordpress-pluginpatch)tests/integration/AuthCallbacksTests.php) — 4 cases incl. Bedrock divergence guardtests/mu-plugins/mu-bedrock-simulator.php) for manual repro./vendor/bin/phpunit -c ./phpunit-multisite.xml.dist --filter=AuthCallbacks)