Issue with Duration Parsing

[RapidTransit]

I was working on something unrelated and needed to parse a string like 10s or 10ms, I did a quick search in my project and came up with:

logging-log4j2/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java

Lines 639 to 674 in b4986fe

	private enum TimeUnit {
	NANOS("ns,nano,nanos,nanosecond,nanoseconds", ChronoUnit.NANOS),
	MICROS("us,micro,micros,microsecond,microseconds", ChronoUnit.MICROS),
	MILLIS("ms,milli,millis,millsecond,milliseconds", ChronoUnit.MILLIS),
	SECONDS("s,second,seconds", ChronoUnit.SECONDS),
	MINUTES("m,minute,minutes", ChronoUnit.MINUTES),
	HOURS("h,hour,hours", ChronoUnit.HOURS),
	DAYS("d,day,days", ChronoUnit.DAYS);
	private final String[] descriptions;
	private final ChronoUnit timeUnit;
	TimeUnit(final String descriptions, final ChronoUnit timeUnit) {
	this.descriptions = descriptions.split(",");
	this.timeUnit = timeUnit;
	}
	ChronoUnit getTimeUnit() {
	return this.timeUnit;
	}
	static Duration getDuration(final String time) {
	final String value = time.trim();
	TemporalUnit temporalUnit = ChronoUnit.MILLIS;
	long timeVal = 0;
	for (TimeUnit timeUnit : values()) {
	for (String suffix : timeUnit.descriptions) {
	if (value.endsWith(suffix)) {
	temporalUnit = timeUnit.timeUnit;
	timeVal = Long.parseLong(value.substring(0, value.length() - suffix.length()));
	}
	}
	}
	return Duration.of(timeVal, temporalUnit);
	}
	}

I'm not sure if it is a bug, but when it finds the first matching suffix it should immediately return, other wise it always falls through to SECONDS for most of the covered cases and the order is slightly messed up as the SECONDS could produce an early match for MINUTES, HOURS, and DAYS.

[ppkarwasz]

@RapidTransit,

Thank you for catching this bug. As far as I can see, most invocations of this code (especially if the unit ends in s) ends up with an uncaught NumberFormatException.

@rgoers, this is only used in the Kubernetes plugin. Can we replace the entire logic with Duration.parse?

[ppkarwasz]

Since the only users of PropertiesUtil#getDurationProperty has been removed in #2218, I propose to replace the implementation of the method with a simple Duration#parse.

[vy]

@ppkarwasz, if there are no other system properties that need a TypeConverter<Duration>, but the Kubernetes module, which is planned to be removed, remove this TypeConverter<Duration> along with it too, please. Note that this implies no TypeConverter<Duration>, not even one using Duration.parse(String).

[ppkarwasz]

@vy,

In Log4j Core there is no java.time.Duration converter, there is however an o.a.l.l.c.a.rolling.action.Duration converter created in the well-known spirit of bringing to Java 7 users Java 8 features today!

I can deprecate it and replace its usages with java.time.Duration, but I'd rather do it in a separate PR.