[jira] [Comment Edited] (LOG4J2-2955) SizeBasedTriggeringPolicy with space between the number and the unit does not work e.g. 1 MB does not work but 1MB does work.

["Markus Spann (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/LOG4J2-2955?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17380555#comment-17380555 ] 

Markus Spann edited comment on LOG4J2-2955 at 7/14/21, 12:49 PM:
-----------------------------------------------------------------

As [~rgoers] has pointed out, the pattern in class {{FileSize}} does handle whitespace between numeric expression and unit correctly.

I would like to suggest other improvements to the class:

The numeric part is parsed into a long.
 Therefore any fractional precision (that the regular expression otherwise supports) is lost: {{"0.5 GB" --> 0}}

What's worse the pattern supports comma and period. The comma is used as the decimal separator in many locales. However with {{Locale.ROOT}} comma is treated as grouping character afaik giving {{"0,50 GB" --> 50}}.

There clearly is room for improvement. I suggest to:
 * calculate with double and convert into long at latest possible time
 * treat comma as decimal separator unless comma _and_ period appear in the expression (i.e. replace comma by decimal dot before parsing).
 (It's safe to assume no user means 50 GB when they spell 0,50)
 * act fail-fast on ParseException. The defaults (10 MB or Long.MAX_VALUE) are almost never a good choice.

I will prepare a pull request.


was (Author: spannjp):
As [~rgoers] has pointed out, the pattern in class {{FileSize}} does handle whitespace between numeric expression and unit correctly.

I would like to suggest other improvements to the class:

The numeric part is parsed into a long.
Therefore any fractional precision (that the regular expression otherwise supports) is lost: {{"0.5 GB" --> 0}}

What's worse the pattern supports comma and period. The comma is used as the decimal separator in many locales. However with Locale.ROOT comma is treated as grouping character giving {{"0,50 GB" --> 50}}.

There clearly is room for improvement. I suggest to:
 * calculate with double and convert into long at latest possible time
 * treat comma as decimal separator unless comma and period appear in the expression (i.e. replace comma by decimal dot before parsing).
(It's safe to assume no user means 50 GB when they spell 0,50)

I'll prepare a pull request.

> SizeBasedTriggeringPolicy with space between the number and the unit does not work e.g. 1 MB does not work but 1MB does work.
> -----------------------------------------------------------------------------------------------------------------------------
>
>                 Key: LOG4J2-2955
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-2955
>             Project: Log4j 2
>          Issue Type: Bug
>          Components: Appenders
>    Affects Versions: 2.12.0
>            Reporter: Deepak Khopade
>            Priority: Major
>
> Hello There,
> Recently we found that when we add an appender for {{RollingFile}} , using {{<SizeBasedTriggeringPolicy size="1 MB"/>}}  does not work; meaning when {{1<space>MB}} and when there is no space i.e. {{<SizeBasedTriggeringPolicy size="1MB"/>}} it works as expected.
> Any thoughts if it's a bug?
> Thanks,
> Deepak



--
This message was sent by Atlassian Jira
(v8.3.4#803005)