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

Restore YamlProcessor duplicate key handling against SnakeYAML 1.18+ (plus compatibility with 1.21) [SPR-16791] #21331

Closed
spring-projects-issues opened this issue May 3, 2018 · 4 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented May 3, 2018

Juergen Hoeller opened SPR-16791 and commented

SnakeYAML breaks our StrictMapAppenderConstructor by using a different createDefaultMap() signature as of SnakeYAML 1.21. It turns out that beyond this current issue, duplicate key detection is broken for SnakeYAML 1.18+ since the YAML parser has some duplicate key handling built in now, never calling Map.put a second time and therefore bypassing our check on the decorated map.

Let's upgrade to SnakeYAML's new policy as of Spring Framework 5.0.6, using LoaderOptions.setAllowDuplicateKeys(false), deprecating our now pointless StrictMapAppenderConstructor (and removing it altogether for 5.1). Stéphane Nicoll, Andy Wilkinson, this might affect Boot as well (remembering #18082).


Affects: 5.0.5

Issue Links:

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Stéphane Nicoll, Andy Wilkinson, since duplicate key detection is effectively broken against SnakeYAML 1.18 (which is out for more than a year already), we could also consider backporting this revision to 5.0.6 and therefore Boot 2.0.2, just leaving StrictMapAppenderConstructor in place in deprecated form there (while it'll be gone in 5.1) . The only real downside is a requirement for SnakeYAML 1.18+ in the 5.0.x line then... but that might not affect many people at this point, in particular not within Boot.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented May 4, 2018

Andy Wilkinson commented

Thanks, Juergen. In Boot 2.0, things have moved on a bit since #18082. We now only use StrictMapAppenderConstructor by sub-classing it (1.5 continues to instantiate it directly). That sub-classing is only done in a private inner class so it should be straightforward for us to move away from it when it's deprecated/removed. As for a SnakeYAML 1.18+ requirement, that's also fine from a Boot perspective as 2.0.x uses 1.19.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Andy Wilkinson, Boot is fine with the latest 5.0.6 snapshot so far? I'm mostly concerned about side effects... after all, we're asking SnakeYAML to disallow duplicate keys at its parser level now, possibly rejecting a few (invalid) things that we couldn't detect before.

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

Juergen Hoeller we've adapted to the new API as of 2.0.2.BUILD-SNAPSHOT (see this commit). CI is green

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants