fix(proxy): redirect stale content URLs to the latest ELPX extraction - #68
Merged
Conversation
Add SDD-0001 and ADR-0001 for redirecting obsolete extraction hashes to the attachment's latest extraction via attachment post meta aliases. Refs exelearning/exelearning#2150
Add failing tests for SDD-0001 ahead of the implementation: - ContentHashAliasesTest: alias registration/resolution repository (fails: class not found). - StaleContentRedirectTest: temporary redirect behavior of the content proxy for retired hashes (fails: class not found), plus passing guards pinning the current safe 404/validation behavior. - ReprocessorTest: retirement ordering and reprocess integration (fails: undefined retire_extraction(); reprocess registers no alias). - RestApiTest: failed saves must create no alias and keep the previous extraction (passing regression guard). Refs exelearning/exelearning#2150
Implement SDD-0001: a request for a retired extraction hash now answers with a temporary (302) same-origin redirect to the equivalent validated file path under the owning attachment's current extraction, instead of a permanent dead link. - New ExeLearning_Content_Hash_Aliases repository persists retired hashes as multi-value attachment post meta (_exelearning_obsolete_hash) with self-alias, shared-current-hash and cross-attachment ambiguity guards, verified persistence, and automatic cleanup on attachment deletion. - ExeLearning_Reprocessor::retire_extraction() registers the alias after the metadata commit and deletes the old extraction only when the alias is verified as stored; refused or failed registration retains the directory. Used by both the REST save and reprocess flows. - ExeLearning_Content_Proxy falls back to the alias lookup only on file_not_found, revalidates the destination path inside the current extraction, preserves RFC3986-encoded query parameters (dropping rest_route), and sends Cache-Control: no-cache on the redirect. - Test fixtures now populate route params via set_url_params(), mirroring how the REST server routes real requests. Fixes exelearning/exelearning#2150
Contributor
Test in WordPress PlaygroundTest the plugin with the code from this branch:
|
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.
Summary
302 Found,Cache-Control: no-cache, must-revalidate).file_not_found.Fixes exelearning/exelearning#2150
Problem
Saving an edited ELPX generates a new extraction hash and removes the old extraction directory. URLs containing the previous hash then return
file_not_foundpermanently — every shared or embedded link breaks on each edit.Design
_exelearning_obsolete_hash) on the attachment that retired it, via the newExeLearning_Content_Hash_Aliasesrepository. Post meta resolves the alias through the attachment identity to its single current_exelearning_extractedvalue (no chains possible), is deleted automatically by WordPress when the attachment is permanently deleted, matches the plugin's "everything in attachment meta" storage model, and adds zero autoloaded state.302 FoundwithCache-Control: no-cache, must-revalidate— the destination is mutable (the attachment can be edited again), so it must never be cached permanently.wp_delete_attachment()behavior); no new cleanup surface, no options-table growth.SDD
docs/architecture/sdd/SDD-0001-stale-content-url-redirects.md(Accepted)docs/architecture/adr/ADR-0001-obsolete-hash-alias-storage.md(Accepted — durable storage/REST-contract decision per repository ADR policy)Test-driven development
test(proxy): cover stale content URL redirects):ContentHashAliasesTest(16 tests — registration/resolution/guards/deletion/no-autoload),StaleContentRedirectTest(17 tests — redirect behavior plus negative-path guards),ReprocessorTestretirement/integration additions,RestApiTestfailed-save guard.ContentHashAliasesTest:Tests: 16, Errors: 16—Error: Class "ExeLearning_Content_Hash_Aliases" not found.StaleContentRedirectTest: all redirect tests errored with the missing class, while the negative-path guards (unknown/invalid hash, traversal, malformed alias, shared hash) already passed, pinning current behavior.ReprocessorTest: 4 ×Call to undefined method ExeLearning_Reprocessor::retire_extraction()plusFailed asserting that an array contains '<hash>'(reprocess registered no alias).ContentHashAliasesTestOK (16 tests, 38 assertions);StaleContentRedirectTestOK (17 tests, 87 assertions);ReprocessorTestOK (27 tests, 79 assertions);RestApiTestfailed-save guard OK (1 test, 4 assertions). Full suite:OK (771 tests, 1629 assertions).Security
ExeLearning_Content_Proxy::get_proxy_url(), the same generator as every proxy URL, including theexelearning_content_originisolated-origin rewrite) and remain on the configured content origin.file_not_found(never afterinvalid_path/access_denied), and the destination is re-validated with the existingvalidate_file_path()(traversal-safe sanitization + realpath containment) against the current hash.http_build_query(…, PHP_QUERY_RFC3986)so no raw user input reaches theLocationheader.Testing
Manual verification
Verified end-to-end against wp-env (pretty permalinks,
wp-jsonURLs):tests/fixtures/test-content.elpx)./wp-json/exelearning/v1/content/<hash1>/index.htmlURL.ExeLearning_Reprocessor::reprocess(), which shares the retire path with the REST save) →<hash2>, then again →<hash3>.curl -I <hash1 URL>?exe-teacher=1→HTTP/1.1 302 Found,Location: /wp-json/exelearning/v1/content/<hash3>/index.html?exe-teacher=1,Cache-Control: no-cache, must-revalidate.curl -I <hash2 URL>→302 Found→<hash3>(both retired hashes redirect directly to the latest hash, no chain).curl -L <hash1 URL>→ final200 OKon the<hash3>URL.200 OKwith the usual security headers, unchanged.404.Plain permalinks (
?rest_route=…) are covered by the unit tests, which run in an environment whererest_url()produces the plain-permalink form; the redirect drops the originalrest_routeargument and preserves the remaining query parameters.Changed files
includes/class-content-hash-aliases.php(new) — obsolete-hash alias repository: guarded registration with read-back verification, single-owner resolution, current-hash refusals.includes/class-content-proxy.php—file_not_found-only redirect fallback (maybe_redirect_stale_hash(),add_preserved_query_args()); everything else untouched.includes/class-elp-reprocessor.php— newretire_extraction()(alias-then-delete, fail-open on persistence failure);reprocess()now uses it.includes/class-exelearning-rest-api.php— the locked save transaction retires the old hash via the reprocessor instead of deleting it unconditionally.exelearning.php— require the new class file.docs/architecture/sdd/SDD-0001-stale-content-url-redirects.md,docs/architecture/adr/ADR-0001-obsolete-hash-alias-storage.md,docs/architecture/{sdd,adr}/records.md— design records.tests/unit/ContentHashAliasesTest.php(new),tests/unit/StaleContentRedirectTest.php(new),tests/unit/ReprocessorTest.php,tests/unit/RestApiTest.php— TDD coverage described above.Compatibility
Requires at least: 6.1,Tested up to: 7.0(readme.txt).Requires PHP: 8.0(readme.txt); tests run on the wp-env PHPUnit environment.Independent security review
An adversarial security/regression review of the full diff (open redirect, traversal-to-redirect conversion, malformed metadata, alias loops, deleted attachments, failed saves, shared-hash hijack, cache lifetime, header injection, permalink modes, isolated content origin, save-lock coverage) found no blocking findings. Non-blocking notes, all pre-existing behavior or cosmetic, were recorded and intentionally left out of scope:
get_proxy_url()does not percent-encode path segments (pre-existing; not exploitable — the file must exist on disk andheader()rejects CR/LF),reprocess()has no per-attachment lock (pre-existing; duplicate alias rows on one attachment are cosmetic since resolution counts distinct attachments), andCache-Controlcould optionally addprivate.Known limitation
When two legacy attachments genuinely shared one content-derived hash and both are later edited, requests for that shared hash cannot be attributed to either attachment without changing the public URL contract; they return the safe 404. Documented in SDD-0001 (“Shared-hash ambiguity”), together with a possible follow-up for the
delete_attachmentdirectory cleanup.