Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
brenuart opened this issue Apr 1, 2022 · 3 comments
Open

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

brenuart opened this issue Apr 1, 2022 · 3 comments

Comments

@brenuart
Copy link
Collaborator

brenuart commented Apr 1, 2022

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
Copy link
Collaborator Author

brenuart commented Apr 1, 2022

@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?

@philsttr
Copy link
Collaborator

philsttr commented Apr 2, 2022

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

@brenuart
Copy link
Collaborator Author

brenuart commented Apr 2, 2022

👍

@brenuart brenuart changed the title NestedJsonProvider: fieldName must is *mandatory* but isn't NestedJsonProvider: fieldName must be *mandatory* but isn't Apr 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants