mirror of
https://github.com/apache/struts.git
synced 2026-08-05 14:47:09 +00:00
release/7.3.0-RC1
8373 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Lukasz Lenart
|
2794832317 | [maven-release-plugin] prepare for next development iteration | ||
|
Lukasz Lenart
|
a88fd76364 | [maven-release-plugin] prepare release STRUTS_7_3_0 STRUTS_7_3_0 | ||
|
Lukasz Lenart
|
0fd3d7a2fb |
docs: add creating-version-notes skill and page template (#1826)
* docs: add creating-version-notes skill and page template Covers Version Notes pages on the cwiki for every maintenance line - 6.x and 7.x share one structure, so the release line changes the data, not the process. The published pages show that cloning the previous release's page reliably leaves a half-updated link, differently each time, so the skill starts from a template instead. The template corrects three defects the published pages carry: a malformed code-macro parameter, hard-coded macro ids duplicated across releases, and trailing empty divs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: extend creating-version-notes with the Migration Guide step Exercising the skill on Version Notes 6.11.0 surfaced four rules it was missing: - The page is a child of Migration Guide (13981), which is also the index; a page not listed there is unreachable. Records how to update that section safely and why its version diff renders empty even when the edit landed. - Reconcile through the ticket's linked PR files. WW-5630 reads "Performance Issue SecurityMemberAccess" but was fixed in ConfigParseUtil, so grepping commit subjects or the class in the title wrongly concludes the backport is missing. - Patch-level dependency bumps ship untick eted by design, so a pom version ahead of the ticket text is expected rather than a reconciliation gap. - The Staging Repository block is included on every line, not an open decision. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: cover the GitHub release notes in creating-version-notes A release also has GitHub release notes at the STRUTS_X_Y_Z tag, and the generated body needs two corrections before it is fit to publish. Entries are split into What's Changed and a nested Dependencies section by whether they carry a WW ticket, not by author: a Dependabot PR with a ticket is release content and stays above, while an untick eted dependency bump from anyone moves down. More importantly, the generated Full Changelog range is not trustworthy. GitHub picks the previous tag by reachability, and our release branches get renamed and re-imported, so it reaches too far back - for 6.11.0 it chose STRUTS_6_8_0 and listed 88 entries that had already shipped in 6.9.0 and 6.10.0. The range must be verified with git log PREV..THIS, which works across unrelated histories. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
702280f48f |
WW-5656 docs(conversion): mark ConversionRule.COLLECTION and the Collection_ prefix as deprecated (#1825)
Both have been documented as deprecated since WebWork 2.1.x, but neither carried an actual @Deprecated annotation, so users only ever learned about it from prose or from an INFO log line that fires solely when the fallback is hit. Runtime behaviour is unchanged - the Collection_ fallback keeps working, and COLLECTION remains a legal @TypeConversion rule handled identically to ELEMENT. Call sites that reference either element deliberately carry a suppression and a note saying why. Also deprecates XWorkConverter.CONVERSION_COLLECTION_PREFIX, a second public spelling of the same Collection_ prefix that the framework itself never reads. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
687436f9bf |
WW-5668 Keep the localized-text providers deserializable across a version upgrade (#1824)
Follow-up to #1821. Pins serialVersionUID to the value implicitly computed for the Struts 7.2.1 class shape instead of 1L, so a session serialized by a 7.2.1 node still loads on a 7.3.0 one during a rolling upgrade rather than failing with InvalidClassException. Such a stream carries no value for the new cache settings, and field initialisers do not run during deserialization, so readObject restores their defaults before rebuilding the caches - without that guard it failed with a NullPointerException. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
a6570b769b |
WW-5668 Bound the localized-text provider caches and align request-locale resolution (#1821)
* WW-5668 docs: design spec for bounded i18n caches and request-locale resolution Follow-up to WW-5540. Bound the AbstractLocalizedTextProvider caches via the existing OgnlCache abstraction (configurable struts.i18n.cacheType/cacheMaxSize), and add opt-in request-locale resolution consistency between Dispatcher and I18nInterceptor (struts.locale.validateRequestLocale, default off). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5668 docs: implementation plan for bounded i18n caches and request-locale resolution Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5668 Add remove(key) to the OgnlCache abstraction Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5668 Bound the localized-text provider caches with configurable size Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5668 Fix reassignable-lock hazard and add volatile to i18n cache fields synchronized (bundlesMap) locked on a monitor that rebuildI18nCaches() can reassign; introduce a dedicated bundlesMapLock and lock on that instead. Mark the five i18n cache fields volatile for safe publication across the reassignment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5668 Add opt-in request-locale resolution consistency to Dispatcher Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5668 Keep the localized-text caches transient so the provider stays serializable Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5668 Add missing Javadoc and cache-rebuild coverage for i18n provider Add the one-line Javadoc that sibling fields/setters carry to validateRequestLocale and its @Inject setter in Dispatcher, and add two tests covering StrutsLocalizedTextProvider's serialize/deserialize cache rebuild and cacheType selection behaviour. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5668 Pin explicit serialVersionUID on the localized-text providers Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5668 Suppress false-positive Sonar S3077 on the thread-safe i18n caches Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5668 Drop the unused throws Exception from the new Dispatcher locale tests Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
1218c49224 |
WW-5666 Apply input length limits consistently when reading request bodies (#1819)
* WW-5666 fix(json): apply the input length limit while reading The configured JSON input length limit was evaluated after accumulating each line of input. It is now evaluated as the input is read, in fixed-size chunks, so enforcement no longer varies with the structure of the input. Line terminators are no longer stripped while reading. They are insignificant whitespace between tokens, but an unescaped control character inside a string value is now preserved rather than silently removed. * WW-5666 fix(core): bound the CSP report body read and make the limit configurable CspReportAction read the submitted report body with a single readLine() and had no limit of its own. Read it up to a limit instead, defaulting to 8192 characters and configurable through struts.csp.report.maxSize. A body above the limit is discarded with a warning rather than processed. The limit is injected when the action is built, before the interceptor stack runs, because withServletRequest is invoked by the servletConfig interceptor ahead of staticParams and params. Values that are not usable as a buffer size are ignored with a warning. |
||
|
Lukasz Lenart
|
a07d0e2926 |
docs: add creating-security-bulletins skill, bulletin template, and triage control-case rule (#1820)
* docs: add creating-security-bulletins skill and bulletin template Captures the editorial process for S2-XXX security bulletins, which has been implicit until now, as a companion to the existing triaging-security-reports skill. Triage establishes what is true; this covers what the published page is allowed to say. The load-bearing part is the disclosure budget. Earlier bulletins explained causes and mitigations in enough detail to build working exploits, and the project moved away from that; this writes the rule down and extends it past Problem to Backward compatibility and Workaround, which is where a carefully guarded advisory tends to leak. Also records conventions that were previously tacit: - Affected Software lists voted releases only, never a build that failed its test period, and never a range inferred from git tags - ratings match a definition on the Security Bulletins page, which is the only authority since the four-level naming postdates older advisories - workarounds are verified in source or not published, including the claim that none exists - behavioural claims are derived from the fix diff rather than its commit message, and the fix is confirmed merged before publication - who is *not* affected is stated explicitly, since scoping shrinks the population that has to act bulletin-template.md is the source of truth for page structure and carries the storage-format skeleton; the restricted wiki template becomes a rendered copy of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: require running a supplied PoC and finding the control case Two gaps surfaced by triaging a real report against the skill. The claim table told the triager to trace a PoC through the code but never to run it, even when the reporter supplied a runnable one. Reading and inferring is weaker evidence than executing, and the report that exposed this shipped a JUnit test and the exact command to run it. More importantly, the skill never said to look for the control case. A single odd behaviour is nearly always arguable as intended -- a type that opts into dynamic binding can be read as the developer asking for it. What settles the question is the sibling that behaves correctly under the same input: when one dispatch path rejects an unannotated member and its neighbour does not, the control is incomplete rather than by design, and that divergence is the finding. Two independent triages of the same report both relied on this argument, and neither the skill nor THREAT_MODEL.md prompted for it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
81264063e6 |
WW-5659 Resolve lazy interceptor params per invocation (#1816)
* WW-5659 docs: design for request-scoped lazy interceptor params
WithLazyParams#injectParams resolves ${...} params onto the interceptor
singleton, so concurrent requests can read one another's resolved values.
For ActionFileUploadInterceptor that means allowedTypes, allowedExtensions,
maximumSize and disabled can cross between requests.
Design fixes the contract rather than the one implementer: resolved params
go into a per-invocation holder the interceptor supplies and receives back,
leaving the singleton immutable after init(). Adds InterceptorParams as the
general contract with DisableParams as opt-in support for the disabled param,
and makes unresolvable expressions fail closed instead of silently disabling
validation.
Reported via GitHub PR #1815; that approach (ThreadLocal on the interceptor)
is not adopted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5659 docs: implementation plan for request-scoped lazy params
Six tasks, each independently testable and compiling: new InterceptorParams
and DisableParams types, LazyParamInjector.resolveInto alongside the old
path, a pure refactor onto a single UploadPolicy value object, the contract
switch plus DefaultActionInvocation wiring, fail-closed handling, and test
migration.
Records one deviation from the spec: the unresolved-param rule is applied
unconditionally rather than by introspecting the seeded value, which is not
implementable deterministically for the Long-typed maximumSize. Task 6
updates the spec to match.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5659 docs: clarify test base class constraint in the plan
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5659 feat(core): add InterceptorParams contract and DisableParams holder
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5659 feat(core): resolve lazy params into a holder instead of the interceptor
* WW-5659 docs(core): correct isUnresolved javadoc and pin empty-value fail-closed behavior
isUnresolved cannot distinguish a failed ${...} resolution from an expression that
legitimately evaluates to an empty string; the parser gives no other signal. The
previous javadoc wrongly claimed the raw template let it tell the two apart. Fix
the javadoc to state the actual, intentional rule (fail-closed: treat both as
unusable), and add a test pinning that a legitimately-empty expression is treated
as unresolved rather than written.
* WW-5659 refactor(core): hold upload policy in one value object
Introduce UploadPolicy (extends DisableParams) to consolidate the three
loose maximumSize/allowedTypes/allowedExtensions fields on
AbstractFileUploadInterceptor into a single config-time value object.
acceptFile now takes the effective policy as an explicit parameter
instead of reading interceptor-level state directly.
Pure refactor, no behaviour change: the existing setters still mutate
the shared singleton via configuredPolicy, and ActionFileUploadInterceptor
copies it once per invocation via copyConfiguredPolicy() before calling
acceptFile. This groundwork lets a later change route lazily-resolved
per-request params into the copy instead of the singleton.
* WW-5659 fix(core): resolve lazy interceptor params per invocation
Co-Authored-By: deprrous <sukhbatsuugii2004@gmail.com>
* WW-5659 test(core): cover both lazy-params skip branches and per-invocation disabled
The two skip branches in DefaultActionInvocation#invokeWithLazyParams had no
coverage: deleting either left the whole suite green. The only tests reaching
that method used LazyFoo/LazyFooWithStackParams, which declare no disabled
param, and MockLazyParams did not extend DisableParams, so the holder branch
was unreachable and shouldIntercept was always true.
Make MockLazyParams extend DisableParams and add two action configs that
isolate one branch each:
- LazyFooLazilyDisabled passes disabled as an interceptor-ref param, so it
reaches InterceptorMapping#getParams(), resolves onto the holder, and
exercises the DisableParams branch.
- LazyFooStaticallyDisabled sets disabled on the interceptor definition
instead. InterceptorBuilder only puts interceptor-ref params into the
mapping, so the holder never sees it and it can only be honoured through
ConditionalInterceptor#shouldIntercept.
Verified by deleting each branch in turn: each deletion fails exactly the one
test that targets it, and no other.
Also make testDisabledIsResolvedPerInvocation earn its name. It previously
asserted only that newLazyParams() returns a fresh object, never resolving
anything, and built a MyDynamicFileUploadAction it never used. It now routes
two actions through a real LazyParamInjector#resolveInto of a
disabled=${uploadDisabled} param and pins that one invocation's resolved flag
survives the other's, and that neither reaches the interceptor singleton.
* WW-5659 fix(core): reject uploads when the policy cannot be resolved
* WW-5659 test(core): exercise real lazy param resolution in dynamic upload tests
* WW-5659 fix(core): mark params unusable when a lazy value cannot be applied
resolveInto had two failure branches behaving oppositely. An unresolvable
${...} skipped the write and notified the holder, so a fail-closed holder such
as UploadPolicy could reject the upload. A value the holder's setter could not
accept — a non-numeric String for the Long maximumSize, say — also skipped the
write but notified nothing, leaving the policy reporting isUnresolved() == false
and maximumSize null, which acceptFile reads as "no size limit". The cap was
silently off and the file accepted: fail-open, through a branch the design never
enumerated.
Notify the holder from the catch block too, and record in the spec the general
rule that every path skipping a write must notify, so a future failure mode gets
checked against it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5659 chore(core): harden the policy sets and tidy the lazy params dispatch
UploadPolicy handed out the mutable HashSet built by commaDelimitedStringToSet,
which the copy constructor shares by reference with the configured policy, so a
subclass overriding the protected acceptFile could have rewritten process-wide
config from a request thread. The sets are unmodifiable now.
Also document that unresolved() records `disabled` like any other param, so an
unresolvable disabled expression leaves the interceptor enabled and rejects
every upload; and in DefaultActionInvocation use normal imports for the params
types, make the interceptor local final, word the three skip logs consistently
and identify the interceptor by its mapping name, and note that the name-based
param merge is inherited behaviour whose duplicate-ref handling is questionable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5659 fix(core): allowlist the lazy params holder for OGNL member access
Moving lazy param resolution off the interceptor and onto a per-invocation
InterceptorParams holder changed the OGNL target of the write. The interceptor
is allowlisted at configuration time by XmlDocConfigurationProvider, because it
is named in the configuration; the holder is named nowhere, so with the shipped
default struts.allowlist.enable=true SecurityMemberAccess refused every setter,
resolveInto's fail-closed handling marked every param unresolved, and
ActionFileUploadInterceptor rejected every upload.
Register the holder's own class hierarchy through ProviderAllowlist when the
interceptor is built, keyed by the holder class so repeated builds collapse onto
one entry. Only the holder's class, superclasses and interfaces are registered -
the setter may be declared on any of them and SecurityMemberAccess checks both
the target and the declaring class. Object is filtered out: it says nothing
about the holder and is excluded by default anyway. No package is allowlisted.
Also run Interceptor#init() before the holder is obtained, so newLazyParams()
sees a fully initialised interceptor.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5659 test(core): prove lazy params resolve with the OGNL allowlist enabled
Every other core test runs with struts.allowlist.enable=false - StrutsTestCaseHelper
turns it off by default and XWorkTestCaseHelper never loads default.properties -
so no core test could see the holder being blocked by SecurityMemberAccess. Only
the showcase DynamicFileUploadTest integration test exercised the production
setting, which is why the regression reached CI.
This test boots the dispatcher with the allowlist enforced and asserts both that
the holder hierarchy is registered at configuration time and that a ${...} param
actually lands on the policy rather than being reported unresolved. Reverting the
registration in DefaultInterceptorFactory fails it with the same
"Declaring class [UploadPolicy] ... is not allowlisted" warning seen in the
showcase failure.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5659 fix(core): stop an unresolvable disabled param from voiding the upload policy
UploadPolicy#unresolved recorded every param name, disabled included, so an
unresolvable <param name="disabled">${...}</param> marked the whole policy
unusable and rejected every upload of the invocation. That is not a safe
default: disabled is not a validation dimension. Its unresolved value is simply
false, which leaves the interceptor running and the rest of the policy intact,
so it cannot relax validation - recording it only invents a second failure mode.
Exclude it from the tracking that gates isUnresolved(), via a new
DisableParams#DISABLED_PARAM constant, and replace the javadoc that defended the
old behaviour. A param that is a validation dimension still voids the policy,
including when it fails alongside disabled.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5659 docs(core): state what the lazy param injector actually did
The two WARN messages in resolveInto claimed consequences the injector does
not control. The ReflectionException branch said the params were 'marked
unusable', but InterceptorParams.unresolved is a defaulted no-op, so only a
holder that overrides it - UploadPolicy does - degrades at all. The
unresolved-expression branch said the configured value was kept, which reads
as a sensible fallback when it is normally the unevaluated ${...} literal
applied at build time.
Both now report only the injector's own actions: the value was not written
and the holder was notified. The nuance about what the holder retains moves
to the javadoc, where there is room to state it accurately.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5659 feat(core): reject unknown lazy interceptor params at configuration time
A param name that no property on the params holder can accept was only
noticed per request: resolveInto caught the ReflectionException, warned, and
notified the holder - which for UploadPolicy means rejecting every upload of
every request behind a WARN. The names are fully known when the configuration
is parsed, so DefaultInterceptorFactory now fails with a ConfigurationException
naming the interceptor, the param and the holder type.
Only the interceptor-ref params are checked. InterceptorBuilder passes that
same map on to the InterceptorMapping, and DefaultActionInvocation.mergedParams
feeds it to resolveInto, so it is exactly the set that reaches the holder.
Params on the <interceptor> definition are applied to the interceptor instance
and never reach the mapping; checking them too would reject working config,
<param name="disabled"> on a definition being the obvious case.
The runtime handling stays as defence in depth. ConfigurationException is now
rethrown rather than swallowed by the generic catch, so the operator reads the
param name instead of "Caught Exception while registering Interceptor class".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5659 refactor(core): drop the deprecated single-arg executeConditional
The overload lost its last caller when the mapping name became available at
the call site, so an existing subclass override would have compiled and then
never run again - silently dead code, worse than a compile error. This branch
already changes the protected acceptFile signature, so keeping the one-arg
form for source compatibility was not consistent either.
Covered by a test asserting the surviving two-arg form is the extension point
and receives the mapping name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5659 fix(core): keep configuration order when merging lazy interceptor params
mergedParams built a HashMap, so the order the configuration carries was
discarded and the order params were applied to the per-invocation holder was
whatever hashing produced. LinkedHashMap makes it deterministic and matches
the LinkedHashMap the InterceptorBuilder already assembles.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* WW-5659 fix(core): keep interceptor params serializable
Interceptor extends Serializable, so an interceptor holding its configured
params as a field must hold something serializable. The fields UploadPolicy
replaced were a Long and two HashSets, all serializable; the holder was not,
which silently broke serialization of every file upload interceptor.
Fix it on the contract rather than the field: InterceptorParams now extends
Serializable, so every holder inherits the requirement. Marking the field
transient would instead have dropped the configured policy on deserialization.
Also renames four test locals that shadowed the interceptor field and drops
a throws clause that could not be reached, both reported by SonarCloud.
* WW-5659 test(core): hoist the params map out of the assertThatThrownBy lambdas
Each lambda called both params(...) and buildInterceptor(...), so a throw from
the helper would have satisfied the assertion just as well as one from the code
under test. Building the map first leaves one throwing call per lambda.
Reported by SonarCloud (java:S5778).
* WW-5659 fix(core): stop seeding the interceptor with raw lazy expressions
A ${...} param is resolved per invocation into the params holder, so applying
its raw text to the interceptor at configuration time only seeded an
unevaluated literal - allowedTypes held "${uploadConfig.allowedMimeTypes}",
matching no content type - or failed conversion outright for a typed property
such as the Long maximumSize.
Withhold those params at build time; static params still apply and still seed
the holder, which is what a lazy param falling back has to fall back to. Also
makes InterceptorParams.unresolved's javadoc about the retained value honest.
Idea from @deprrous in GitHub PR #1815.
Co-Authored-By: deprrous <sukhbatsuugii2004@gmail.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: deprrous <sukhbatsuugii2004@gmail.com>
|
||
|
dependabot[bot]
|
26141511f8 |
build(deps): bump ossf/scorecard-action from 2.4.3 to 2.4.4 (#1818)
Bumps [ossf/scorecard-action](https://github.com/ossf/scorecard-action) from 2.4.3 to 2.4.4. - [Release notes](https://github.com/ossf/scorecard-action/releases) - [Changelog](https://github.com/ossf/scorecard-action/blob/main/RELEASE.md) - [Commits](https://github.com/ossf/scorecard-action/compare/4eaacf0543bb3f2c246792bd56e8cdeffafb205a...2d1146689b8cda280b9bc96326124645441f03bc) --- updated-dependencies: - dependency-name: ossf/scorecard-action dependency-version: 2.4.4 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
d52ffed7ff |
build(deps): bump github/codeql-action from 4.37.2 to 4.37.3 (#1817)
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.37.2 to 4.37.3. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/v4.37.2...v4.37.3) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.37.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Lukasz Lenart
|
94a8fcb26c |
WW-3784 Order annotated wildcard actions most-specific-first (#1813)
* WW-3784 docs: design for specificity-ordered wildcard matching in annotated actions Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3784 docs: implementation plan for annotated wildcard specificity ordering Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3784 feat(convention): add action-name specificity comparator Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3784 fix(convention): add Apache License header to test file Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3784 feat(core): add PackageConfig.Builder.reorderActionConfigs Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3784 docs: add javadoc for PackageConfig.Builder.reorderActionConfigs Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3784 feat(convention): order annotated wildcard actions most-specific-first Sorts each convention-built package's action configs by pattern specificity so a specific pattern (some/usefull/*) is matched before a general one (some/*), regardless of class-scan order. Also makes convention action ordering deterministic. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3784 docs: correct wildcard cross-segment claims and note comparator limitations The spec incorrectly stated that WildcardHelper's single `*` is greedy and crosses `/`, and that `some/*` shadows `some/usefull/*`. Verified against WildcardHelper.java and NamedVariablePatternMatcher.java: only `**` crosses `/`, so those two patterns are actually disjoint (different segment counts) and never compete for the same request. Correct the Problem narrative, ticket example, and matcher bullets to state this accurately, and document two known limitations of the specificity comparator (raw wildcard-token-count key can misrank `**` ahead of narrower multi-token patterns; parent-package actions bypass sorting). Also add a test asserting the natural-order alphabetical tiebreak key. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3784 test(convention): prove specificity ordering fixes wildcard shadowing end-to-end Adds an end-to-end routing test driving the production reorder (PackageConfig.Builder.reorderActionConfigs + ActionNameSpecificityComparator) through the real ActionConfigMatcher/WildcardHelper. some/** and some/usefull/* genuinely overlap for some/usefull/sleeping (** crosses '/'), so the test asserts the general pattern shadows the specific one when registered first, and that specificity ordering makes the specific action reachable again. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
6f802987f5 |
WW-3871 Derive ConversionRule prefixes for @TypeConversion keys (#1812)
* WW-3871 docs: add design spec for @TypeConversion key derivation Specifies deriving the ConversionRule prefix for @TypeConversion keys at class, method and field level via a single resolver, adds ElementType.FIELD as a target, and records the break/continue and empty-key fixes in the same code block. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * WW-3871 docs: note interaction with the 7.3.0 converter mapping cache Records that addConverterMapping runs inside the computeMappingIfAbsent builder introduced by WW-5539, which executes outside any lock, so the new field pass adds no deadlock risk but must stay side-effect free. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * WW-3871 docs: add implementation plan for @TypeConversion key derivation Seven TDD tasks covering ConversionRule#prefix(), the shared resolveKey helper, class- and field-level derivation, the break/continue and empty-key fixes, an end-to-end binding proof and the Javadoc updates. Refines the spec's resolveKey signature to take the two annotation attributes rather than the annotation instance, so it can be unit tested directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * WW-3871 feat(core): add ConversionRule#prefix() owning the rule-to-prefix table * WW-3871 refactor(core): split addConverterMapping into per-source passes * WW-3871 feat(core): derive conversion mapping keys through a single resolver * WW-3871 fix(core): derive class level conversion keys and stop dropping later entries * WW-3871 feat(core): support @TypeConversion on fields * WW-3871 test(core): assert bare conversion keys bind through the action lifecycle Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * WW-3871 docs(core): document conversion key derivation and field level support * WW-3871 docs(core): add deprecated Collection_ prefix to parameter table * WW-3871 fix(core): widen resolveKey idempotence guard against any rule prefix resolveKey only recognized a key as already-prefixed if it started with its own declared rule's prefix. COLLECTION and ELEMENT are interchangeable throughout the conversion pipeline (DefaultConversionAnnotationProcessor handles them in the same branch, DefaultObjectTypeDeterminer.getElementClass reads Element_ then falls back to the deprecated Collection_), so key="Element_users" with rule=COLLECTION silently doubled to Collection_Element_users instead of being left alone, losing the mapping. Match against every known rule's prefix instead. Also documents two related precedence subtleties surfaced during review: processFieldAnnotations' Javadoc now notes that an inherited method can claim a key before a subclass's own field annotation is considered, since getMethods() includes inherited methods and runs first; and the unresolvable-key WARN in processMethodAnnotations now names the method's declaring class rather than the class being scanned, since getMethods() can surface the same inherited method at every level of the hierarchy. Design spec section 2 updated to match the implementation. * WW-3871 docs(core): correct TypeConversion Javadoc property attribute and determiner package Two pre-existing errors in the block this ticket's commits already touch: the APPLICATION example used a non-existent "property" attribute where "key" is the working form (see ConversionTestAction.java:97), and the rule() Javadoc pointed at org.apache.struts2.util.DefaultObjectTypeDeterminer instead of the actual org.apache.struts2.conversion.impl package. * WW-3871 test(core): cover key-prefix crossover, empty class-level key, and KeyProperty_ end-to-end binding - testResolveKeyLeavesAnAlreadyPrefixedKeyAlone: add the COLLECTION/ELEMENT crossover cases that demonstrate the resolveKey guard fix (fail before, pass after). - New EmptyKeyConversionAction fixture plus testClassLevelEmptyKeyRegistersNoMapping: a class-level @TypeConversion with no key must be skipped, not registered under "". This was the one behavioural bullet in the spec's test plan with no coverage. - MyBeanActionTest.testBareConversionKeysBindTheSameWayAsPrefixedOnes: add an assertion that the bare KeyProperty_ derivation actually binds the list index onto the created bean's id property end to end, not just that a converter mapping exists. * WW-3871 fix(core): skip APPLICATION-scoped @TypeConversion with no explicit key Method- and field-level @TypeConversion(type = APPLICATION) with no key previously derived a member name (e.g. a setter's property name) and registered it in the global default converter map via addDefaultMapping. That map is only ever read by class name (lookup(String, boolean) and lookup(Class)), so the entry was permanently unreachable. Skip it before deriving a name, logging a WARN naming the declaring class and member; the class-level pass already handled this correctly via resolveKey returning null. Adds a fixture and tests proving no default mapping is registered under the derived member name in either pass. * WW-3871 docs(core): fix broken TypeConversion Javadoc example and align spec TypeConversion's example class declared `users` twice (once unannotated, once again at its annotated field), so the sample no longer compiled as written; drop the earlier, redundant declaration. The same example's setConvertInt showed @TypeConversion(type = APPLICATION) with no key - exactly the case the previous commit's XWorkConverter fix now skips. Drop the type attribute so it reads as a class-scoped conversion, matching the corrected ConversionTestAction fixture. The correct APPLICATION example further down (execute(), key = "java.util.Date") is untouched. Also records the APPLICATION no-key skip rule in the design spec's carve-out paragraph so spec and code agree. * WW-3871 fix(core): dedupe method-pass WARN logging for inherited @TypeConversion processMethodAnnotations iterates clazz.getMethods(), which includes inherited public methods, and buildConverterMapping calls it once per class in the hierarchy. A single misconfigured @TypeConversion on a base class method was therefore logging its WARN once per subclass level. Gate both WARN call sites on method.getDeclaringClass() == clazz so each fires exactly once, at the level that owns the method; the derivation/registration logic keeps running on every visit unchanged. Adds a small permanent test proving the gate is logging-only: an inherited annotated setter still resolves and registers through a subclass that overrides nothing. * WW-3871 docs(core): clarify field-name key default and dedicated-annotation precedence Two gaps in the @TypeConversion Javadoc, both newly relevant now that the annotation targets fields: - The key() default on a field is the field name, not the JavaBean property name (processFieldAnnotations uses field.getName()). A field like _users backing property users would otherwise derive CreateIfNull__users, a key DefaultObjectTypeDeterminer never looks up. - org.apache.struts2.util's dedicated field annotations (@Key, @Element, @KeyProperty, @CreateIfNull) are consulted by DefaultObjectTypeDeterminer before it falls back to the converter mapping @TypeConversion populates, so a dedicated annotation silently wins over an equivalent @TypeConversion on the same property. Verified against getAnnotation/getElementClass/ getKeyProperty in DefaultObjectTypeDeterminer before documenting it. * WW-3871 docs(core): note COLLECTION derives the deprecated Collection_ prefix ConversionRule.COLLECTION.prefix() intentionally returns Collection_, the spelling DefaultObjectTypeDeterminer treats as deprecated and logs an INFO about on every fallback hit, kept for compatibility with existing annotations. Document that the derivation is deliberate and point readers at ELEMENT as the current form. * WW-3871 refactor(core): extract shared annotation-registration pipeline processMethodAnnotations and processFieldAnnotations were the same five-step pipeline (skip non-@TypeConversion, skip APPLICATION-scoped without a key, derive the name, resolve the key, register unless already mapped) written twice, driving SonarCloud S3776 cognitive complexity to 26 and 21 respectively and triggering three S135 multiple-break/continue findings. Extract steps 2-5 into a private registerAnnotatedMember(mapping, tc, Member, fallbackName, logSkips) helper that both passes delegate to. Each pass is now just its loop plus one instanceof check. The method pass keeps its per-declaring-class log gate (getMethods() revisits inherited methods once per hierarchy level); the field pass always logs, since getDeclaredFields() is visited once per class. The two WARN wordings, which differed only in a trailing clause, are merged into one message accurate for both a method and a field. No change to the registered mapping, pass order, or precedence for any class - verified via the existing XWorkConverterTest, AnnotationXWorkConverterTest, MyBeanActionTest, and ConversionRuleTest suites (92 tests, same count and same triggering warnings before and after) plus the full core module suite (3043 tests). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * WW-3871 docs(core): fix inaccurate and self-contradicting TypeConversion key Javadoc @Key, @Element, @KeyProperty and @CreateIfNull are @Target({FIELD, METHOD}), not field-only, and the same paragraph already says they are read from the field, setter and getter - drop "field" from "dedicated field annotations". Fold the field-vs-property-name correction into key()'s opening sentence instead of stating "defaults to the property name" and rebutting it three lines later, and align the parameters table row for key with the same rule. * WW-3871 docs(core): clarify XWorkConverter annotation-registration logging Give the success DEBUG the same [declaringClass#member] shape the three skip messages already use, instead of logging the bare member name that identifies neither the class nor whether it was a method or a field. Reword the "already mapped" DEBUG so it covers its commonest trigger - the same annotation seen one hierarchy level down, not just a genuinely higher-precedence source. Note in the logSkips comment that buildConverterMapping only visits each class' direct interfaces, so a misconfigured annotation declared on a super-interface method never gets logged at all, even though registration is unaffected. Also drop a stray extra blank line. No behavioural change: registration/derivation logic is untouched. * WW-3871 test(core): make inherited-method-annotation test diagnostic testInheritedMethodAnnotationStillRegistersThroughASubclass previously asserted nothing the logSkips gate could break: the hierarchy walk always reaches InheritedMethodConversionAction itself, where declaringClass == clazz, so the key registers there regardless of whether registration is (wrongly) gated alongside logging. The test passed identically with logSkips hardcoded true or false. Give InheritedMethodConversionSubAction a contesting field annotation for the same property the inherited setter claims. The inherited method annotation registers at the subclass level - before the subclass's own field pass runs - so its value must keep winning; that is the invariant documented on processFieldAnnotations, and it is exactly what gating registration would break, since the subclass field would start winning over the inherited method annotation instead. Verified: temporarily wrapping the registerAnnotatedMember call in processMethodAnnotations with `if (logSkips)` makes this test fail (expected:<true> but was:<false>); reverting it passes again. Mutation was not committed. Corrected both Javadocs, which overclaimed what the old assertion proved. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
d906f23448 |
WW-3530 Fix visitor-validator cache-key collision under wildcard actions (#1811)
* WW-3530 docs: add design spec for visitor-validator cache-key fix Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3530 docs: add implementation plan for visitor-validator cache-key fix Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3530 test(core): cover visitor-validator cache-key context handling under wildcard actions Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3530 fix(core): keep visitor-validator context in cache key under wildcard actions Apply the wildcard config-name substitution only when validating the action's own class. Visited objects carry a stable, explicit visitor context that must remain part of the cache key, otherwise two visitor validators on one field with different contexts collide and the second is silently dropped. Fixes https://issues.apache.org/jira/browse/WW-3530 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3530 docs: document <s:form> render-path caching limitation Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3530 docs: correct WW-2996 scope claim and note default-context visitor limitation Final-review finding: default-context visitor validators under wildcard actions key on the volatile resolved action name for the visited class, reintroducing bounded WW-2996-style cache growth (memory only; correct validators still load). Correct the 'WW-2996 untouched' wording to 'untouched for the action's own class', document the subpath as an accepted limitation folded into the follow-up ticket, and clarify that end-to-end visitor execution is covered by existing visitor suites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3530 chore(core): add DEBUG logging for validator cache-key branch decision Log the built key together with clazz, context, validatingActionClass, wildcard, and the action config name, so the wildcard-vs-visited-object branch taken in buildValidatorKey can be diagnosed at runtime. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3530 test(core): use assertNotEquals for cache-key inequality; fix comment grammar Address SonarCloud S5785 (assertFalse+equals -> assertNotEquals) and a Copilot grammar nit in the WW-4536 comment. DEBUG logging kept as-is per author decision. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
532ca7f864 |
WW-2934 Skip field validators when a field has a conversion error (opt-in) (#1810)
* WW-2934 docs: add design spec for skipping validators on conversion error Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-2934 docs: add implementation plan for skipping validators on conversion error Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-2934 feat(core): skip field validators on conversion error behind opt-in flag Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-2934 test(core): cover annotation manager + document conversion-error skip exemption Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-2934 test(core): assert custom conversion message survives and cover nested field skip Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
0b2bc2be14 |
WW-3427 Add regression test for conversion errors on aliased properties (#1814)
* WW-3427 test(core): cover conversion errors on aliased properties Reproduce the WW-3427 scenario: an aliased property whose custom TypeConverter throws TypeConversionException. AliasInterceptor already reports such errors (setReportingConversionErrors on the secure child stack, then copies conversion errors back to the original ActionContext), but nothing exercised the alias + conversion-error path. The test drives an action through params -> alias -> conversionError and asserts the failure surfaces both in ActionContext.getConversionErrors() and as a field error, confirming WW-3427 is fixed. Removing the copy-back in AliasInterceptor makes it fail with "swallowed", proving it guards the behavior. Test-only; no production changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-3427 test(core): add Apache license header to conversion.properties RAT flagged the new test resource as having an unapproved license. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
12015d0bf5 |
WW-5540 Add caching to AbstractLocalizedTextProvider (#1808)
* WW-5540 docs: add caching design spec for AbstractLocalizedTextProvider Design for caching the class/package hierarchy traversal result in findText, keyed on (classloader, class name, textKey, locale). Caches the raw resolved pattern (or a NOT_FOUND marker) only; translation and formatting stay per-call. Wires invalidation into the existing reloadBundles/clearBundle/clearMissingBundlesCache sites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: add implementation plan and refine spec Add the 3-task TDD implementation plan and record the formatWithNullDetection fall-through decision in the spec. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: deprecate+delegate findMessage/getMessage in plan Resolve pre-flight duplication/dead-code finding: old traversal helpers delegate to the raw twins and are marked @Deprecated instead of being duplicated. Add a direct characterization test for the findMessage delegator. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 refactor(core): split raw message resolution from formatting Add getRawMessage/formatMessage and a raw twin findMessageRaw. Re-express getMessage via formatMessage and make findMessage delegate to findMessageRaw + formatMessage; deprecate both as legacy extension points superseded by the raw-resolution path. The deprecated findMessage triggers the bundle reload on entry, preserving the reload side effect the old getMessage-per-probe walk provided. Groundwork for the traversal caches. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: refine Task 1 plan (deprecate/delegate + reload-on-entry) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 perf(core): cache class-hierarchy text resolution Cache the class/interface/superclass traversal in findText keyed on (classloader, class name, key, locale), storing the raw pattern or a NOT_FOUND marker. Formatting stays per call and falls through to the next tier when a cached pattern formats to null. Invalidated on reloadBundles/clearBundle/clearMissingBundlesCache; reload is hoisted to the top of findText so caches are cleared before they are read. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: draft follow-up ticket for null-control-flow cleanup Capture the deferred result-wrapper refactor (raised during WW-5540) as a ready-to-file Jira draft; keep WW-5540 focused on caching. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 perf(core): cache package-hierarchy text resolution Cache the *.package traversal in findText the same way as the class hierarchy, with the same keying, fall-through, and invalidation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 test(core): tighten localized-text cache tests Assert single cache entry in the per-call-format tests (proves the raw pattern is cached, not the formatted result), and mirror the package-cache clearBundle/clearMissingBundlesCache invalidation test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: note devMode null-valueStack eager-reload edge Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: link follow-up doc to filed ticket WW-5655 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 chore(core): add ASF license header to CacheFixture.properties Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 chore(core): add since/forRemoval to @Deprecated annotations Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: drop follow-up draft superseded by WW-5655 The ticket is filed; the draft's content now lives in WW-5655 itself. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 fix(core): address fresh-eyes review findings - Document that the deprecated getMessage/findMessage are no longer invoked by findText, and name formatMessage as the override point - Fall back to the ActionContext-based reloadBundles() when findText is called without a value stack, so the RELOADED flag is tracked and the caches can warm on that path in reload/devMode - Narrow resolveClassHierarchyRaw/resolvePackageHierarchyRaw to package-private (the cache key omits indexedKey, which is safe only when derived from textKey as the internal call sites do) - Suppress java:S2129 on the NOT_FOUND identity sentinel Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: strip stray NUL bytes from design spec Two literal NUL bytes in the sentinel example made git/GitHub treat the whole markdown file as binary and unreviewable in the PR UI; align the example with the shipped sentinel name. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 test(core): cover ModelDriven tier, per-locale keys, indexed keys Close the review-noted coverage gaps: the ModelDriven tier resolves via the shared class-hierarchy cache (action miss + model hit), each locale gets its own cache entry backed by a new _de fixture bundle, and indexed keys (name[N] -> name[*]) resolve and cache per full textKey. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 fix(core): address Copilot review comments - Partition the caches by System.identityHashCode of the context classloader so a custom ClassLoader overriding hashCode() cannot collide or collapse the per-loader partitions - Derive the indexed key inside the resolvers (miss-only) instead of accepting it as a parameter, so the cache key trivially covers every input that influences the resolution result Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 fix(core): resolve SonarCloud quality-gate findings - Suppress S4973 on isNotFound: the identity comparison against the non-interned NOT_FOUND sentinel is the design, not a bug - Reduce findMessageRaw cognitive complexity (S3776) by extracting getRawMessageWithAlternate, reused by the package walk - Add missing @Override annotations and suppress the deliberate deprecated-delegator call in the test helper (S1161, S5738) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 fix(core): make findMessageRaw cycle guard effective The `checked` set tested `contains(clazz.getName())` but never added the class, so the diamond-interface cycle guard was a no-op (a latent issue inherited from the original findMessage). Add the class name after the contains-check so repeated interface branches aren't re-traversed. Behavior is unchanged (lookups are idempotent); this only avoids redundant recursion. The deprecated findMessage delegates here, so it's fixed too. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs(core): reference WW-5658 in deprecated method javadoc Point the @deprecated javadoc of getMessage and findMessage at WW-5658, the ticket tracking their removal in the next major release. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
833220346c |
WW-5474 Count files only for maxFiles, add maxParameterCount (#1806)
* WW-5474 docs(multipart): design for files-only maxFiles + maxParameterCount Spec for correcting struts.multipart.maxFiles to count file parts only (consistently across the jakarta and jakarta-stream parsers) and adding struts.multipart.maxParameterCount to cap non-file form fields, restoring the DoS guard the old accidental total-part cap provided. Fail-closed on breach. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5474 docs(multipart): implementation plan for maxFiles/maxParameterCount Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5474 fix(multipart): count files only for maxFiles, add maxParameterCount (jakarta) The jakarta parser passed maxFiles to commons-fileupload2 setMaxFileCount, which counts every part (fields + files), so maxFiles wrongly limited total parameters. Enforce a files-only count and non-file field count in Struts, failing closed on breach; keep a total-parts commons backstop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5474 fix(multipart): honor -1 unlimited sentinel in total-parts backstop prepareServletFileUpload applied the total-parts backstop whenever both maxFiles and maxParameterCount were non-null, without checking for the -1 "unlimited" sentinel already honored by enforceMaxFiles/enforceMaxParameterCount. With maxFiles=-1 and maxParameterCount=256, maxParts computed to 255 and was passed to commons-fileupload2's setMaxFileCount (which counts ALL parts), wrongly rejecting large file-only uploads. Only apply the backstop when both limits are finite (non-null and >= 0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5474 fix(multipart): apply files-only maxFiles + maxParameterCount to stream parser Replace the field-name-based exceedsMaxFiles with the shared files-only enforcement and add parameter-count enforcement, matching the jakarta parser and failing closed on breach. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5474 fix(multipart): track all parsed items to avoid temp-file leak on fail-closed breach servletFileUpload.parseRequest() fully materializes every part - spilling large ones to disk - before processUpload() iterates over the result. The loop only added each DiskFileItem to diskFileItems as it was reached, so when enforceMaxFiles/enforceMaxParameterCount threw mid-loop on a breach, every item positioned after the breaching one was never registered for cleanup. With no FileCleaningTracker on the factory, cleanUp() had no way to reclaim those temp files, leaking disk space on the hardening path. Materialize the parsed list once and register all items for cleanup before processing so cleanUp() reclaims every temp file regardless of where enforcement aborts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5474 fix(multipart): guard debug logging in enforce helpers (Sonar S2629) Wrap the LOG.debug calls in enforceMaxFiles/enforceMaxParameterCount with isDebugEnabled() so normalizeSpace() is not evaluated when debug is disabled, matching the exceedsMaxStringLength pattern in the same class. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5474 fix(multipart): address Copilot review - parser parity + overflow guard - JakartaMultiPartRequest: only count/enforce a file part toward maxFiles when it has a non-null field name, matching JakartaStreamMultiPartRequest's accept criteria (defensive: commons-fileupload2 already drops parts without a name attribute before parseRequest returns, so the two parsers stay consistent regardless). - AbstractMultiPartRequest: compute the total-parts backstop with Math.addExact and clamp to Long.MAX_VALUE on overflow, so extremely large configured limits cannot wrap negative and silently disable the commons backstop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5474 fix(multipart): log "processing a form field" only for form fields Move the debug log into the isFormField branch so file parts are not mislabelled; the file branch already logs "Processing a file". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
c3e9db1b21 |
WW-5413 Avoid writing small in-memory multipart uploads to disk (#1805)
* WW-5413 docs(core): design for in-memory multipart upload optimization Lazy-materializing UploadedFile plus a new getInputStream() accessor so small (in-memory) uploads no longer eagerly write a temp file, while getContent() keeps returning a File for backward compatibility. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5413 docs(core): implementation plan for in-memory upload optimization Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5413 feat(core): add UploadedFile.getInputStream() streaming accessor Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5413 feat(core): add lazily-materializing StrutsInMemoryUploadedFile Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5413 fix(core): make StrutsInMemoryUploadedFile serializable and thread-safe * WW-5413 refactor(core): drop eager temp-file write for in-memory uploads * WW-5413 test(core): cover deferred-write behavior for in-memory uploads Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5413 perf(core): avoid materializing in-memory uploads during interceptor validation * WW-5413 chore(core): clean up partial materialization and cover isMissing() * WW-5413 docs(core): sync design/plan with interceptor fix and deviations Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5413 chore(core): deprecate now-unused STRUTS_MESSAGES_INVALID_CONTENT_TYPE_KEY Mark the orphaned constant @Deprecated(forRemoval = true) instead of leaving it silently unused. The message key it referenced was only emitted from an unreachable block in acceptFile() that was removed with the in-memory upload optimization. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5413 test(core): cover materialization failure and getInputStream default branches Address review follow-ups on PR #1805: - document that processFileField's retained 'throws IOException' is intentional (subclass source compatibility), not an oversight - add a negative test: getContent() on an unwritable save dir throws StrutsException, stays unmaterialized, and leaves no partial file behind - cover the UploadedFile.getInputStream() default File branch and the no-content IOException branch Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5413 fix(core): address SonarCloud and Copilot review findings - materialize() now writes with StandardOpenOption.CREATE_NEW and fails closed if the target already exists, so a pre-planted file/symlink is never overwritten or followed (Copilot security note) + regression test - defensively copy the content byte array on construction and reject null content, so the instance owns its bytes and cannot observe caller mutation (Copilot / review) - delete() uses Files.deleteIfExists and logs the real cause on failure instead of a silent File.delete() boolean (Sonar MAJOR) - reorder field modifiers to JLS order 'transient volatile' (Sonar) - tests: assertThat(dir).isEmptyDirectory() instead of listFiles().isEmpty() (Sonar); drop unused DiskFileItem import (Sonar) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
8a5323fbdf |
WW-5539 Concurrency performance enhancements (#1799)
* WW-5539 docs: add concurrency performance enhancements design Design for removing coarse locks from XWorkConverter, DefaultActionValidatorManager and StrutsTypeConverterHolder in favour of concurrent collections. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5539 docs: add concurrency performance implementation plan Five tasks derived from the approved design: make StrutsTypeConverterHolder concurrent, add the computeMappingIfAbsent SPI method, remove the locks from XWorkConverter and DefaultActionValidatorManager, then benchmark and raise the PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5539 docs: make conditionalReload store its rebuilt mapping buildConverterMapping no longer stores its result, so reload mode would have rebuilt from disk on every request without ever caching. * WW-5539 Make StrutsTypeConverterHolder collections concurrent The holder is a container singleton whose HashMaps were read without any lock by XWorkConverter.lookup() while being written elsewhere, risking lost updates and torn reads during resize. Null TypeConverters are now ignored with a warning rather than stored, since ConcurrentHashMap forbids null values and a null converter left the holder in an inconsistent state. * WW-5539 Rename test to match what it actually covers The method exercised only the unknown-mapping cache, not noMapping. * WW-5539 Add TypeConverterHolder#computeMappingIfAbsent Adds an atomic build-once-and-cache operation so callers no longer need check-then-act around the class mapping cache, and deprecates the three primitives it subsumes: getMapping, addMapping and containsNoMapping. The method is a default method delegating to those primitives, so third-party TypeConverterHolder implementations keep working unchanged. * WW-5539 Deduplicate the no-mapping path in computeMappingIfAbsent ConcurrentHashMap.computeIfAbsent stores nothing when the mapping function returns null, so every concurrent caller re-ran the builder for a class with no conversion mapping - the common case for an ordinary action, and the exact thundering herd this method exists to prevent. Negative results now store a sentinel in the same map, so the builder runs once per class either way. getMapping and containsNoMapping translate the sentinel, preserving their existing contracts. * WW-5539 docs: sync plan with negative-cache sentinel fix * WW-5539 Pin down addNoMapping's override semantics Storing the no-mapping sentinel deliberately replaces any mapping cached for the class, matching the pre-7.3.0 effective behaviour where such a class was short-circuited before its cached mapping was ever read. putIfAbsent would instead serve a stale mapping after a failed build. Also asserts the sentinel translation in getMapping directly, and stops the interface javadoc promising a specific empty-map instance that implementations are not required to return. * WW-5539 Document that addNoMapping may replace a cached mapping The behaviour was documented only on the Struts implementation, but addNoMapping stays a non-deprecated SPI primitive that third parties both call and implement, so the contract belongs on the interface. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5539 Remove coarse locks from XWorkConverter getConverter() synchronized on the Class object being converted, which is a globally visible monitor any other library may contend on, and which serialised every conversion for a given action class including cache hits. It now delegates to TypeConverterHolder#computeMappingIfAbsent. registerConverter and registerConverterNotFound drop their synchronized modifier; they are single delegations to a concurrent map, and the lock never covered the readers in lookup() in any case. buildConverterMapping no longer stores its result - storage is owned by computeMappingIfAbsent. * WW-5539 Remove global lock from DefaultActionValidatorManager getValidators() was synchronized on the singleton manager, so every validated request in the application serialised on it - and the lock covered the per-request Validator construction loop, which operates on per-request objects and never needed mutual exclusion. Both caches become ConcurrentHashMap and cached config lists are wrapped unmodifiable, since several threads now iterate them concurrently. * WW-5539 Make the validator concurrency test race a cold cache The test computed its expected count with a getValidators call before starting the threads, which warmed the cache and left all 16 workers on the fast path - never exercising first-build contention, the race the test is named for. Also drops an unused import and awaits executor termination. * WW-5539 Address final review findings Restores the protected unknownMappings field verbatim as a deprecated, unused vestige: retyping it changed the field descriptor, so a subclass compiled against 7.2.0 would have hit NoSuchFieldError on upgrade without recompiling. Real storage moves to a private concurrent set. Also stops conditionalReload running for negative-cached classes, which had been costing a failed classloader resource scan per property per request in devMode, and restores the unknown-mapping clearing that the null-converter guard was skipping. * WW-5539 Fix concurrency regressions from coarse-lock removal Four correctness fixes surfaced in PR review of the concurrent-collections refactor: - StrutsTypeConverterHolder.addDefaultMapping: restore put-before-remove ordering. The inverted order let a concurrent XWorkConverter.lookup observe (unknown=false, default=false), sending it into lookupSuper() and letting it overwrite the more specific converter being registered. - StrutsTypeConverterHolder.computeMappingIfAbsent: stop building inside a ConcurrentHashMap bin lock. The builder reaches ObjectFactory.buildConverter, which can autowire arbitrary user TypeConverters; running that under a CHM bin lock risked a recursive-update exception or self-deadlock. Callers now only get the guarantee that they converge on the same cached instance, not that the builder runs exactly once - documented on the interface and reflected in the concurrency tests. - DefaultValidatorFactory.validators: switch to ConcurrentHashMap now that DefaultActionValidatorManager.getValidators is no longer synchronized, so runtime registerValidator() calls no longer race unsynchronized reads of a plain HashMap. - XWorkConverter.conditionalReload: route empty devMode rebuilds through addNoMapping instead of addMapping, so an empty reload result is stored as the NO_MAPPING sentinel rather than a plain empty map that would silently disable further reloads for the class. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5539 Add tests closing coverage gaps from coarse-lock removal SonarCloud's quality gate failed at 59.8% coverage on new code (need >=80%). Adds tests for the specific lines JaCoCo identified as uncovered, without touching production code: - TypeConverterHolder.computeMappingIfAbsent's default method body (the SPI compatibility fallback for third-party holders that predate 7.3.0 and don't override it) - new TypeConverterHolderTest against a minimal non-overriding implementation. - StrutsTypeConverterHolder.getMapping/containsNoMapping's remaining non-sentinel branch. - XWorkConverter.conditionalReload's reloadingConfigs==true path (both the addMapping and addNoMapping outcomes), buildConverterMappingUnchecked's checked-to-IllegalStateException wrapping, and getConverter's catch(Throwable) negative-caching. - DefaultActionValidatorManager's else-if(reloadingConfigs) cache rebuild, loadFile's checkFile&&fileNeedsReloading re-parse, and buildValidatorConfigs' already-checked short-circuit. TypeConverterHolder.java and StrutsTypeConverterHolder.java are now at 0 missed lines/branches. XWorkConverter.java and DefaultActionValidatorManager.java have all requested target lines covered; remaining misses are pre-existing, unrelated gaps left alone per scope. Full core suite: 3026 tests (3015 + 11 new), 0 failures/errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5539 Fix SonarCloud deprecation and test-hygiene issues - Add since/forRemoval attributes to the 7 @Deprecated elements on TypeConverterHolder.getMapping/addMapping/containsNoMapping and StrutsTypeConverterHolder's overrides plus the unknownMappings field (java:S6355). - Add the missing @deprecated Javadoc tag to the three StrutsTypeConverterHolder overrides, pointing at computeMappingIfAbsent as the replacement (java:S1123). - Remove the unused throws Exception from testGetConverterBuildsMappingExactlyOncePerClass (java:S1130). - Document why StubFileManager.setReloadingConfigs/monitorFile are intentionally empty no-ops (java:S1186). - Rename a local variable that shadowed the converter field in testConditionalReloadRebuildsEmptyMappingAndStoresItViaAddNoMapping (java:S1117). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5539 Suppress removal warnings for the deprecated holder primitives javac treats [removal] as a category separate from [deprecation], so marking the three primitives forRemoval left four warnings behind: the deliberate addMapping call in conditionalReload, and the three overrides that must exist for as long as the interface declares them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5539 Address Copilot review comments Fixes getMapping's @return (it returns a Map, not a TypeConverter) and drops the "atomically" wording from its @deprecated tag, which no longer matches computeMappingIfAbsent's contract now that the builder may run more than once under concurrent first access. Syncs the design and plan docs with the shipped approach: the unknownMappings field is kept for binary compatibility rather than retyped, and the override uses get/build/putIfAbsent rather than computeIfAbsent. * WW-5539 docs: correct the classloader out-of-scope note The conversion caches are container-scoped singletons with no external references, so their Class keys do not independently pin the webapp classloader - that is governed by whatever retains the container (WW-5537). Reframed as optional defense-in-depth cache clearing, folded into WW-5537 Task 5b, rather than a standalone leak fix. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
dependabot[bot]
|
73f17c1be7 |
build(deps-dev): bump org.webjars:jquery from 3.7.1 to 4.0.0 (#1802)
Bumps [org.webjars:jquery](https://github.com/jquery/jquery) from 3.7.1 to 4.0.0. - [Release notes](https://github.com/jquery/jquery/releases) - [Changelog](https://github.com/jquery/jquery/blob/main/changelog.md) - [Commits](https://github.com/jquery/jquery/compare/3.7.1...4.0.0) --- updated-dependencies: - dependency-name: org.webjars:jquery dependency-version: 4.0.0 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
eb69ad6f3a |
build(deps): bump org.webjars:bootstrap from 5.3.7 to 5.3.8 (#1803)
Bumps [org.webjars:bootstrap](https://github.com/webjars/bootstrap) from 5.3.7 to 5.3.8. - [Commits](https://github.com/webjars/bootstrap/compare/bootstrap-5.3.7...bootstrap-5.3.8) --- updated-dependencies: - dependency-name: org.webjars:bootstrap dependency-version: 5.3.8 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
88d95b2a5f |
build(deps): bump org.webjars:webjars-locator-lite from 1.1.3 to 1.1.4 (#1801)
Bumps [org.webjars:webjars-locator-lite](https://github.com/webjars/webjars-locator-lite) from 1.1.3 to 1.1.4. - [Release notes](https://github.com/webjars/webjars-locator-lite/releases) - [Commits](https://github.com/webjars/webjars-locator-lite/compare/webjars-locator-lite-1.1.3...webjars-locator-lite-1.1.4) --- updated-dependencies: - dependency-name: org.webjars:webjars-locator-lite dependency-version: 1.1.4 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
ef8361caa7 |
build(deps): bump github/codeql-action from 4.37.0 to 4.37.2 (#1800)
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.37.0 to 4.37.2. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/v4.37.0...v4.37.2) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.37.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
a324538650 |
build(deps): bump org.htmlunit:htmlunit from 5.2.0 to 5.3.0 (#1804)
Bumps [org.htmlunit:htmlunit](https://github.com/HtmlUnit/htmlunit) from 5.2.0 to 5.3.0. - [Release notes](https://github.com/HtmlUnit/htmlunit/releases) - [Commits](https://github.com/HtmlUnit/htmlunit/compare/5.2.0...5.3.0) --- updated-dependencies: - dependency-name: org.htmlunit:htmlunit dependency-version: 5.3.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Lukasz Lenart
|
b154b7ca43 |
ci(owasp): cap job timeout and disable NVD auto-update in check step (#1807)
* ci(owasp): cap job timeout and disable NVD auto-update in check step The OWASP job intermittently failed with no reason other than timeouts. Root cause is the unreliable NIST NVD feed (see dependency-check#8633): keyless NVD downloads are heavily rate-limited and stall. Two fixes: - Add timeout-minutes: 30 so a hung NVD download fails fast instead of dragging to the 6h GitHub Actions default. - Add -DautoUpdate=false to the check step so it reads only the cache populated by the preceding update-only step. Previously the check step carried neither the mirror datafeed URL nor the API key, so on any cache staleness/miss it synced directly against NIST - the unreliable path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci(owasp): fall back to NVD mirror when the API update fails The NIST NVD API is unreliable even with an API key (retries exhausted, see dependency-check#8633). Previously the mirror datafeed was used only when no API key was present, so apache/struts always took the flaky API path and never the mirror. Make the API update step continue-on-error and run the mirror update as a fallback when the API step fails (or when no API key is configured). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
William Dutton
|
9f030f62be |
Library updates for cve's, suppression cleanup and not fail github action job SonarCloud if SONARCLOUD_TOKEN not found (summary report instead). (#1667)
* OWASP + Github workflow updates * Library updates * Dependancy suppression cleanup * #1667 PR Review updates, use NIST_NVD_API_KEY when available else use mirror for forks not configured, remove workflow_call for now since we don't on call |
||
|
Lukasz Lenart
|
f32270cdb0 |
WW-5580 chore(core): use Configuration.VERSION_2_3_34 for FreeMarker config (#1798)
Aligns the FreeMarker incompatible_improvements setting with the FreeMarker 2.3.34 dependency already declared in the build. FreeMarker 2.3.34 declares VERSION_2_3_34 as an incompatible improvements break-point but does not gate any behaviour on it, so this is a no-op at runtime and purely keeps the setting in sync with the dependency. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
f5880be2ef |
WW-5591 chore(core): mark XWorkObjectPropertyAccessor as deprecated (#1797)
The class is no longer used by the framework and can be removed in a future version. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
6d27132926 |
WW-4858 Evaluate JSON name allowlist at leaf keys only (#1784)
* WW-4858 fix(json): evaluate name allowlist at leaf keys only
The JSON population filter walked the object tree and applied every name
check at every node before recursing. Accepted name patterns and the
ParameterNameAware callback target the full dotted binding path, so gating
an intermediate node (e.g. "bean") against a leaf-specific rule dropped the
entire subtree before the leaf ("bean.stringField") was ever evaluated —
diverging from ParametersInterceptor, which only evaluates complete leaf
names. For arrays it also meant the accepted allowlist judged the container
name instead of the element path.
Split the per-key gate: length, excluded patterns, @StrutsParameter
authorization and property filters stay per-node (exclusion is prefix-safe
and authorization is intentionally hierarchical); accepted patterns and
ParameterNameAware move to leaf keys only, including scalar array elements
at their indexed path ("items[0]"). This reproduces the flat-path semantics
exactly. Excluded/include-property behavior is unchanged.
Tests: nested-object leaf populates under a leaf-targeting accepted pattern
and a ParameterNameAware action that rejects the intermediate node; accepted
patterns now apply to the array element path; nested include-property
filtering still works.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-4858 fix(json): apply per-node checks to scalar array elements
Scalar list elements were gated only by the leaf name-allowlist and value
checks, skipping the per-node checks (length, excluded patterns,
@StrutsParameter authorization, property filters). That left the JSON path
more permissive than ParametersInterceptor, which evaluates all of these
against the full indexed name "items[0]".
Apply isAcceptableNode(elementPrefix, ...) to scalar list elements so an
element is gated exactly as the flat path gates "items[0]". Note this makes
scalar-list @StrutsParameter authorization use the element path (depth 1,
read method) rather than only the container (depth 0), matching the flat
path.
Tests: excluded name pattern and @StrutsParameter authorization now apply at
the list element path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-4858 refactor(json): extract keyTypeName helper to lower cognitive complexity
Move the non-String-key logging ternary out of filterUnacceptableKeysRecursive
into a keyTypeName helper. Pure extraction, no behavior change; drops the
method's cognitive complexity from 17 to 14, under Sonar's S3776 threshold.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
Lukasz Lenart
|
f2c1f50da2 |
WW-5604 Recognize CDI/Weld client proxies in SecurityMemberAccess (#1796)
* WW-5604 Add CdiProxyService to detect Weld client proxies Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5604 Register CdiProxyService as the active ProxyService Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5604 Address review: positive allowlist test, guard Weld member check, fix javadoc Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5604 Add WELD_AVAILABLE guard and weld-api version property Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5604 Cover null, non-proxy, non-method and Weld-absent paths Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5604 Remove unreachable guard and cover unwrap fallbacks Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
464817e0b0 |
WW-5653 Upgrade Bootstrap to 5.3.x in sample apps (#1793)
* WW-5653 docs: add Bootstrap 5.3.x sample-app migration design Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 docs: add Bootstrap 5 migration implementation plan Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 build: add Bootstrap 5, Bootstrap Icons, showcase jQuery webjars Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 feat(showcase): serve Bootstrap 5 and jQuery via webjars Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(showcase): serve html5 demo Bootstrap CSS via webjar Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 feat(showcase): migrate navbar and top-level pages to Bootstrap 5 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(showcase): migrate leftover Bootstrap 2 icon and fixed-navbar classes Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 feat(showcase): migrate tag-demo pages to Bootstrap 5 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 feat(showcase): migrate validation-demo pages to Bootstrap 5 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 feat(showcase): migrate fileupload and conversion pages to Bootstrap 5 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 feat(showcase): migrate wait, token and empmanager pages to Bootstrap 5 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(showcase): remove BS3 carets and well class Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(showcase): use ms-auto for right nav and add nav-link to Home Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 feat(rest-showcase): serve Bootstrap 5 CSS via webjars Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 feat(rest-showcase): migrate JSP markup to Bootstrap 5 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(rest-showcase): migrate legacy BS2/BS3 grid classes to Bootstrap 5 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(showcase): drop obsolete css/js excludePattern so webjar JS is served The old struts.action.excludePattern whitelisted the vendored /styles/*.css and /js/*.js layout for container serving. After moving Bootstrap/jQuery to webjars under /static/webjars/**, the '.*/js/.*\.js' entry matched the webjar JS path (e.g. bootstrap.bundle.min.js) and excluded it from Struts' static handler, so it fell through to the container and 404'd. Remaining webapp assets (prettify.js, main.css) are served via default-servlet fall-through. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(samples): migrate remaining BS2/BS3 classes (tables, buttons, progress, forms, navbar) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 chore(showcase): comment out verbose debug loggers Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(showcase): migrate Bootstrap 5 markup in ftl/vm/html templates Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 test(showcase): disable JS in FreeMarkerManagerTest for Bootstrap 5 HtmlUnit's JS engine cannot parse Bootstrap 5's ES6 (bootstrap.bundle.min.js uses 'class'), and the decorator now serves it, so the default WebClient threw on script error. The test only asserts server-rendered FreeMarker output, so JavaScript is disabled (matching Html5TagExampleTest). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(showcase): replace dead Bootstrap 2 form and alert classes Migrate new-person.ftl form off BS2 control-group/controls/form-actions to Bootstrap 5 (mb-3, form-label, form-control), and replace the dead alert-error class with alert-danger across the showcase pages. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 test(showcase): make integration tests tolerate Bootstrap 5 under HtmlUnit HtmlUnit 5.2.0 cannot parse Bootstrap 5's minified ES6 (bootstrap.bundle.min.js uses 'class'), which broke the showcase HtmlUnit integration tests once the decorator started serving the bundle. - Add ParameterUtils.createWebClient() which disables throwExceptionOnScriptError, and route all integration tests through it (they assert server-rendered output, not Bootstrap's client-side behaviour). - Load bootstrap.bundle.min.js with 'defer' so a page's own inline scripts (e.g. the async chat demo) still execute before HtmlUnit hits the bundle's parse error; defer is also the recommended real-browser loading strategy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 test(showcase): poll for async chat result instead of fixed sleep AsyncTest relied on a fixed Thread.sleep(4000) for the server-push chat round-trip, which is not enough on slower/newer JVMs (reproduced failing on JDK 25). Poll the result element for up to ~30s via waitForBackgroundJavaScript instead, making the test robust across JVMs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 test(showcase): drive AsyncTest via HTTP instead of HtmlUnit The browser-driven chat test was flaky on JDK 25 in CI: HtmlUnit's handling of the async server-push long-poll timed out (message never rendered), even with polling. The test's purpose is to validate the Servlet 3 async endpoints, which needs no browser or JavaScript. Rewrite it to POST /async/sendMessage and read /async/receiveNewMessages directly over HTTP and assert the JSON, making it deterministic and independent of Bootstrap/HtmlUnit JS parsing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(showcase): drop progressbar role from wait progress bar Resolves a SonarCloud accessibility finding (S6819) introduced by the Bootstrap 5 migration. The BS5 progress component is styled on .progress/.progress-bar divs; the role/aria attributes were newly added (the BS3 original had none), so removing them clears the finding while keeping the Bootstrap 5 styling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(showcase): correct BS5 alert wrapper and drop stray table tag Address Copilot review on PR #1793: - decorators/main.jsp: the dismissible wrapper was a second .alert with no variant while the variant class (alert-danger, etc.) stayed on the inner <ul>, producing a nested, uncoloured alert box. Move the alert* classes onto the wrapper and strip them from the <ul> so the wrapper is the single alert container. - orders-edit.jsp: remove the stray, unclosed <table> start tag before </s:form> (pre-existing invalid markup carried over during the migration). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(showcase): remove BS3 navbar-header wrapper and orphan dropdown-submenu Clean up two leftover Bootstrap 3 artifacts in the showcase decorator navbar: - Drop the `navbar-header` wrapper (no BS5 CSS behind it) and make the brand and toggler direct children of the `.container-fluid` flex container (justify-content: space-between), with the brand first per BS5 convention. - Remove the empty, unclosed `<li class="dropdown-submenu">` orphan before the first item in the Examples menu; BS5 has no dropdown-submenu feature. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(showcase): replace soft-deprecated navbar-light with data-bs-theme navbar-light is soft-deprecated in Bootstrap 5.3. Switch the showcase navbar to the current data-bs-theme="light" idiom; bg-light is retained for the background. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(showcase): normalize page-header replacement to border-bottom utilities A handful of showcase pages replaced the BS3 page-header with a bare <div> while the rest used <div class="border-bottom pb-2 mb-3">. Normalize those 35 header wrappers to the same border-bottom pb-2 mb-3 utilities so all showcase page headers render consistently. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5653 fix(rest-showcase): add mb-3 to page-header replacement Align rest-showcase order page headers with the showcase standard by using border-bottom pb-2 mb-3 (was border-bottom pb-2), so header spacing is consistent across both sample apps. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
b2548542ec |
WW-5640 refactor(webjars): rename DefaultWebJarUrlProvider to StrutsWebJarUrlProvider (#1795)
Use the Struts* prefix convention for the framework's default WebJarUrlProvider implementation instead of Default*. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Lukasz Lenart
|
11c10ee8f9 |
WW-5620 Standardize logging on Log4j2 (#1794)
* WW-5620 docs: add Log4j2 logging standardization design spec Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5620 docs: add Log4j2 logging standardization implementation plan Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5620 Migrate FinalizableReferenceQueue to Log4j2 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5620 Migrate AbstractDefaultToStringRenderable to Log4j2 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5620 Remove unused injectable j.u.l.Logger DI factory from ContainerBuilder Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5620 Remove dead first-party SLF4J dependency declarations Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Sri lakshmi kanthan P
|
60bc7b8d7a |
WW-5652 Add support for Java records and Optional serialization in the JSON plugin. (#1792)
* feat(json): support serialization of Java records in JSON processing * feat(json): add support for serializing Optional values in JSON processing |
||
|
Arun
|
963a81f43d |
WW-5647 Use ConcurrentHashMap for XSLT template cache (#1781)
* Use ConcurrentHashMap for XSLT template cache and add double-check locking * Prevent noCache from polluting shared template cache; add dedup and noCache regression tests |
||
|
Lukasz Lenart
|
cc00343f1b |
WW-5650 Obtain a fresh JSON reader/writer per request in JSONInterceptor (#1782)
* WW-5650 revert StrutsJSONReader to plain single-use instance fields * WW-5650 revert StrutsJSONWriter to plain single-use instance fields * WW-5650 obtain a fresh JSONUtil per request in JSONInterceptor * WW-5650 resolve JSONUtil lazily only on JSON request paths Move getJSONUtil() into the JSON and JSON-RPC branches of intercept() so requests with a non-JSON content type no longer construct and discard an unused JSONUtil/reader/writer graph. Also trim a stray trailing blank line in StrutsJSONWriter. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5650 test(json): assert JSONWriter bean stays prototype-scoped Guards the response-side invariant from WW-5644: StrutsJSONWriter now uses plain instance fields and is not thread-safe, so cross-request safety relies solely on the writer bean being prototype-scoped. Assert distinct instances per container lookup so a future switch to singleton scope fails the build. Addresses review feedback on #1782 without adding a getWriter() accessor purely for tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
dependabot[bot]
|
db64c103d8 |
build(deps-dev): bump org.webjars:jquery from 3.7.1 to 4.0.0 (#1789)
Bumps [org.webjars:jquery](https://github.com/jquery/jquery) from 3.7.1 to 4.0.0. - [Release notes](https://github.com/jquery/jquery/releases) - [Changelog](https://github.com/jquery/jquery/blob/main/changelog.md) - [Commits](https://github.com/jquery/jquery/compare/3.7.1...4.0.0) --- updated-dependencies: - dependency-name: org.webjars:jquery dependency-version: 4.0.0 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
31036a8a49 |
build(deps): bump github/codeql-action from 4.36.3 to 4.37.0 (#1786)
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.36.3 to 4.37.0. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/v4.36.3...v4.37.0) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.37.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
6cb6abf4ba |
build(deps): bump com.fasterxml.jackson:jackson-bom (#1787)
Bumps [com.fasterxml.jackson:jackson-bom](https://github.com/FasterXML/jackson-bom) from 2.22.0 to 2.22.1. - [Commits](https://github.com/FasterXML/jackson-bom/compare/jackson-bom-2.22.0...jackson-bom-2.22.1) --- updated-dependencies: - dependency-name: com.fasterxml.jackson:jackson-bom dependency-version: 2.22.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Arun
|
de599c91d3 |
WW-5646 Modernize path normalization in Include component (#1780)
* Modernize path normalization in Include component and handle edge cases * Add tests for edge case dot-dot handling in Include path normalization * Fix reversed path segment order in getContextRelativePath() The for-each loop iterated the ArrayDeque head-to-tail (most recently pushed first), which is the reverse of the old Stack's insertion-order iteration. This caused rebuilt paths like "car/view.jsp" to come out as "view.jsp/car". Use descendingIterator() to restore the original oldest-first ordering when rebuilding the flat path string. |
||
|
ⳕⲛτⲉⲅⲥⲉⳏτⲟⲅ 🕵🏻
|
e5eb01abda |
WW-5642 fix(rest): authorize @StrutsParameter on record/creator-bound REST body properties (#1774)
* fix(rest): authorize @StrutsParameter on record/creator-bound REST body properties ParameterAuthorizingModule enforces @StrutsParameter on REST/JSON body deserialization by wrapping each property's deserializeAndSet/ deserializeSetAndReturn. Jackson never calls either method for creator-bound properties (Java records, @JsonCreator constructors, @ConstructorProperties) — it calls SettableBeanProperty#deserialize directly, which is declared final and bypasses the wrapper entirely. With struts.parameters.requireAnnotations enabled, any record-typed field anywhere in a REST action's request body was populated with no authorization check at all. Add AuthorizingValueDeserializer, which wraps the property's value deserializer instead of the property itself, and install it from AuthorizingSettableBeanProperty#withValueDeserializer — scoped to CreatorProperty so ordinary setter/field/builder properties, already authorized via the existing wrapper, aren't checked twice. * fix(rest): treat redaction-induced construction failures as unauthorized, not fatal AuthorizingValueDeserializer substitutes null for a rejected creator-bound property (record component, @JsonCreator/@ConstructorProperties param). For reference-typed, unvalidated components this is a harmless stand-in for "not set" -- but two cases turn that substitution into an unhandled exception that crashes deserialization of the entire request body instead of just dropping the unauthorized subtree: - A record/constructor with its own non-null validation (e.g. a compact constructor doing Objects.requireNonNull) throws ValueInstantiationException when the redacted component reaches it. - With DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES enabled, Jackson itself throws MismatchedInputException when a primitive-typed creator component is redacted to null. Add RedactionAwareDeserializer, wrapping every bean-type deserializer via a new BeanDeserializerModifier#modifyDeserializer hook. It tracks (via a new redaction-scope stack in ParameterAuthorizationContext) whether the object currently under construction had a property redacted by authorization; if construction then throws, the object is treated as unauthorized (returns null) instead of propagating the raw exception -- matching the same fail-closed outcome already used when a non-creator nested property is rejected outright. A guard test confirms genuine, unrelated validation failures (nothing redacted) still propagate normally, so real client errors aren't masked. Also verified (and added regression coverage for) the other gaps raised in review: static factory-method @JsonCreator, @ConstructorProperties, top-level records, 3-level nested creator chains, and List/Map creator params whose elements are further creator-bound or plain-POJO types -- all of these were already handled correctly by the existing withValueDeserializer interception. * test(rest): cover array creator param; document redaction edge cases Addresses the three non-blocking review notes on WW-5642: - Add testArrayOfRecordsAsCreatorParam_elementsAuthorizedByIndexedPath and a WithArray fixture, exercising the type.isArray() branch of AuthorizingValueDeserializer#prefixForNested so the collection matrix (List/Map/array) is fully covered. - Document in AuthorizingValueDeserializer that redacting a primitive creator component becomes the type default (0/false) when FAIL_ON_NULL_FOR_PRIMITIVES is off -- a deliberate choice, the client value never lands either way. - Document in RedactionAwareDeserializer that a redaction co-located with an unrelated mapping error is folded into "object dropped" -- a deliberate fail-closed trade-off, never exposing a partial object. --------- Co-authored-by: g0w6y <g0w6y@users.noreply.github.com> |
||
|
Lukasz Lenart
|
cf22320e33 |
docs(security): treat a fix or PR as disclosure, require private report first (#1785)
The disclosure rules only forbade publishing exploit/PoC code, so a contributor who opens a public PR that fixes or hints at a suspected vulnerability reads them as satisfied — the fix itself telegraphs the weakness before a fixed release exists. Add a dedicated "Do not disclose through a pull request, commit, or issue" section directing reporters to email security@struts.apache.org first, and extend the PoC rule in Report Quality Rules to state that a fix, patch, or hardening change is a public disclosure in the same way a PoC is. Aligns SECURITY.md with the rule already stated in CLAUDE.md/AGENTS.md. 🤖 Generated by AI Assistant |
||
|
Lukasz Lenart
|
40fcae3101 |
WW-4858 test(json): cover nested-leaf accepted-name and include patterns (#1783)
Add two tests to JSONInterceptorTest exercising the nested-object path for
the name/value filtering added in WW-4858:
- testAcceptedNamePatternRejectsNestedKey: accepted name patterns are raw
full-match regexes with no hierarchy expansion, so the intermediate node
("bean") must itself match an accepted pattern or the whole subtree is
dropped before the leaf is visited.
- testIncludePropertiesAppliedToNestedInputWhenEnabled: include patterns do
expand across the hierarchy, so "bean.stringField" also matches the
intermediate "bean" and the nested leaf populates while the excluded
sibling "bean.intField" is dropped.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
Arun
|
b70ecc8e15 |
WW-5645 Canonicalise static content paths and remove redundant URL decode (#1777)
* Add path segment validation utility to StaticContentLoader * Remove redundant URL decode in buildPath and reject malformed path segments early * Use shared path segment validation in WebJar URL provider * Document encoding contract on RequestUtils.getServletPath * Add tests for path segment validation in static content loader * Add encoded traversal test for WebJar static content serving * Add encoded traversal tests for WebJar URL provider * Fix missing closing brace in StaticContentLoader causing compile failure * Use per-segment matching in containsMalformedPathSegment to avoid false positives * Remove redundant dot-segment check now handled by containsMalformedPathSegment * Fix indentation on validateStaticContentPath closing brace * Remove unused encoding field and setter from DefaultStaticContentLoader * Replace denylist with path canonicalisation in Validator * Wire canonicalisePath into static content serving * Wire canonicalisePath into WebJar URL provider * Update tests for canonicalise approach and remove unused setEncoding call * Remove setEncoding calls from tests to match updated DefaultStaticContentLoader * Remove setEncoding calls from tests to match updated DefaultStaticContentLoader * Remove redundant encoded-traversal tests per maintainer review — end-to-end 404 already covered |
||
|
ⳕⲛτⲉⲅⲥⲉⳏτⲟⲅ 🕵🏻
|
18955b98a4 |
WW-5644 fix(json): confine StrutsJSONWriter write state to the writing thread (#1776)
* fix(json): confine StrutsJSONWriter write state to the writing thread JSONUtil obtains its JSONWriter once via @Inject and reuses that same instance across every concurrent response handled by that JSONResult/ JSONInterceptor configuration. StrutsJSONWriter kept its output buffer, cyclic-reference stack, root object, and expression-path state (buf/stack/root/buildExpr/exprStack/excludeProperties/ includeProperties/excludeNullProperties) as plain instance fields, all reset in place at the start of write(). Two concurrent write() calls on the same instance therefore race on that reset: one call's in-progress buffer can be wiped and overwritten by a second, unrelated concurrent call before the first call reads it back via buf.toString(), so one request's serialized JSON can be returned as a completely different, concurrently-served request's response body. Move buf/stack/root/buildExpr/exprStack/excludeProperties/ includeProperties/excludeNullProperties into a WriteState confined to a ThreadLocal, scoped to a single write() call. Method signatures and behavior are otherwise unchanged so existing StrutsJSONWriter subclasses keep working; ignoreHierarchy/dateFormat/enumAsBean/ excludeProxyProperties stay as plain instance fields since they are set to the same value on every call for a given writer configuration and are safe to share. * test(json): raise writer concurrency test to 16 threads for reliable repro Verified independently that the 2-thread version can miss the race on machines with more cores than contending threads (with no CPU contention, the OS scheduler has no need to preempt either thread mid-call, so the corruption window is rarely hit): 0 reproductions in 8 reruns against unpatched code on a 10-core machine. Sixteen threads reproduced it reliably (44,646/320,000 corrupted responses against unpatched StrutsJSONWriter), and confirmed zero corruption against the fix under the same load. --------- Co-authored-by: g0w6y <g0w6y@users.noreply.github.com> |
||
|
ⳕⲛτⲉⲅⲥⲉⳏτⲟⲅ 🕵🏻
|
75a285a106 |
WW-5643 fix(json): confine StrutsJSONReader parse state to the parsing thread (#1775)
* fix(json): confine StrutsJSONReader parse state to the parsing thread JSONInterceptor obtains its JSONReader once via @Inject and reuses that same instance across every concurrent request handled by that interceptor. StrutsJSONReader kept its parse cursor, token buffer and nesting-depth counter (used to enforce maxDepth/maxElements/ maxStringLength/maxKeyLength) as plain instance fields, so two concurrent read() calls on the same instance tore each other's state: one request's depth counter could be decremented by an unrelated concurrent request finishing its own parse, letting payloads deeper than the configured maxDepth through, and the shared character cursor and string/number buffer let fragments of one request's JSON body leak into a different, concurrently-parsed request's result. Move the cursor, current character, token, buffer and depth into a ParseState confined to a ThreadLocal, scoped to a single read() call. Method signatures and behavior are otherwise unchanged so existing StrutsJSONReader subclasses keep working; the limit fields (maxElements/maxDepth/maxStringLength/maxKeyLength) stay as plain instance fields since they are set to the same value on every call for a given interceptor configuration and are safe to share. * test(json): raise reader concurrency test to 16 threads for reliable repro Verified independently that the 2-thread version can miss the race on machines with more cores than contending threads (with no CPU contention, the OS scheduler has no need to preempt either thread mid-call, so the corruption window is rarely hit): 0 reproductions in 8 reruns against unpatched code on a 10-core machine. Sixteen threads reproduced both symptoms reliably against unpatched StrutsJSONReader (81 cross-thread data leaks and 79 maxDepth bypasses out of 160,000 attempts), and confirmed zero of either against the fix under the same load. Combined the two prior tests into one, since both symptoms come from the same shared parse state and are naturally checked together per thread. --------- Co-authored-by: g0w6y <g0w6y@users.noreply.github.com> |
||
|
dependabot[bot]
|
8fb8bcbcf0 |
build(deps): bump net.sf.jasperreports:jasperreports (#1779)
Bumps [net.sf.jasperreports:jasperreports](https://github.com/Jaspersoft/jasperreports) from 7.0.4 to 7.0.7. - [Release notes](https://github.com/Jaspersoft/jasperreports/releases) - [Changelog](https://github.com/Jaspersoft/jasperreports/blob/master/changes.txt) - [Commits](https://github.com/Jaspersoft/jasperreports/compare/7.0.4...7.0.7) --- updated-dependencies: - dependency-name: net.sf.jasperreports:jasperreports dependency-version: 7.0.7 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
dependabot[bot]
|
f03198c6f8 |
build(deps-dev): bump commons-logging:commons-logging (#1760)
Bumps [commons-logging:commons-logging](https://github.com/apache/commons-logging) from 1.3.6 to 1.4.0. - [Changelog](https://github.com/apache/commons-logging/blob/master/RELEASE-NOTES.txt) - [Commits](https://github.com/apache/commons-logging/compare/rel/commons-logging-1.3.6...rel/commons-logging-1.4.0) --- updated-dependencies: - dependency-name: commons-logging:commons-logging dependency-version: 1.4.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Lukasz Lenart
|
525c7dae37 |
WW-4858 Honor parameter filtering during JSON population (#1773)
* WW-4858 docs(json): design for honoring parameter filtering during JSON population Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-4858 docs(json): implementation plan for JSON parameter filtering Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-4858 feat(json): enforce excluded/accepted name patterns on JSON population Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-4858 feat(json): enforce param-name max length on JSON population Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-4858 feat(json): honor ParameterNameAware and ParameterValueAware on JSON population Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-4858 feat(json): add opt-in excluded/accepted value patterns on JSON population Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-4858 feat(json): opt-in applying excludeProperties/includeProperties to JSON input Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-4858 test(json): cover nested and list-element paths; clarify filter comments Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |