You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/03/04 02:38:50 UTC
[GitHub] [logging-log4j2] carterkozak opened a new pull request #783: LOG4J2-3424: Correctly handle property attribute values
carterkozak opened a new pull request #783:
URL: https://github.com/apache/logging-log4j2/pull/783
This fixes handling of properties in .properties configurations
as well as those defined using `<Property name="key" value="value"/>`
which are parsed slightly differently from those which use the
`<Property name="key">value</Property>` syntax.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #783: LOG4J2-3424: Correctly handle property attribute values
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #783:
URL: https://github.com/apache/logging-log4j2/pull/783#discussion_r819221798
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/config/Property.java
##########
@@ -118,15 +119,36 @@ public static Property createProperty(final String name, final String value) {
* @param value The value.
* @return A Property.
*/
+ public static Property createProperty(
+ final String name,
+ final String rawValue,
+ final String value) {
+ if (name == null) {
+ LOGGER.error("Property name cannot be null");
+ }
+ return new Property(name, rawValue, value);
+ }
+
+ /**
+ * Creates a Property.
+ *
+ * @param name The key.
+ * @param rawValue The value without any substitution applied.
+ * @param configuration configuration used to resolve the property value from the rawValue
+ * @return A Property.
+ */
@PluginFactory
public static Property createProperty(
@PluginAttribute("name") final String name,
@PluginValue(value = "value", substitute = false) final String rawValue,
- @PluginValue("value") final String value) {
+ @PluginConfiguration final Configuration configuration) {
Review comment:
Attribute are consumed as they're read, this way we consume the value once, and process it using the configuration substitutor.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] garydgregory commented on a change in pull request #783: LOG4J2-3424: Correctly handle property attribute values
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #783:
URL: https://github.com/apache/logging-log4j2/pull/783#discussion_r819689059
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/config/Property.java
##########
@@ -118,15 +119,38 @@ public static Property createProperty(final String name, final String value) {
* @param value The value.
* @return A Property.
*/
+ public static Property createProperty(
+ final String name,
+ final String rawValue,
+ final String value) {
+ if (name == null) {
+ LOGGER.error("Property name cannot be null");
Review comment:
If it's an error and we continue as if nothing happened, then either this should not be logged as an ERROR or some error-like behavior should happen. It's just weird to log an ERROR and then continue as if nothing happened.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak closed pull request #783: LOG4J2-3424: Correctly handle property attribute values
Posted by GitBox <gi...@apache.org>.
carterkozak closed pull request #783:
URL: https://github.com/apache/logging-log4j2/pull/783
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #783: LOG4J2-3424: Correctly handle property attribute values
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #783:
URL: https://github.com/apache/logging-log4j2/pull/783#discussion_r819703318
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/config/Property.java
##########
@@ -118,15 +119,38 @@ public static Property createProperty(final String name, final String value) {
* @param value The value.
* @return A Property.
*/
+ public static Property createProperty(
+ final String name,
+ final String rawValue,
+ final String value) {
+ if (name == null) {
+ LOGGER.error("Property name cannot be null");
Review comment:
Agree that this is odd, it matches the behavior of the previous plugin-based constructor. I have to imagine anything that receives a Property instance with a null key will throw, so it's probably best to throw in this case. I can update all the constructors
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on pull request #783: LOG4J2-3424: Correctly handle property attribute values
Posted by GitBox <gi...@apache.org>.
carterkozak commented on pull request #783:
URL: https://github.com/apache/logging-log4j2/pull/783#issuecomment-1059558347
Merged + changelog
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on pull request #783: LOG4J2-3424: Correctly handle property attribute values
Posted by GitBox <gi...@apache.org>.
carterkozak commented on pull request #783:
URL: https://github.com/apache/logging-log4j2/pull/783#issuecomment-1058805314
planning to clean this up and fix revapi tomorrow
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org