HV-1916 Do not write container information to the shared root path node - #2083
HV-1916 Do not write container information to the shared root path node#2083Develop-KIM wants to merge 1 commit into
Conversation
|
Thanks for your pull request! This pull request appears to follow the contribution rules. › This message was automatically generated. |
|
Hey @Develop-KIM 👋🏻 thanks for the pull request, I think we might be trying to fix a non-existing issue here, please see the comment: https://hibernate.atlassian.net/browse/HV-1916?focusedCommentId=147800 |
`inIterable()`, `atIndex()`, `atKey()` and `inContainer()` write the container information onto the current leaf node of the violation path. For a violation built from a class-level constraint the leaf bean node is dropped, so when the first node added to the path is customized that current leaf is the path's root node -- and `MutableNode.ROOT_NODE` is a singleton shared by every path. The write therefore stays there for the lifetime of the JVM. It is invisible in the violation path itself, because the first node is re-parented to the pristine materialized root, but every path built afterwards hands the flags out on its first node, e.g. to a `TraversableResolver`, which then sees a top-level property reported as being at an index inside an iterable. Replace the shared root with a private copy before writing to it. Signed-off-by: Donghwan Kim <kimdonghwan913@gmail.com>
|
Thanks for taking a look, and you're right about the report itself — While digging into it I did run into one thing I think is a real bug, so I've cut the PR down to just that part.
// validator A: class-level constraint doing
// .addPropertyNode( "jobs" ).inIterable().atIndex( 3 ).addConstraintViolation()
pollutingValidator.validate( new ClassLevel() );
// validator B, built separately, validating a bean with a single @NotNull String name
validator.validate( new Plain() );Different bean, different validator, next call. Whether or not the chain that triggers it is a sensible thing to write, I don't think a misuse should be able to poison every validation that follows it — and the JPA resolver is on the receiving end of those nodes. The fix is now three calls in Happy to close this if you'd rather leave it as is, and I can move it to a fresh JIRA key since HV-1916 is closed — whichever you prefer. |
https://hibernate.atlassian.net/browse/HV-1916
Reduced after review. The reported symptom is indeed a misuse of the API — see the discussion below. What is left here is the one defect the investigation turned up, which is independent of it.
MutableNode.ROOT_NODEis a singleton shared by every path.inIterable()/atIndex()/atKey()/inContainer()write the container information onto the current leaf node of the path being built, and for a violation built from a class-level constraint — whereConstraintViolationBuilderImpl#dropLeafNodeIfRequireddrops the leaf bean node — that current leaf is the shared root when the first node of the path is customized:The write then stays on the singleton for the lifetime of the JVM. It never shows up in the violation path itself, because the first node is re-parented to the pristine materialized root on the way out, but every path built afterwards hands the flags out on its first node. A
TraversableResolversees it:traversablePropertyfor a plain@NotNullproperty of an unrelated beanname inIterable=false index=nullname inIterable=true index=3Fix
MutablePathreplaces the shared root with a private copy before any of those mutators writes to it. Violation paths come out exactly as they do today; the only thing that changes is that the write no longer escapes the path it belongs to.Verification
The added test fails on
main(expected [false] but found [true]) and passes with the fix. Fullenginesuite green — 2485 tests, JDK 25 — andspotless:check/checkstyle:checkclean.I used Claude Code while working on this; the analysis and the change are mine and I can walk through them.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on licensing, please check here.