SnakeYAML LoaderOptions.setAllowDuplicateKeys(false) should fail on duplicate keys.

[nielsbasjes]

When creating a YAMLFactory we can specify the SnakeYAML LoaderOptions.

One of the settings in there is to make the load fail if a duplicate key is found in the parsed yaml.

This feature does not work when using Jackson.

To demonstrate (I'm donating these tests to your project) two ways of parsing the same document

• the first test (using the SnakeYaml code) passes (i.e. throws a DuplicateKeyException)
• the second test (using the Jackson code) fails (i.e. does NOT throw a DuplicateKeyException)

String yamlDocument = "key1: a\nkey1: b";

@Test
void loadingDuplicateKeysShouldFailIfNotAllowedSnakeYaml() {
    LoaderOptions yamlLoaderOptions = new LoaderOptions();
    yamlLoaderOptions.setAllowDuplicateKeys(false);

    Yaml yaml = new Yaml(yamlLoaderOptions);
    assertThrows(DuplicateKeyException.class, () -> {
        yaml.load(yamlDocument);
    });
}

@Test
void loadingDuplicateKeysShouldFailIfNotAllowedJackson() throws IOException {
    LoaderOptions yamlLoaderOptions = new LoaderOptions();
    yamlLoaderOptions.setAllowDuplicateKeys(false);

    YAMLFactory yamlFactory = YAMLFactory.builder().loaderOptions(yamlLoaderOptions).build();

    try(YAMLParser yamlParser = yamlFactory.createParser(yamlDocument)) {
        assertThrows(DuplicateKeyException.class, () -> {
            new ObjectMapper().readTree(yamlParser);
        });
    }
}

[yawkat]

only the options supported by snakeyaml's ParserImpl are supported by jackson. allowDuplicateKeys is an option of the snakeyaml binder, not the parser, so it's not supported.

imo the current behavior is reasonable.

[nielsbasjes]

I find being able to fail on duplicate keys a feature I enable most of the time.

Right now I'm setting a flag that is supported by the LoaderOptions yet it does not have the documented effect because the way it has currently been implemented.

I understand that if you have implemented it in a way that cannot support this, but then I should get some kind of "unsupported feature" exception when I try to do this.

Also is there an other way achieve the same effect (i.e. fail on duplicate keys)?

[cowtowncoder]

Jackson actually does have StreamReadFeature.STRICT_DUPLICATE_DETECTION to enable such handling -- but only if backend supports it. Currently YAML backend doesn't.

Now, like @yawkat explained, the reason option does not work is because it controls part of SnakeYAML we do not use: Jackson YAML backend uses lower level stream API.
Use of LoaderOptions itself is discouraged (although occasionally has to be, for now).

But it would definitely be good if Jackson's STRICT_DUPLICATE_DETECTION was implemented.
It might not be tons of work; there is support for doing it via JsonReadContext that all JsonParser sub-classes use. It's just not wired by YAML module.

I will file a separate issue in case anyone wanted to tackle this -- I'd be happy to help but may not have time to implement this myself.

[cowtowncoder]

Errr. I may have to let my former self correct me... it looks like I may have implemented support in 2017. At least there is yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/deser/ParserDupHandlingTest.java that appears to verify this functionality.

@nielsbasjes It might be as simple as just enabling StreamReadFeature.STRICT_DUPLICATE_DETECTION on YAMLParser (there's older alias of JsonParser.Feature.STRICT_DUPLICATE_DETECTION as alternative).

[nielsbasjes]

Awesome! So I only have to set it up in a slightly different way.

Question: If you want I can put up a merge request later this week that translates anyone using LoaderOptions() setAllowDuplicateKeys(false) into enabling StreamReadFeature.STRICT_DUPLICATE_DETECTION.

[nielsbasjes]

@cowtowncoder I think this can be made more usable with something as simple as this #415

[cowtowncoder]

Fixed via #415; will be in 2.15.0-rc3.