Skip to content

Honor legacy basic authentication encoding property - #4358

Merged
ramanathan1504 merged 7 commits into
apache:2.xfrom
efegokdemir:fix/4347-auth-encoding
Oct 7, 2026
Merged

ramanathan1504 merged 7 commits into
apache:2.xfrom
efegokdemir:fix/4347-auth-encoding

Conversation

@efegokdemir

Copy link
Copy Markdown
Contributor

Fixes #4347

Problem

BasicAuthorizationProvider reads logging.auth.encoding with an overload that applies the platform default charset. When that property is absent, it also discarded the fallback lookup for the Spring-compatible property.

Solution

  • Read the legacy property without an implicit platform-default fallback.
  • Assign the Spring-compatible property lookup result so UTF-8 is used when neither property is configured.
  • Add a regression test covering non-ASCII credentials and the legacy encoding property.
  • Add the required 2.x changelog entry.

Testing

  • ./mvnw -pl :log4j-core-test -am -Dtest=BasicAuthorizationProviderTest -Dsurefire.failIfNoSpecifiedTests=false -Dspotless.check.skip=true -Dspotless.apply.skip=true -Drat.skip=true -Dcheckstyle.skip=true test — passed
  • git diff --check — passed

Signed-off-by: Efe Gökdemir <efe@rexcode.co.uk>

@ramanathan1504 ramanathan1504 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix is the exact two-line change from #4347, and the test fails on 2.x without it.
Can you add a second test with no encoding property that expects UTF-8 bytes, and name the UTF-8 default in the changelog entry too? Today the UTF_8 fallback in BasicAuthorizationProvider can change to another charset and the test still passes.

Comment thread src/changelog/.2.x.x/4347_auth_encoding.xml Outdated
Comment thread src/changelog/.2.x.x/4347_auth_encoding.xml
@github-project-automation github-project-automation Bot moved this to Changes requested in Log4j pull request tracker Sep 28, 2026
Assisted-by: Codex (GPT-5)
Signed-off-by: Efe Gökdemir <efe@rexcode.co.uk>
@efegokdemir

Copy link
Copy Markdown
Contributor Author

Updated in 07623e6. Added a regression test that verifies non-ASCII credentials use UTF-8 when no encoding property is configured, and clarified the changelog entry to document that default.

git diff --check passes. The focused Maven test could not start in this environment because only Java 8 is installed while the Maven wrapper requires --add-exports; CI should provide the supported JDK.

@ramanathan1504 ramanathan1504 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@efegokdemir Thanks!

Comment thread src/changelog/.2.x.x/4347_auth_encoding.xml Outdated
@ramanathan1504
ramanathan1504 enabled auto-merge (squash) October 7, 2026 04:02
@ramanathan1504
ramanathan1504 merged commit c0de1b8 into apache:2.x Oct 7, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

BasicAuthorizationProvider ignores logging.auth.encoding and does not default to UTF-8

2 participants