-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
#3305 - fix default CharSequence serialization #3332
Conversation
* Helper method that will remove all properties that are added by default JDK methods, | ||
* e.g. java.lang.CharSequence.isEmpty() (since JDK 15). | ||
*/ | ||
private void removeDefaultProperties(List<BeanPropertyWriter> properties) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not quite sure about the policy with new protected methods, so I minimized the visibility for the first iteration
Thank you @seregamorph! I think the idea of the fix is sound -- I may want to tweak details a bit, but this makes sense to me. I'll add this on my TODO list: lately my time on Jackson has been severely limited and so it may take a while to get this processed, but I wanted to add a note that it is on my radar. |
Sure, thanks. |
@cowtowncoder I have a feeling that you plan to release the version soon with CVE. Can you please include this fix as well? We need it in our project. Thanks |
@seregamorph Yes, I will definitely get this fix included first. It is next one on my list. |
Thank you @seregamorph! I used this approach, only changing some of minor details (after checking if there might be other places to get rid of the property, finding no better one) -- thank you for suggesting the fix. I'll close the PR; fix will go in 2.13.1, which should be released some time this month (December 2021). |
@cowtowncoder great, thanks!
|
I wasn't think of backporting, but maybe I should. Hard to gauge how important the fix is, vs. any potential regression. As to test, yes, I added it, I think under |
Patch was included in:
|
Fixes #3305 and #3331 (dup).