logstash-logback-encoder icon indicating copy to clipboard operation
logstash-logback-encoder copied to clipboard

NestedJsonProvider: fieldName must be *mandatory* but isn't

Open brenuart opened this issue 4 years ago • 3 comments

The *NestedJsonProvider providers allow to nest other providers within a sub-object whose name is given by the fieldName configuration property. Although the fieldName is mandatory, the provider doesn't check if a non-null and non-empty value is specified when started.

The field name is initialised to nested by default but another value can be set via the setFieldName(String) setter. Note that because of the way it works, the Logback XML configuration mechanism will never attempt to call this method with a null or an empty string. In this context the fieldName is therefore always guaranteed to be set to a "valid" value. However this may not be the case when Logback is configured programmatically instead of through an XML file.

Conclusion: the AbstractNestedJsonProvider should ideally validate the fieldName property when started and throw an exception if it is null or empty.

brenuart avatar Apr 01 '22 21:04 brenuart

@philsttr Some thoughts about what is a valid property name...

A null fieldName is obviously not a valid value: Jackson does not allow writing a "null" property name. Although it is questionnable, Jackson accepts an "empty" property name... according to me this should be refused as well. But what about other validations? Should we accept:

  • a string made of blanks, like " " ?
  • should we trim the value ?
  • etc

Until now we took the option to accept "anything" the user has configured. Is it still a valid option?

brenuart avatar Apr 01 '22 21:04 brenuart

I wouldn't restrict anything other than null. Just keep logstash-logback-encoder lightweight and unopinionated and allow anything that jackson allows.

philsttr avatar Apr 02 '22 20:04 philsttr

👍

brenuart avatar Apr 02 '22 21:04 brenuart