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 #99 do not close stream when autoclose is disabled in yaml generator/parser #112

Merged

Conversation

vboulaye
Copy link
Contributor

fix for #99
I reported the checks done before closing the enclosed writer from the WriterBasedJsonGenerator in jackson core to the YamlGenerator
A similar check was apparently also missing in the parser, I copied the one from the ReaderBasedJsonParser to the YamlParser
I also copied the original comments that explain the reasoning behind the checks (even though they date from 2008, they still apply)

@vboulaye vboulaye force-pushed the closed-stream-when-autoclose-disabled branch from 08d9da2 to 97952f0 Compare October 20, 2018 20:07
@cowtowncoder
Copy link
Member

Looks legit, thank you very much for contributing this!

Before I merge the fix to be included in 2.9.8 (thank you also for making this against 2.9 branch, makes my life easier), one last thing: unless I have asked for and received CLA (apologies if so), we'd need it from:

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

(or, if you need corporate-CLA for work done on behalf of a company, there's alternative)

and the usual way is to print, fill & sign, scan, email to info at fasterxml dot com.
One CLA is enough for all Jackson projects, contributions, so it's just one-time hassle.
Once that is done, I'll merge this in .

Thank you again!

@vboulaye
Copy link
Contributor Author

I just sent the CLA.

By the way thank you for everything you do on this project, I'm glad I could help a little bit!

@cowtowncoder cowtowncoder merged commit 9666f9b into FasterXML:2.9 Oct 24, 2018
@cowtowncoder cowtowncoder added this to the 2.9.8 milestone Oct 24, 2018
@cowtowncoder cowtowncoder added the yaml Issue related to YAML format backend label Oct 24, 2018
@cowtowncoder
Copy link
Member

Thank you for fixing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
yaml Issue related to YAML format backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants