Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX EmptyStringToNullELResolver NPE (issue 5226) #5228

Closed
wants to merge 2 commits into from

Conversation

pizzi80
Copy link
Contributor

@pizzi80 pizzi80 commented Apr 12, 2023

Signed-off-by: pizzi80 <paolo@given2.com>
Signed-off-by: pizzi80 <paolo@given2.com>
@@ -32,7 +34,7 @@ public Class<?> getCommonPropertyType(ELContext context, Object base) {
@Override
@SuppressWarnings("unchecked")
public <T> T convertToType(ELContext context, Object value, Class<T> targetType) {
if (value == null && targetType == String.class) {
if ( value == null && targetType == String.class && EVALUATION_CONTEXT_CLASS_SIMPLE_NAME.equals(context.getClass().getSimpleName()) ) {
Copy link
Contributor

@BalusC BalusC May 13, 2023

Choose a reason for hiding this comment

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

This is really a hack not a fix. This is prone to fail when a different EL implementation is used which doesn't use EvaluationContext as class name in this specific situation.

I'll have to look closer at this specific issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I know 😃 😃
but at the moment I had to fix my fork, and it works at least on Tomcat ...
I'm not able to fix it on a long term perspective 🥲

Copy link
Contributor

@BalusC BalusC May 13, 2023

Choose a reason for hiding this comment

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

This hack goes wrong on #{bean.setValue(null)}. It will unexpectedly set empty string there which will bring us back to zero wrt "EmptyStringToNullELResolver".

@BalusC
Copy link
Contributor

BalusC commented May 20, 2023

Declined for reason as explained in #5226 (comment)

@BalusC BalusC closed this May 20, 2023
@pizzi80 pizzi80 deleted the detached2 branch June 15, 2023 13:48
@arjantijms arjantijms added the 5.0 label Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EmptyStringToNullELResolver NPE
3 participants