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

Off by 1 bug in PropertyValueBuffer #1432

Closed
kpdonn opened this issue Oct 26, 2016 · 4 comments
Closed

Off by 1 bug in PropertyValueBuffer #1432

kpdonn opened this issue Oct 26, 2016 · 4 comments
Milestone

Comments

@kpdonn
Copy link

kpdonn commented Oct 26, 2016

Came across this looking at FasterXML/jackson-module-kotlin#46

There is a bug here: https://github.com/FasterXML/jackson-databind/blob/master/src/main/java/com/fasterxml/jackson/databind/deser/impl/PropertyValueBuffer.java#L276-L281

_paramsSeenBig.set(ix); needs to be before if (--_paramsNeeded <= 0) { return true; } otherwise it is skipped for the last element. That code block only gets executed if there are 32+ parameters.

@cowtowncoder
Copy link
Member

cowtowncoder commented Oct 26, 2016

Thank you for reporting this. I had the feeling something like this would be wrong based on that comment for a problem you reported for kotlin module.

Would it be easy to get a simple repro to guard against regression? If not, I can try to cook one up.

Either way I can then fix this for 2.8(.5)

@kpdonn
Copy link
Author

kpdonn commented Oct 27, 2016

There is a test in Kotlin in FasterXML/jackson-module-kotlin#46. It doesn't look like the bug has any outward effects normally in Java since the only thing it seems to break is PropertyValueBuffer.hasParameter() which isn't normally called.

Here is something I managed to come up with to reproduce it in plain Java: https://gist.github.com/kpdonn/368a760ab52a9bbb0b334c2a37fae0d2

@cowtowncoder
Copy link
Member

@kpdonn Appreciate your help, thank you!

@cowtowncoder cowtowncoder added this to the 2.7.9 milestone Oct 28, 2016
@cowtowncoder cowtowncoder removed the 2.8 label Oct 28, 2016
cowtowncoder added a commit that referenced this issue Oct 28, 2016
@apatrida
Copy link
Member

I uncommented out the test in the kotlin module for the 2.9.x branch, it now passes.

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

No branches or pull requests

3 participants