Bump SnakeYAML and Expose Parse Options #13
Conversation
This ends up breaking the new `too-many-aliases-works` test
|
@slipset I guess it would be useful to set up CircleCI and Github in such a way that it's easy to verify if a PR breaks tests. |
|
@borkdude Looking through all the options, it seems like it should build PR's, but I can see that it doesn't. |
`max-aliases` exposes `setMaxAliasesForCollections` `allow-recursive` exposes `setAllowRecursiveKeys`
erichaberkorn
force-pushed
the
bump-snakeyaml
branch
from
78d0d90 to
4d4fda6
Compare
|
@slipset It seems the last build on master didn't do anything:
|
|
This is very strange. I have CircleCI setup on my fork and the checks are showing up properly there https://github.com/erichaberkorn/clj-yaml/commits/bump-snakeyaml |
|
@erichaberkorn Ah, that may be the issue. If you have your own CircleCI setup, then it won't run as a PR build probably. |
test/clj_yaml/core_test.clj
Outdated
| ") | ||
|
|
||
| (deftest allow-recursive-works | ||
| (is (thrown? YAMLException (parse-string recursive-yaml))) |
There was a problem hiding this comment.
Let's change this assertion to thrown-with-msg? and check that the exception has the word Recursive in the message.
test/clj_yaml/core_test.clj
Outdated
| (string/join "\n"))) | ||
|
|
||
| (deftest max-aliases-for-collections-works | ||
| (is (thrown? YAMLException (parse-string too-many-aliases))) |
There was a problem hiding this comment.
Let's change this assertion to thrown-with-msg? and check that the exception has the string "Number of aliases" in the message.
I made this change in three commits because it demonstrates that this is a breaking change. All the tests pass on d590965 and bumping to SnakeYAML 1.26 on f385f74 (even though it is a minor version bump) causes the test to fail.
SnakeYAML 1.26 patches CVE-2017-18640, which resulted in
LoaderOptionsaddingsetMaxAliasesForCollectionsandsetAllowRecursiveKeysto control the amount of aliases SnakeYAML will attempt to parse and whether it allows recursive keys. This PR exposes those options toparse-stringvia:max-aliases-for-collectionsand:allow-recursive-keys.