WW-5589 Complete UIBean field encapsulation with private fields and public getters #1421
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Comprehensive research document analyzing the root cause of OGNL security warnings when using getText() with resource bundle keys starting with "label". Key findings: - UIBean has protected String label field without public getter - SecurityMemberAccess enforces public-only member access - OGNL property resolution happens before string concatenation evaluation - Warning is a false positive - expression works but triggers introspection Analysis includes: - Detailed OGNL evaluation flow through CompoundRootAccessor - SecurityMemberAccess check sequence and blocking mechanism - Select tag listValue processing and iterator stack manipulation - Comparison with similar protected fields in other components Recommended solution: Change protected fields to private with public getters to follow JavaBean conventions and eliminate warnings (Date component pattern). Related: WW-5364 added components package to OGNL allowlist (commit 39c3e33) Closes [WW-5368](https://issues.apache.org/jira/browse/WW-5368) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Change UIBean and related component fields from protected to private
with public getters to prevent false-positive OGNL SecurityMemberAccess
warnings when evaluating expressions with resource bundle keys.
Previously, expressions like getText('label.key.'+top) would trigger
warnings: "Access to non-public [protected String UIBean.label] is blocked!"
because OGNL attempted to access protected fields directly.
Changes:
- UIBean: Changed label, name, value, id fields to private, added getters
- Bean, Param, Text, I18n: Changed name/value fields to private, added getters
- Updated all subclasses to use getters instead of direct field access
- Added test to verify OGNL can access fields via public getters
Fixes #WW-5368
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
…th getters - Convert all remaining protected fields in UIBean to private: - Template-related: templateSuffix, template, templateDir, theme - Style/CSS: cssClass, cssStyle, cssErrorClass, cssErrorStyle - Form attributes: key, disabled, tabindex, title, accesskey - Label attributes: labelPosition, labelSeparator, requiredPosition, errorPosition, requiredLabel - Event handlers: onclick, ondblclick, onmousedown, onmouseup, onmouseover, onmousemove, onmouseout, onfocus, onblur, onkeypress, onkeydown, onkeyup, onselect, onchange - Tooltip attributes: tooltip, tooltipConfig, javascriptTooltip, tooltipDelay, tooltipCssClass, tooltipIconPath - Dynamic attributes map - Add public getter methods for all converted fields with JavaDoc - Update UIBean subclasses to use getters instead of direct field access: - Anchor.java: use getTemplate() - DoubleSelect.java: use getOnchange() - Link.java: use getDisabled(), getTitle() - Submit.java: use getKey(), getTemplate() - Label.java: use getKey() - Reset.java: use getKey() - Add comprehensive test coverage in UIBeanTest: - testNoOgnlWarningsForAdditionalFields() verifies OGNL can access all newly converted fields via getters - Tests key, title, disabled, cssClass, template, theme, tabindex, and event handler fields This completes WW-5589 by ensuring all UIBean fields follow JavaBean conventions (private fields with public getters), preventing OGNL from attempting direct field access which could trigger false-positive security warnings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR completes the comprehensive encapsulation of all UIBean fields by converting ALL remaining protected fields to private with public getter methods. This extends the fix from WW-5368 (PR #1420) to cover all protected fields in UIBean.
Closes WW-5589
Background
WW-5368 identified that OGNL triggers false-positive security warnings when resource bundle keys or expressions contain tokens matching protected UIBean field names (e.g., "label", "name", "value"). The root cause was OGNL attempting to access protected fields during expression evaluation before realizing they should be treated as string literals.
PR #1420 fixed the immediate issue by converting just the four most problematic fields (label, name, value, id) from protected to private with public getters. This PR extends that solution to ALL remaining protected fields for consistency and to prevent similar issues with other common field names like "key", "title", "disabled", "template", etc.
Changes
Core Changes to UIBean.java
Converted ALL remaining protected fields to private with public getters:
Template-related fields:
templateSuffix,template,templateDir,themeStyle/CSS fields:
cssClass,cssStyle,cssErrorClass,cssErrorStyleForm attribute fields:
key,disabled,tabindex,title,accesskeyLabel attribute fields:
labelPosition,labelSeparator,requiredPosition,errorPosition,requiredLabelEvent handler fields:
onclick,ondblclick,onmousedown,onmouseup,onmouseover,onmousemove,onmouseoutonfocus,onblur,onkeypress,onkeydown,onkeyup,onselect,onchangeTooltip fields (deprecated):
tooltip,tooltipConfig,javascriptTooltip,tooltipDelay,tooltipCssClass,tooltipIconPathOther:
dynamicAttributesMapAdded comprehensive public getter methods with JavaDoc for all fields.
Subclass Fixes
Updated UIBean subclasses to use getters instead of direct field access:
template→getTemplate()onchange→getOnchange()disabled→getDisabled(),title→getTitle()key→getKey(),template→getTemplate()key→getKey()key→getKey()Test Coverage
Added new test
testNoOgnlWarningsForAdditionalFields()in UIBeanTest that:Test Results
Benefits
Migration Impact
This is a binary-compatible change:
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com