Should not put Boolean into List<String> property
I have a bean that I do NOT want to have a matching getter (yes, I know that technically it's not a bean in that case, but still), but when I do so SnakeYaml's guess at the data type is wrong.
In fact, it generates a List<String> that contains a Boolean!
public void testListOfStrings() {
Constructor constructor = new Constructor();
constructor.addTypeDescription(new TypeDescription(Foo.class, "!foo"));
Yaml yaml = new Yaml(constructor);
Foo foo = (Foo) yaml.load("!foo\ncountryCodes: [NZ,AU,NO]");
assertEquals(3, foo.countryCodes.size());
assertEquals("NZ", foo.countryCodes.get(0));
assertEquals("AU", foo.countryCodes.get(1));
assertEquals("NO", foo.countryCodes.get(2));
}
public static class Foo {
private List<String> countryCodes = new ArrayList<String>();
public void setCountryCodes(List<String> countryCodes) {
for (Object countryCode : countryCodes) {
System.out.println(countryCode.getClass().getName());
}
this.countryCodes = countryCodes;
}
}
The output of this is:
java.lang.String
java.lang.String
java.lang.Boolean
java.lang.ClassCastException: java.lang.Boolean cannot be cast to java.lang.String
at examples.CustomHalfBeanResolverTest.testListOfStrings(CustomHalfBeanResolverTest.java:65)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:86)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:78)
at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:212)
at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:68)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at com.intellij.rt.execution.application.AppMain.main(AppMain.java:140)
This is because somewhere there is code that think 'NO' means 'False', rather than 'Norway' :-)
I know I have work-arounds:
- Add a Getter (even if it just throws an exception!)
- Put quotes around the country codes
.... but I think SnakeYaml can be a little bit more clever by looking at 'setters' to find a data type if there is no 'getter'.
Thanks for listening. This was a great bug to find -- a List<String> that contains a Boolean!
PS: Version 1.18 (test run against trunk actually, using Java 1.8.0_121
Comments (14)
-
-
Thanks for listening. This was a great bug to find -- a List<String> that contains a Boolean!
Exactly the same happens if you do this: ["foo", "bar", false]
-
Feel free to point out where SnakeYAML does not work according to the spec.
(Otherwise the issue will be closed.)
-
Account Deactivated reporter Hmmm. I would think that List<String> is a pretty strong hint that it's a list of Strings, and therefore all associated values should be treated as Strings, not Booleans.
That said, if you are adamant that there is no bug here, then I feel obliged to report another bug, because according to your logic the following test should fail, and it doesn't. The presence of the 'getCountryCodes' method changes the behaviour.
public void testThatShouldFailApparently() { Constructor constructor = new Constructor(); constructor.addTypeDescription(new TypeDescription(Foo.class, "!foo")); Yaml yaml = new Yaml(constructor); Foo foo = (Foo) yaml.load("!foo\ncountryCodes: [NZ,AU,NO]"); assertEquals(3, foo.countryCodes.size()); assertEquals("NZ", foo.countryCodes.get(0)); assertEquals("AU", foo.countryCodes.get(1)); assertEquals("NO", foo.countryCodes.get(2)); } public static class Foo { private List<String> countryCodes = new ArrayList<String>(); // If this method is not here then "NO" is a Boolean, // but when this method is here, then "NO" is a String. // *I* think it should always be considered a String, given the 'countryCodes' datatype :-) public List<String> getCountryCodes() { throw new RuntimeException("I serve no purpose other than to allow 'NO' to be treated as a String"); } public void setCountryCodes(List<String> countryCodes) { for (Object countryCode : countryCodes) { System.out.println(countryCode.getClass().getName()); } this.countryCodes = countryCodes; } }
The "guessing" I was talking about seems to be happening somewhere in
org.yaml.snakeyaml.introspector.PropertyUtils
, I think. -
I added a test to show how it works.
Feel free to ask if it is still unclear.
-
maybe
#322is related -
Account Deactivated reporter Sadly, I do find it unclear :-(
testStaticFooWithoutGetter seems to be saying that NO in a List<String> should be interpreted as Boolean.FALSE, but YES in a String should be interpreted as the string literal 'YES'. That doesn't make a lot of sense to me.
If the properties were declared as Object rather than String then this data-type selection of NO --> Boolean would be fine.
I'm also not sure what PublicFooWithoutGetter is proving. Yes, there is no getter, but because the value is public there's no need for a getter. What happens if you make 'countryCodes' private instead -- that's the more realistic example (for me).
Fundamentally, can you explain why anybody would expect different behaviour, depending on whether there is a getter or not -- especially when the difference is so subtle, and catastrophic.
-
As I mentioned in previous comment, I believe it is related to
#322and to the fact that we are not so "smart" in detecting Property Type when readMethod is missing. -
- changed status to duplicate
Duplicate of
#322. -
Account Deactivated reporter I agree. It's exactly that problem. I'm sorry I didn't explain myself clearer. You'll notice that your
#322comment was posted at the same time as my last one -- so I didn't see it. -
- changed status to resolved
fixes
#322and#382Now in case of missing readMethod we are trying to "guess" MethodProperty type from writeMethod's first (should be the only) parameter.→ <<cset 55b33e57b1ab>>
-
@ksainsbury , hope after this changeset we are a bit "smarter" ;) Feel free to add more failing test cases.
-
Account Deactivated reporter Thanks for the speedy fix!
-
It will be delivered in version 1.19: https://bitbucket.org/asomov/snakeyaml/wiki/Changes
- Log in to comment
It is not a "guess". It is a requirement: http://yaml.org/type/bool.html
If you want to tune it check Resolver.