Gouri Sankar A created WW-5844:
----------------------------------
Summary: HttpMethodInterceptor fails open for the implicit
execute() method; restDefaultStack / beanValidationDefaultStack omit httpMethod
entirely
Key: WW-5844
URL: https://issues.apache.org/jira/browse/WW-5844
Project: Struts 2
Issue Type: Bug
Components: Core Interceptors
Affects Versions: 7.4.0, 7.3.0, 7.0.0, 6.12.0
Reporter: Gouri Sankar A
Two compounding gaps in the @HttpGet/@HttpPost verb-restriction control:
(a) HttpMethodInterceptor misses an @Http* annotation in four distinct,
independent ways:
- A method-level annotation on the implicit "execute" method: the
method-level check only runs when proxy.isMethodSpecified() is true, which is
false for an action invoked without an explicit method= -- so @HttpPost placed
directly on execute() is never read for that case. (The separate, unconditional
class-level check still runs regardless of isMethodSpecified(), so a
class-level annotation on the same action is still enforced; this is narrower
than it may first appear.)
- A class-level annotation on a base class is not enforced on a subclass that
doesn't repeat it, and a method-level annotation is lost the same way when a
subclass overrides the annotated method without repeating the annotation --
plain Java annotation semantics, since none of the Http* annotations is
@Inherited. This happens independently of isMethodSpecified(), including with
an explicit method= configured.
- The same loss happens one level further out: an annotation declared on an
interface (a default method, or a class-level annotation on the interface
itself) is not found by an action class that only implements that interface
through an intermediate superclass, rather than directly.
Any of the four reaching an unguarded action means any HTTP verb reaches it.
(b) restDefaultStack (plugins/rest) and beanValidationDefaultStack
(plugins/bean-validation) are the only two bundled interceptor stacks anywhere
in core or any plugin that carry params but omit httpMethod entirely -- every
application extending rest-default or struts-bean-validation gets no verb
rejection at all, regardless of annotations (RestActionMapper does read the
verb to choose which method to route to, but nothing is ever rejected for using
the "wrong" one). Every bundled stack in core and every plugin was enumerated
to confirm this is exactly two gaps, not a wider pattern.
Prevalence: apps/showcase has 156 <action> tags and only 54 use an explicit
method=, so roughly two-thirds rely on the implicit default -- gap (a) is
realistic wherever a developer adds @HttpPost to an execute()-mapped action as
defence-in-depth rather than naming the method.
Live-verified on a throwaway Struts Boot app, loopback only: two action shapes
(implicit execute(), named method) crossed with two package types
(rest-default-, bean-validation-derived) = 4 combinations. Before the patch,
GET reached the action and ran it in all 4 cases (should have been rejected by
@HttpPost in each). After, all 4 GETs returned 405 and the action never ran;
POST was unaffected as a control.
Proposal:
- HttpMethodInterceptor always resolves the invoked method, including the
implicit execute() case, and walks the full class/superclass/interface
hierarchy for the nearest annotation at both the method and class level,
checking every level's own implemented interfaces (not only the leaf class's)
-- Class#getInterfaces() only returns interfaces a class declares itself, not
ones a superclass further up implements. Mirrors the existing
AnnotationUtils.findAnnotation walk shape used elsewhere in the convention
plugin.
- httpMethod added to restDefaultStack and beanValidationDefaultStack.
- A new, generic AccStackInvariantsTest (core) walks every stack definition in
struts-default.xml and asserts any stack carrying params also carries
httpMethod -- a durable regression guard against a third stack developing the
same gap.
Tests: full suites pass with the patch applied -- core 3749/0 failures (2
skipped), rest 245/0, bean-validation 25/0, convention 94/0, json 177/0 --
including the existing HttpMethodInterceptorTest and the WW-5718 stack-ordering
pins (RestDefaultStackTest/BeanValidationDefaultStackTest), both unaffected.
Affected versions: both gaps present, same code pattern, by source inspection,
in the 6.12.0, 7.0.0, 7.3.0 and 7.4.0 tags.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
