fix(seo): stop core guessing a destination for a 404 - #159
Merged
Conversation
`redirect_guess_404_permalink()` matches the requested slug as a PREFIX -- `post_name LIKE 'about%'` -- and redirects to whatever comes back first. A reader following a dead link is told the page moved and then shown something else. That is worse than being told it is gone: a 404 is a fact, a wrong 301 is an answer. The query has a leading value and a trailing wildcard, so it cannot use the post_name index, and it runs on every 404 that carries a name. That is a scan any visitor can ask for as often as they like, which is the half of this that is not a matter of taste. Genuine canonical redirects are untouched, and that was checked rather than assumed: the guess is the last thing redirect_canonical() tries (canonical.php:215), after trailing-slash, ?p=ID-to-slug and category-base have each had their turn. Those correct a request that names the right post, and they keep working. On by default, which reverses core. Disabling it loses nothing correct -- it only stops the guessing. The one site it costs is one that renames slugs without leaving redirects behind and relies on the guess to catch the fallout; that site sets the property false. Named `$disable_404_permalink_guess` rather than `$guess_404_permalink` after writing it the other way first. Every flag in this group reads "true means the kit does the thing" -- $disable_xmlrpc, $disable_emojis, $disable_feeds, $disable_self_pingbacks, $disable_file_editing -- and the inverted name also broke the family's own test_no_hooks_registered_when_all_flags_disabled, which was the signal that the polarity was wrong rather than merely unusual. Ported from a downstream project where it had been a hand-rolled add_filter() in functions.php. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JMYy6JHLf4mU4H4Hd47spb (petr@pari.cz)
…guess # Conflicts: # CHANGELOG.md
…pped it The release-stamp workflow cut v1.44.0 at 16:23. PR #158 merged at 16:29, and its Unreleased entry landed under the heading the stamp had just written -- so CHANGELOG.md credited 1.44.0 with $acfml_skip_frontend_field_translation while `git show v1.44.0:src/StarterBase.php` does not contain it. Nobody would have caught that by reading the file. It was found by checking the tag before cutting the next release, which is now the reason to keep doing that. The entry moves to Unreleased, where it is true, and ships with the next version alongside this branch's own change. Carried in this PR rather than its own because the merge with main already put this file in conflict here, and a two-line move does not earn a third pull request. Recorded so the next reader knows why an unrelated section moved in a 404 change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JMYy6JHLf4mU4H4Hd47spb (petr@pari.cz)
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
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.
Ported from a downstream project where it had been a hand-rolled
add_filter()infunctions.php. It is a reasonable default for this stack rather than a project quirk.What core does
redirect_guess_404_permalink()matches the requested slug as a prefix and redirects to whatever comes back first:Two problems, and only the first is a matter of taste.
A wrong 301 is worse than a 404. A reader following a dead link is told the page moved and then shown something unrelated. A 404 is a fact; a guess is an answer, and it is confidently wrong.
The query cannot use the index. Leading value, trailing wildcard. It runs on every 404 that carries a name in the URL — a scan any visitor can ask for as often as they like, with no authentication and no rate limit.
Genuine canonical redirects are untouched
Checked rather than assumed. The guess is the last thing
redirect_canonical()tries:Trailing-slash,
?p=ID-to-slug and category-base have each had their turn before it. Those correct a request that names the right post, and they keep working.On by default, which reverses core
Disabling the guess loses nothing correct — it only stops the guessing. The site it costs is one that renames slugs without leaving redirects behind and relies on the guess to catch the fallout:
Naming
Written as
$guess_404_permalink = falsefirst. Every flag in this group reads "true means the kit does the thing" —$disable_xmlrpc,$disable_emojis,$disable_feeds,$disable_self_pingbacks,$disable_file_editing.The inverted name also broke the family's own
test_no_hooks_registered_when_all_flags_disabled, because "all flags off" would have registered a filter. That failure was the signal the polarity was wrong rather than merely unusual — worth recording, because the test caught a naming problem, not a bug.Tests
Two, both verified load-bearing by removal: flipping the default fails the default test, removing the wiring fails the registration test, and neither touches the other. Added to the existing
RegisterSecurityHardeningHooksTestharness rather than a new file, and the property is registered in itsbareInstanceWithAllFlagsOff()list.1878 tests, PHPStan clean.