Commits
Tarun Sukhani authored fe6d079daa1
fix(docviewer): stop /@docs looping for clients without accept-language (PF-163)
PlayDocumentation.index() derives docLang from the accept-language header and
delegates to page(), which the ControllersEnhancer turns into a redirect.
Reverse routing picks whichever route can bind the args given, so an empty
docLang produced the single-segment /@docs/home — which onRoutesLoaded maps
straight back to index():
Router.prependRoute("GET", "/@docs/{id}", "PlayDocumentation.page"); // line 45
Router.prependRoute("GET", "/@docs/home", "PlayDocumentation.index"); // line 46
prependRoute pushes each route to the front, so line 46 shadows line 45 for
exactly that path and index() redirects to itself forever. Browsers always send
accept-language and land on the two-segment /@docs/{docLang}/home, which is why
this survived; curl, health checks, uptime monitors and crawlers did not.
Default an empty docLang to "en" so the redirect always carries a language
segment. page() already treats "en" as the untranslated documentation/manual/
directory, so the rendered page is byte-for-byte what browsers were getting.
Fixed in the controller rather than by deleting the shadowing route: line 46 is
not redundant — it is what gives the bare /@docs/home path its language
auto-detection. Removing it would have silently dropped localisation for that
URL. The route predates PF-96, which only renamed the /@documentation prefix.
Verified on a scaffolded `play new` app:
before: GET /@docs -> 302, loops to redirect cap
after: GET /@docs -> 200 in 1 redirect (/@docs/en/home)
GET /@docs (Accept-Language en) -> 200, byte-identical to pre-fix
GET /@docs (Accept-Language fr) -> 200 via /@docs/fr/home
GET /@docs/home -> 200 in 1 redirect
`ant test` green.
No automated regression test: docviewer compiles only src/ into its jar, so
app/ controllers are compiled at runtime by ApplicationClassloader and are not
reachable from any JUnit sourceset. Covering this needs docviewer mounted in
framework/test-src/integration/testapp — shared infra every integration test
boots — which is a larger change than this fix warrants.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>