Skip to content

Suppress case variants of mapped names in isPropertySuppressed - #431

Open
rootvector2 wants to merge 1 commit into
apache:masterfrom
rootvector2:mapped-suppression-case-variant
Open

Suppress case variants of mapped names in isPropertySuppressed#431
rootvector2 wants to merge 1 commit into
apache:masterfrom
rootvector2:mapped-suppression-case-variant

Conversation

@rootvector2

Copy link
Copy Markdown
Contributor

isPropertySuppressed compares the raw expression token against the suppressed set, but the mapped-descriptor fallback it guards builds get<Base>(String) / set<Base>(String, ?) from MappedPropertyDescriptor.capitalizePropertyName(name), so any name that differs only in the case of its first character resolves the same accessor pair and is never matched by the check. With mappedProperty suppressed, mappedProperty(key) is blocked but MappedProperty(key) still reads and writes it, including through BeanUtilsBean.populate with a caller-supplied parameter map. Mapped accessors are never JavaBeans property descriptors, so data.getDescriptor(name) is always null for them and this check is the only thing in front of the accessor.

Comparing on the same capitalized base the descriptor derives keeps the guard and the descriptor from drifting apart again; capitalizePropertyName drops to package-private for that. Found while re-auditing the suppression path from #413.

PropertyUtilsTest.testCustomIntrospectionSuppressedMappedPropertyCaseVariant fails without the runtime change (MappedProperty still resolves a MappedPropertyDescriptor) and passes with it. Full mvn default goal is green: 1284 tests, 0 failures.

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

The check compared the raw expression token, while the mapped-descriptor fallback it guards derives its accessor names from the capitalized property name, so a name differing only in the case of its first character resolved the same accessors and was still readable and writable.
@garydgregory garydgregory changed the title suppress case variants of mapped names in isPropertySuppressed Suppress case variants of mapped names in isPropertySuppressed Jul 29, 2026
@garydgregory
garydgregory requested a review from Copilot July 29, 2026 19:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR closes a suppression bypass for mapped properties where case-variants of the property token (e.g., MappedProperty(key) vs mappedProperty(key)) could still resolve the same mapped accessor pair and remain readable/writable despite suppression.

Changes:

  • Align isPropertySuppressed() with MappedPropertyDescriptor’s accessor-name derivation by comparing suppressed names on the same capitalized base form.
  • Expose MappedPropertyDescriptor.capitalizePropertyName() at package scope to avoid duplication/drift in capitalization logic.
  • Add a regression test covering suppression of a mapped-property case variant.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/test/java/org/apache/commons/beanutils2/PropertyUtilsTest.java Adds a regression test ensuring case-variants of suppressed mapped properties are also blocked.
src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java Updates suppression check to compare on the capitalized base name used for mapped accessor resolution.
src/main/java/org/apache/commons/beanutils2/MappedPropertyDescriptor.java Makes capitalizePropertyName package-private so the suppression logic can reuse the same derivation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

return true;
if (introspector instanceof SuppressPropertiesBeanIntrospector) {
for (final String suppressed : ((SuppressPropertiesBeanIntrospector) introspector).getSuppressedProperties()) {
if (base.equals(MappedPropertyDescriptor.capitalizePropertyName(suppressed))) {
@garydgregory

Copy link
Copy Markdown
Member

@rootvector2 see copilot comments. If you ran copilot on your own branch before creating a PR, we could save some time here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants