ppkarwasz commented on issue #3153: URL: https://github.com/apache/logging-log4j2/issues/3153#issuecomment-2453323116
@JWT007, Thank you for catching this problem. I believe that the missing validation affects both types of configuration. When configuring through a configuration file, it does not make sense to have an **empty** `text` configuration property. The field should be annotated with [`@Required`](https://logging.apache.org/log4j/2.x/javadoc/log4j-core/org/apache/logging/log4j/core/config/plugins/validation/constraints/Required.html), which in this case will actually check that the parameter is not empty. In case of programmatic configuration there are two possible solutions: 1. We can use the [`Builder.isValid()`](https://logging.apache.org/log4j/2.x/javadoc/log4j-core/org/apache/logging/log4j/core/util/Builder.html#isValid()) method I introduced in #771, e.g. `if (!isValid()) return false;`. This has the advantage of working even if the caller forgets to call `setMatchString` on the builder. 2. We can add a call to [`Assert.requireNonEmpty()`](https://logging.apache.org/log4j/2.x/javadoc/log4j-core/org/apache/logging/log4j/core/util/Assert.html#requireNonEmpty(T)) in the setter. I was not a big fan of this solution in #1231, but it doesn't seem so bad to me right now. Probably we should implement both solutions at once. Would you be willing to make a PR? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
