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

Optionally allow leading decimal in float tokens #611

Closed
wants to merge 1 commit into from
Closed

Optionally allow leading decimal in float tokens #611

wants to merge 1 commit into from

Conversation

jamesagnew
Copy link

JSON doesn't allow a leading decimal in a float (e.g. { "someval": .123 } ) and Jackson rightly errors out when you try to parse this.

I have a need however (mostly caused by a previous Gson implementation now replaced with Jackson in HAPI FHIR silently accepting this syntax) to parse JSON containing this invalid format.

The attached pull request adds a read feature enabling the parsing of these leading decimal points.

@cowtowncoder
Copy link
Member

@jamesagnew I thought I added a comment here but looks like I did not. So, I think this makes sense: I'll have a look and may be able to merge this to be included in 2.11 (will cherry-pick from master commit, should be fine).
Thank you for submitting the patch.

But one thing I would need (if I haven't yet asked for it; I don't think I have) before merging the patch is CLA. It's found from

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

and is usually easiest to print, fill & sign, scan/photo, email to info at fasterxml dot com.
This is only needed once before the first contribution, one is fine for any and all submissions.

Thank you once again!

@jamesagnew
Copy link
Author

Thanks @cowtowncoder! CLA has been signed and submitted.

cowtowncoder added a commit that referenced this pull request Apr 21, 2020
@cowtowncoder
Copy link
Member

Quick note: with CLA, I am proceeding this one -- but merging pieces manually, mostly due to challenges b/w master and 2.11 branches (wrt refactoring of JsonReadFeature / JsonParser.Feature). Should be done soon.

cowtowncoder added a commit that referenced this pull request Apr 23, 2020
cowtowncoder added a commit that referenced this pull request Apr 23, 2020
cowtowncoder added a commit that referenced this pull request Apr 23, 2020
cowtowncoder added a commit that referenced this pull request Apr 23, 2020
@cowtowncoder cowtowncoder modified the milestones: 2.11., 2.11.0 Apr 23, 2020
@cowtowncoder
Copy link
Member

Finally got it all done; added async-parser tests too (those are separate as input source is different). Thank you again for contributing this! Hoping to now release 2.11.0 by the end of the week.

@jamesagnew
Copy link
Author

@cowtowncoder Appreciate the fast response to a random PR from a new contributor! You're better at this than I am. :)

@cowtowncoder
Copy link
Member

@jamesagnew I've had some experience. But the patch was good and made this pretty easy. Adding new support for non-standard parsing is often tricky due to my over-optimization coding habits, but this turned out to be easier than I'd have expected. :)

Thank you once again for contributing this!

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.

2 participants