wylovelyi commented on issue #2522:
URL:
https://github.com/apache/logging-log4j2/issues/2522#issuecomment-6055635556
I reproduced this, but I think the root cause is a bit different from what's
assumed here, and it affects more patterns than just `1.2.*`. Posting to
confirm the intended semantics before attempting a fix, because the docs seem
to be internally inconsistent here.
## What actually happens
On `2.x` (current `main`, commit `38a9240` for `NameAbbreviator.java`), with
`org.apache.commons.test.Foo`:
| Pattern | Actual output | Docs say |
|---|---|---|
| `%c{1.}` | `o.a.c.Foo` | `o.a.c.Foo` ✅ |
| `%c{1.1.1.*}` | `o.a.c.test.Foo` | `o.a.c.test.Foo` ✅ |
| `%c{1.2.*}` | `o.ap.commons.test.Foo` | `o.a.c.test.Foo` ❌ |
| `%c{1.3.*}` | `o.apa.commons.test.Foo` | `o.a.commons.test.Foo` ❌ |
| `%c{1.8.*}` | `o.apache.commons.test.Foo` | `org.apache.commons.test.Foo`
❌ |
So `1.3.*` and `1.8.*` are affected by exactly the same problem — I don't
see them mentioned anywhere, which suggests they've been silently wrong since
the pattern support was added. `1.1.1.*` only looks correct by coincidence: all
three counts happen to be `1`.
## Root cause
`NameAbbreviator.getAbbreviator()` (the pattern-parsing loop around line 99)
treats the digit before `*` as a per-segment character count:
```java
if (trimmed.charAt(pos) == '*') {
charCount = Integer.MAX_VALUE;
} else if (trimmed.charAt(pos) >= '0' && trimmed.charAt(pos) <= '9') {
charCount = trimmed.charAt(pos) - '0'; // <-- also consumes the digit
before '*'
...
}
```
For `1.2.*` this produces the fragment list `[1 char, 2 chars, keep-all]`,
so `org` → `o`, `apache` → `ap`, and everything after that is preserved
verbatim. Hence the extra `p` in `o.ap.…`.
## Why I'd like maintainer input before patching
The prose in https://logging.apache.org/log4j/2.x/manual/pattern-layout.html
says:
> If the number after the first period is followed by an asterisk it
indicates how many of the rightmost tokens will be printed in full.
Read literally, that would make `%c{1.2.*}` = "1 char for leading segments +
the 2 rightmost tokens in full" = **`o.a.commons.test.Foo`**, i.e. the current
implementation output would be *wrong* for `1.2.*`.
But the example table in that same section lists `%c{1.3.*}` →
`o.a.commons.test.Foo`, which is only correct under the "digit = char count"
reading that the code implements. So the table and the prose disagree with each
other, and I can't tell which one is intended.
Also worth noting: this report's expected output for `1.2.*`
(`o.a.c.test.Foo`) is actually the documented result for `1.1.1.*`, so I don't
want to assume which reading is being requested.
## Two possible directions
1. **Change the implementation** so that `N.*` means "keep the N rightmost
tokens in full". This would align the code with the prose, but it changes
existing behavior for patterns like `1.2*` and `1.3*` and would break current
assertions in `NameAbbreviatorTest` (e.g. `1.2*` →
`o.a.l.l.c.pattern.NameAbbreviatorTest`). That's a backward-incompatible
change, so it would need a deprecation discussion.
2. **Correct the documentation** — fix the example table so `1.2.*`/`1.3.*`
show what the code actually produces, and clarify the prose to describe the
digit-before-`*` semantics explicitly. Zero behavior change, and it would make
the docs match the (heavily used) existing behavior.
Could you confirm which semantics is intended for `%c{N.*}`? I'm happy to
prepare a PR either way — I just don't want to pick the breaking option
unilaterally.
For reference, my analysis above is based on reading
`log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/NameAbbreviator.java`
and reproducing the parse + abbreviate logic; I haven't yet run it against a
built jar.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
