[
https://issues.apache.org/jira/browse/WW-5726?focusedWorklogId=1041473&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1041473
]
ASF GitHub Bot logged work on WW-5726:
--------------------------------------
Author: ASF GitHub Bot
Created on: 14/Sep/26 18:04
Start Date: 14/Sep/26 18:04
Worklog Time Spent: 10m
Work Description: lukaszlenart opened a new pull request, #1941:
URL: https://github.com/apache/struts/pull/1941
Fixes [WW-5726](https://issues.apache.org/jira/browse/WW-5726)
## Problem
A property that is mergeable (`@JsonMerge`, or merge enabled by
configuration), has a value type deserializer (`@JsonTypeInfo` on the property
or its declared type) and a non-null initial value is deserialized by Jackson
past both authorizing wrappers. `BeanDeserializerBase.resolve()` builds a
`MergingSettableBeanProperty` around the already-wrapped property; with a
non-null current value it calls the `final`
`SettableBeanProperty#deserializeWith`, which resolves a fresh deserializer for
the existing value's class and deserializes into it in place. Neither
`AuthorizingSettableBeanProperty.deserializeAndSet` nor
`AuthorizingValueDeserializer` runs and `set()` is never called: the property
binds unchecked and its members are authorized as the enclosing bean's own
members, so a grant on a sibling of the same name authorizes the nested write.
No type id is needed in the body — the subtype comes from the existing value.
Sibling of WW-5725, which covered the other `set()` and value-deserializer
entry points.
## Change
`AuthorizingValueDeserializer` now carries the property's value
`TypeDeserializer` and answers `supportsUpdate()` with `FALSE` when one is
present. `resolve()` asks the value deserializer before it builds the merging
wrapper — through whichever property wrapper it has put around the authorizing
one by then (`ManagedReferenceProperty`, `ObjectIdReferenceProperty` included,
since they carry the value deserializer through) — so the property stays on the
ordinary authorized path: it is checked itself, its members under its own
prefix, and the value is **replaced rather than merged**.
- Under Jackson's default `MapperFeature.IGNORE_MERGE_FOR_UNMERGEABLE` the
declined merge is silently ignored; an application that disabled that feature
gets Jackson's own `InvalidDefinitionException` ("values of type X cannot be
merged").
- A non-polymorphic merge keeps merging (WW-5725 path,
`testMergeIntoExistingValueIsAuthorized` unchanged).
- A WARN at resolve time (once per mapper and bean type) makes the disabled
merge visible.
Hiding the merge info from the wrapper's `getMetadata()` was tried first and
rejected in review: `ManagedReferenceProperty` and `ObjectIdReferenceProperty`
copy the metadata field before `resolve()` consults it, so
`@JsonManagedReference` or `@JsonIdentityInfo` on the polymorphic type kept the
bypass open. Both are now regression tests.
## Compatibility
This is decided when the deserializer is built, so it applies to every
`JacksonJsonHandler`/`JacksonXmlHandler` user, not only when
`struts.parameters.requireAnnotations=true`. An application that merges into a
polymorphic property with a non-null initial value through the REST plugin must
now send the type id in the body, and the existing value's state is replaced,
not merged. No other property kind is affected. Version Notes entry to follow.
## Tests
`ParameterAuthorizingModuleTest`: the ticket scenario (sibling grant no
longer reaches the merged subtype), the `@JsonIdentityInfo` and
`@JsonManagedReference` wrapper variants, the replace path through
`readerForUpdating`, and the strict-mode refusal with
`IGNORE_MERGE_FOR_UNMERGEABLE` disabled.
`mvn test -DskipAssembly -pl plugins/rest`: 184 tests, 0 failures.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Issue Time Tracking
-------------------
Worklog Id: (was: 1041473)
Remaining Estimate: 0h
Time Spent: 10m
> Merged polymorphic property with a non-null initial value is not authorized
> in the REST plugin
> ----------------------------------------------------------------------------------------------
>
> Key: WW-5726
> URL: https://issues.apache.org/jira/browse/WW-5726
> Project: Struts 2
> Issue Type: Bug
> Components: Plugin - REST
> Reporter: Lukasz Lenart
> Assignee: Lukasz Lenart
> Priority: Major
> Fix For: 7.4.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> h2. Summary
> A property that is mergeable ({{@JsonMerge}}, or merge enabled through
> configuration), carries a value type deserializer ({{@JsonTypeInfo}} on the
> property or its declared type), and has a non-null initial value is
> deserialized by Jackson through a path that reaches neither
> {{AuthorizingSettableBeanProperty}} nor {{AuthorizingValueDeserializer}}. The
> property is assigned without an authorization check, and its members are
> authorized against the enclosing prefix instead of their own.
> Sibling of WW-5725, which covers the other {{set()}} and value-deserializer
> entry points; this one cannot be closed from the property wrapper.
> h2. Current behaviour
> {{MergingSettableBeanProperty}} is constructed by
> {{BeanDeserializerBase.resolve()}} around the already-wrapped property, so
> {{BeanDeserializer}} calls its {{deserializeAndSet}}, which never reaches
> {{AuthorizingSettableBeanProperty.deserializeAndSet}}. With a non-null
> current value it calls the {{final}}
> {{SettableBeanProperty.deserializeWith(p, ctxt, oldValue)}}. When
> {{_valueTypeDeserializer}} is set, that method derives the subtype from
> {{oldValue.getClass()}}, resolves a fresh contextual deserializer for it, and
> deserializes in place into {{oldValue}} — bypassing the property's
> {{_valueDeserializer}} (where {{AuthorizingValueDeserializer}} sits). Because
> the merge is in place, {{newValue == oldValue}} and {{set()}} is not called
> either.
> Consequences: the property itself is never authorized, and no path is pushed,
> so the merged bean's members are authorized as {{member}} at the enclosing
> prefix rather than {{property.member}}. A grant on a sibling property of the
> enclosing bean with the same member name therefore authorizes the nested
> write. No type id is required in the body; the subtype comes from the
> existing value.
> The non-polymorphic merge (no type deserializer) goes through
> {{_valueDeserializer.deserialize(p, ctxt, oldValue)}} and is covered by
> WW-5725.
> Applies to JSON and XML bodies alike. 6.x is unaffected: the REST plugin
> there has no Jackson authorization module.
> h2. Proposed change
> Both facts are known when {{ParameterAuthorizingModule.updateBuilder}} runs:
> {{property.getMetadata().getMergeInfo() != null}} and
> {{property.hasValueTypeDeserializer()}}. Refuse the combination there, so the
> definition fails closed rather than binding unchecked — either by reporting a
> bad definition or by stripping the merge for that property so it takes the
> ordinary authorized path.
> The alternative of intercepting the in-place typed deserialize from
> {{RedactionAwareDeserializer.createContextual}} would keep the merge working,
> but it has to avoid a second path push on the normal path and is the more
> fragile of the two.
> Add a test in the same direction as the WW-5725 tests: a {{@JsonMerge}}
> {{@JsonTypeInfo}} property with an initialised field, an authorizer granting
> the enclosing bean's sibling member of the same name, asserting the nested
> member is not assigned.
> h2. Compatibility notes
> An application that relies on merging into a polymorphic property with a
> non-null initial value through the REST plugin will see that definition
> rejected (or merge disabled for it) until the property is given a null
> initial value or the type deserializer is dropped. No other property kind is
> affected.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
