[
https://issues.apache.org/jira/browse/WW-5844?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Lukasz Lenart resolved WW-5844.
-------------------------------
Resolution: Fixed
> 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: 6.12.0, 7.0.0, 7.3.0, 7.4.0
> Reporter: Gouri Sankar A
> Priority: Major
> Fix For: 7.5.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> 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)
