Add quotes for all values listed in https://yaml.org/type/bool.html - Backport #137 for 2.9 #138
Conversation
GuillaumeSmaha
force-pushed
the
backport_protect_value_to_avoid_implict_convertion
branch
from
45101fb to
ed5c229
Compare
Fix FasterXML#129 - Backport of FasterXML#137 for 2.9
GuillaumeSmaha
force-pushed
the
backport_protect_value_to_avoid_implict_convertion
branch
from
ed5c229 to
c46c372
Compare
quotes around string value and fix compatibility with parser using yaml 1.1. Fix OpenAPITools#3196 In spec 1.1, some words without quotes can be implicitly converted to a boolean like "yes" or "on" (See https://yaml.org/type/bool.html) In v3.core.util.Yaml with MINIMIZE_QUOTES enabled, the formatter created a YAML but forget to protect some words. PRs are created to fix this in the formatter: - FasterXML/jackson-dataformats-text#137 - FasterXML/jackson-dataformats-text#138 Until the PRs will be merged and released, this patch fix it by re-adding quotes.
|
@cowtowncoder What do think to backport the fix in 2.9.x ? |
|
@GuillaumeSmaha at this point I'd like to minimize backporting of fixes for stability -- so although this is not necessarily high-risk fix, I am not planning to backport it. In related news, I just released |
|
Thank you, I just wanted to know. |
|
Sure, that's a valid question. But also: I am really interested in getting feedback -- so if you (or someone you know) can run existing regression tests for some system that is using Jackson 2.9, that'd be great. I need to post to mailing lists about this, as well as write a blog post about new features of course. But this is good time to figure out minor compatibility issues that are easy to resolve before 2.10.0 final. |
|
My aim was to fix this issue in OpenAPI generator project. |
|
@cowtowncoder I tried on swagger-core, here my branch: https://github.com/GuillaumeSmaha/swagger-core/tree/upgrade_fasterxml_jackson My first problem was that I forgot yo upgrade I am checking fasterxml aand snakeyaml with their changelog: https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.10 and https://bitbucket.org/asomov/snakeyaml/wiki/Changes but if you have any idea, feel free to share :) |
|
It seems the big number thows now an exception, maybe related to FasterXML/jackson-core@c4dd84e |
|
My initial reaction is that the problem would be related to added Module Info in 2.10 (which 2.9 didn't have), and your running things on JDK 9+ or later? If so, stricter visibility checks may be imposed by JDK, without changes to either SnakeYAML or Jackson. Now... those exception messages are highly confusing, but it could just be reporting wrong range (I think value is outside of As to Map keys (property names), I think there was a change but can not recall details. It's probably a bit complicated issue, wrt what exactly is attempted. |
|
@GuillaumeSmaha Actually, on quoting of Map keys, I'd be interested in knowing more about your special usage. The challenge here is to separate intents so that:
But to get all of this working, there is a bit of work to do. I think I will actually file a new issue to ensure that YAML generator at least implements |
|
@cowtowncoder I just saw your commit 456f36f |
|
Just to inform you. I have other error like that: I will look more in details the change done in 2.10 and see if it is related to this change. |
|
Interesting; that could be related to shading of SnakeYAML. |
|
Yes I think also it can be related to SnakeYAML. I will dig in the diff between 1.23 and 1.24 |
|
One additional note: I think this could be due to Jackson not catching and rethrowing some failures (done to insulate app calling Jackson from internal details of using snakeyaml), so it may be combination of issues. I should be able to add missing handling, if any, to YAML module quite easily, if stack trace can show the call path. |
Fix #129 - Backport of #137 for 2.9