Regression bug with parser upgrading from 1.10

Issue #317 invalid
Former user created an issue

I decided to update to the latest snakeyaml-1.16.jar and hit a regression bug caused by forced conformance to the JavaBean spec. I also tried 1.12 and 1.15 and hit the same issues but couldn't find anything in the change lists that describes the change in parsing behaviour.

I had set up my Types to use private constructors without setters, because these are configuration objects that affect the runtime and are only used when the config file is parsed. Forcing config Types to use the JavaBean spec effectively creates a public API, which now requires documentation to explain. The old code used reflection to invoke private constructors and populate private fields directly, which is much safer and hides the configuration implementation.

So, to make the new snakeyaml parser work with my code required me to change all my config types to use public constructors and setters. This was bad enough. But then I discovered that ALL fields need to have public mutators; I designed a Bean as a facade, whereby my yaml String was transformed to a typesafe object in the accessor. What I just learned is that to make the new snakeyaml parser to work, I have to create a public mutator and expose this field! These changes broke the fundamental rule of an API, changing fundamental behaviour without a deprecation warning.

Comments (12)

  1. Alexander Maslov

    This sounds strange. Default private constructor should work and private fields also.

    If you could share failing code, I could try to figure out what's wrong.

  2. Former user Account Deleted

    Setting the bean access method fixes the problem. Was this new behaviour introduced post 1.10? If so, probably a good idea to add it to the change list for when it was introduced.

    Thanks.

  3. Andrey Somov

    Dear Robin, can you please show the code which fails ? Yaml.setBeanAccess(BeanAccess.FIELD) is in production since v1.7 There must be something else which is influencing the problem.

  4. Former user Account Deleted

    snippets from my config:

    ```

    --- !!com.xxx.config.ServerConfig
    servers: 
    - &listener1 { 
       id: socket,
       enabled: true,
       address: 127.0.0.1,
       port: 4444
     }
    
    logparsers:
    - &utf-8 { 
       layout: none 
     }
    

    ...

    package com.xxx.config;
    public final class ServerConfig
    {
      public /*final*/ Set<Server> servers;
      public /*final*/ Set<MessageParser> logparsers;
      public /*final*/ Map<Server, Codec> servercodecs;
    }
    
    
    package com.xxxx.parser;
    public final class MessageParser
    {
      // these values are injected when the yaml config is loaded
      private/* final */String layout;
      private/* final */String pattern;
      private/* final */List<String> fields;
    
      private Layout parserLayout;
    
      // fields are set through reflection by the yaml parser and the constructor likewise
      @SuppressWarnings( "unused" )
      private MessageParser()
      {}
    
      // other accessors and such 
      @Test
      public void testConfigFileIsValid()
        throws FileNotFoundException
      {
        Yaml yaml = new Yaml();
        //yaml.setBeanAccess( BeanAccess.FIELD );   // uncomment to make the parser work correctly post 1.10
        URL url = ClassLoader.getSystemResource( "config.yml" );
        ServerConfig config = (ServerConfig)yaml.load( new FileReader( url.getFile() ) );
    
        Assert.assertNotNull( config );
      }
    

    So the default behaviour seems to have changed after 1.10, whereby the private fields and private constructor are not reflected the same way. The private field parserLayout is created on first access to the getLayout() method. Before using the setBean behaviour, I had to create a mutator for the field, as well as the fields that the yaml parser populates.

  5. Former user Account Deleted

    The ServerConfig is as you see it. The MessageParser class is the problematic one. When the ServerConfig's Set<MessageParser> is processed, which requires instantiating MessageParser types, the failure occurs. The failures can be individually controlled, such that changing the constructor to be public fixes one error, adding mutators for the private fields that are populated by the yaml config fixes another error, and adding a mutator for the Layout type, which would cover the Layout field that is not related to the yaml config file, fixes another error. All of these errors are mitigated by setting the bean property you mentioned.

    p.s. I had issues with the bitbucket parser messing up my code snippets, so I had to move them all to a single code block. The @Test annotation belongs in a test class and is not part of the MessageParser.

  6. Alexander Maslov

    I have tried your example and in is NOT POSSIBLE to load it without BeanAccess.FIELD in any tried version (1.5, 1.6, 1.7, 1.10.) There is no easy way to load it in 1.5 and 1.6 simply because there was no BeanAccess.FIELD. Starting from 1.7 if you want SnakeYAML to load "automagically" your classes you MUST use BeanAccess.FIELD.

    There should be some other reason why it works in your setup like it works (the field access has been added as part of the fix for issue 55) and it is mentioned in changes.xml

  7. Andrey Somov

    I share the same view - there is no regression problem in SnakeYAML. I think the only way to proceed is to extract the failing code to a small independent project which we can test.

  8. Former user Account Deleted

    Well, I'm not sure from where I downloaded the parser that I was using, but its release name was snakeyaml-1.10-android.jar. I don't have the source and I haven't decompiled it. I was curious about the -android part in the name but it worked perfectly well. I found it a few months ago and I'm pretty sure it was from a google-code site. Is this jar not related to your project?

  9. Former user Account Deleted

    Sorry for the confusion. I guess I found the project at about the time things were changing to bitbucket? I tried to find a project site with source, etc., at the time I was looking for a parser, to make sure the project was active, but didn't come across bitbucket for some reason.

    But now I'm curious why field access is not the default? Considering that beans are populated from a config file, I expect the standard developer use case would be to configure the runtime once and not change app state by modifying/re-injecting the yaml-beans. For this reason, from an API perspective, immutable beans seem like the safest approach. Then it's up to the developer to create a public API explicitly. And while it's obviously possible to hide the config with the bean access setting, it doesn't really jump out in the documentation.

    Thanks.

  10. Andrey Somov

    We have mailing list for general questions and proposals. Putting unrelated stuff together does not help other developers (and Google) to find and solve problems.

  11. Log in to comment