Skip to content

Harmonize Logback's console and file logging charset #46846

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@

package org.springframework.boot.logging.logback;

import java.io.Console;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;

import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.encoder.PatternLayoutEncoder;
Expand Down Expand Up @@ -55,7 +57,7 @@
*/
class DefaultLogbackConfiguration {

private static final String DEFAULT_CHARSET = Charset.defaultCharset().name();
private static final String DEFAULT_CHARSET = StandardCharsets.UTF_8.name();

private static final String NAME_AND_GROUP = "%esb(){APPLICATION_NAME}%esb{APPLICATION_GROUP}";

Expand Down Expand Up @@ -106,7 +108,7 @@ private void defaults(LogbackConfigurator config) {
config.conversionRule("wEx", ExtendedWhitespaceThrowableProxyConverter.class,
ExtendedWhitespaceThrowableProxyConverter::new);
putProperty(config, "CONSOLE_LOG_PATTERN", CONSOLE_LOG_PATTERN);
putProperty(config, "CONSOLE_LOG_CHARSET", "${CONSOLE_LOG_CHARSET:-" + DEFAULT_CHARSET + "}");
putProperty(config, "CONSOLE_LOG_CHARSET", "${CONSOLE_LOG_CHARSET:-" + getConsoleCharset() + "}");
putProperty(config, "CONSOLE_LOG_THRESHOLD", "${CONSOLE_LOG_THRESHOLD:-TRACE}");
putProperty(config, "CONSOLE_LOG_STRUCTURED_FORMAT", "${CONSOLE_LOG_STRUCTURED_FORMAT:-}");
putProperty(config, "FILE_LOG_PATTERN", FILE_LOG_PATTERN);
Expand All @@ -123,6 +125,15 @@ private void defaults(LogbackConfigurator config) {
config.logger("org.springframework.boot.actuate.endpoint.jmx", Level.WARN);
}

private String getConsoleCharset() {
Console console = getConsole();
return (console != null) ? console.charset().name() : DEFAULT_CHARSET;
}

@Nullable Console getConsole() {
return System.console();
}

void putProperty(LogbackConfigurator config, String name, String val) {
config.getContext().putProperty(name, resolve(config, val));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package org.springframework.boot.logging.logback;

import java.io.Console;
import java.nio.charset.Charset;
import java.util.function.BiConsumer;
import java.util.function.Function;

Expand Down Expand Up @@ -78,11 +77,6 @@ public LogbackLoggingSystemProperties(Environment environment,
return super.getConsole();
}

@Override
protected Charset getDefaultFileCharset() {
return Charset.defaultCharset();
}

@Override
protected void apply(@Nullable LogFile logFile, PropertyResolver resolver) {
super.apply(logFile, resolver);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,19 @@

package org.springframework.boot.logging.logback;

import java.io.Console;
import java.io.IOException;
import java.nio.charset.StandardCharsets;

import ch.qos.logback.classic.LoggerContext;
import org.junit.jupiter.api.Test;

import org.springframework.util.StreamUtils;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;

/**
* Tests for {@link DefaultLogbackConfiguration}
Expand All @@ -32,6 +37,10 @@
*/
class DefaultLogbackConfigurationTests {

private final LoggerContext loggerContext = new LoggerContext();

private final LogbackConfigurator logbackConfigurator = new LogbackConfigurator(this.loggerContext);

@Test
void defaultLogbackXmlContainsConsoleLogPattern() throws Exception {
assertThatDefaultXmlContains("CONSOLE_LOG_PATTERN", DefaultLogbackConfiguration.CONSOLE_LOG_PATTERN);
Expand All @@ -42,6 +51,48 @@ void defaultLogbackXmlContainsFileLogPattern() throws Exception {
assertThatDefaultXmlContains("FILE_LOG_PATTERN", DefaultLogbackConfiguration.FILE_LOG_PATTERN);
}

@Test
void consoleLogCharsetShouldUseConsoleCharsetIfConsoleAvailable() {
DefaultLogbackConfiguration logbackConfiguration = spy(new DefaultLogbackConfiguration(null));
Console console = mock(Console.class);
given(console.charset()).willReturn(StandardCharsets.UTF_16);
given(logbackConfiguration.getConsole()).willReturn(console);
logbackConfiguration.apply(this.logbackConfigurator);
assertThat(this.loggerContext.getProperty("CONSOLE_LOG_CHARSET")).isEqualTo(StandardCharsets.UTF_16.name());
}

@Test
void consoleLogCharsetShouldDefaultToUtf8WhenConsoleIsNull() {
DefaultLogbackConfiguration logbackConfiguration = spy(new DefaultLogbackConfiguration(null));
given(logbackConfiguration.getConsole()).willReturn(null);
logbackConfiguration.apply(this.logbackConfigurator);
assertThat(this.loggerContext.getProperty("CONSOLE_LOG_CHARSET")).isEqualTo(StandardCharsets.UTF_8.name());
}

@Test
void consoleLogCharsetShouldUseSystemPropertyIfSet() {
withSystemProperty("CONSOLE_LOG_CHARSET", StandardCharsets.US_ASCII.name(), () -> {
new DefaultLogbackConfiguration(null).apply(this.logbackConfigurator);
assertThat(this.loggerContext.getProperty("CONSOLE_LOG_CHARSET"))
.isEqualTo(StandardCharsets.US_ASCII.name());
});
}

@Test
void fileLogCharsetShouldUseSystemPropertyIfSet() {
withSystemProperty("FILE_LOG_CHARSET", StandardCharsets.ISO_8859_1.name(), () -> {
new DefaultLogbackConfiguration(null).apply(this.logbackConfigurator);
assertThat(this.loggerContext.getProperty("FILE_LOG_CHARSET"))
.isEqualTo(StandardCharsets.ISO_8859_1.name());
});
}

@Test
void fileLogCharsetShouldDefaultToUtf8() {
new DefaultLogbackConfiguration(null).apply(this.logbackConfigurator);
assertThat(this.loggerContext.getProperty("FILE_LOG_CHARSET")).isEqualTo(StandardCharsets.UTF_8.name());
}

private void assertThatDefaultXmlContains(String name, String value) throws Exception {
String expected = "<property name=\"%s\" value=\"%s\"/>".formatted(name, value);
assertThat(defaultXmlContent()).contains(expected);
Expand All @@ -51,4 +102,21 @@ private String defaultXmlContent() throws IOException {
return StreamUtils.copyToString(getClass().getResourceAsStream("defaults.xml"), StandardCharsets.UTF_8);
}

private static void withSystemProperty(String name, String value, Runnable action) {
String previous = System.getProperty(name);
try {
System.setProperty(name, value);
action.run();
}
finally {
if (previous != null) {
System.setProperty(name, previous);
}
else {
System.clearProperty(name);
}
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,10 @@ void consoleCharsetWhenNoPropertyUsesSystemConsoleCharsetWhenAvailable() {
}

@Test
void fileCharsetWhenNoPropertyUsesDefault() {
void fileCharsetWhenNoPropertyUsesUtf8() {
new LoggingSystemProperties(new MockEnvironment()).apply(null);
assertThat(System.getProperty(LoggingSystemProperty.FILE_CHARSET.getEnvironmentVariableName()))
.isEqualTo(Charset.defaultCharset().name());
.isEqualTo(StandardCharsets.UTF_8.name());
}

}
Loading