WW-5688 Resolve id-bearing REST URIs into the root namespace when declared - #1862
Open
lukaszlenart wants to merge 1 commit into
Open
WW-5688 Resolve id-bearing REST URIs into the root namespace when declared#1862lukaszlenart wants to merge 1 commit into
lukaszlenart wants to merge 1 commit into
Conversation
…en declared RestActionMapper mapped a URI carrying an id into the default namespace while mapping the same action without an id into "/". Since the configuration only fails over from "/" to "" and never the other way round, an action declared in a package with namespace="/" resolved for index but 404'd for show, update and destroy. DefaultActionMapper already handles this: WW-2461 added a rootAvailable check in June 2008, three months before WW-2820 reported the REST symptom, but the fix was never ported to the mapper the REST plugin had forked earlier. Port it, keeping the ordering that computes the action name while the namespace is still empty, since the name is relative to it. The promotion only fires when a package explicitly declares namespace="/" and nothing more specific matched. Convention derives "" or "/sub" and never "/", so applications that do not opt in are unaffected, and because a "/" lookup already falls back to "", the set of resolvable actions is a superset of the previous one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
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.



Fixes WW-5688, split out of the 2008 umbrella WW-2820 (originally reported by Alvin Singh). Of the five claims bundled there, this was the only one that still reproduced; the other four are dispositioned in a comment on that ticket, which is now closed.
The bug
RestActionMappermapped a URI carrying an id into the default namespace, while mapping the same action without an id into/. BecauseRuntimeConfiguration.getActionConfig()only fails over from/to""and never the other way round, an action declared in a package withnamespace="/"resolved forindexbut 404'd forshow,updateanddestroy:GET /dog/GET /dog/1The fix
DefaultActionMapperhas handled this since WW-2461 (June 2008) via arootAvailablecheck — three months before WW-2820 reported the REST symptom. That fix was never ported to the copy ofparseNameAndNamespace()the REST plugin had forked earlier. This PR ports it verbatim, including the ordering that computes the action name while the namespace is still empty, since the name is a substring relative to it.Blast radius
Deliberately narrow:
namespace="/"and nothing more specific matched. Convention derives""or/suband never/, so it cannot trigger this on its own — the bundledrest-showcaseis unaffected./lookup already falls back to"", the set of resolvable actions after the change is a strict superset of the previous one. Nothing that resolved before stops resolving.Tests
RestActionMapperRootNamespaceTest(new) resolves all the way to theActionConfigrather than stopping at the mapping, because the reported symptom is a 404. Confirmed RED before the fix:show/update/destroyfailed whileindexand id-extraction passed, so the tests discriminate exactly this defect.RestActionMapperTest.testParseNameAndNamespaceWithRootPackagecovers the mapper level, including that a longer declared namespace still outranks the root. The pre-existingtestParseNameAndNamespacepins the no-root-package direction, which is unchanged.Full rest plugin suite (124 tests) and core's mapper tests (82 tests) pass.
Not a security fix
Verified before opening: this makes actions less reachable rather than more, and aligning
/dog/1with what/dogalready resolves to exposes no surface the/dogpath does not already expose.Backporting to
support/struts-6-x-xis deliberately left as a separate decision — it is a behaviour change on a maintenance line.🤖 Generated with Claude Code