Migrate from NCSARequestLog to CustomRequestLog

Closes gh-16416

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java

@@ -25,9 +25,7 @@ import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
-import java.util.Locale;
 import java.util.Map;
-import java.util.TimeZone;
 
 import io.undertow.UndertowOptions;
 
@@ -890,6 +888,17 @@ public class ServerProperties {
 			 */
 			private boolean enabled = false;
 
+			/**
+			 * Log format.
+			 */
+			private FORMAT format = FORMAT.NCSA;
+
+			/**
+			 * Custom log format, see org.eclipse.jetty.server.CustomRequestLog. If
+			 * defined, overrides the "format" configuration key.
+			 */
+			private String customFormat;
+
 			/**
 			 * Log filename. If not specified, logs redirect to "System.err".
 			 */
@@ -910,47 +919,6 @@ public class ServerProperties {
 			 */
 			private boolean append;
 
-			/**
-			 * Enable extended NCSA format.
-			 */
-			private boolean extendedFormat;
-
-			/**
-			 * Timestamp format of the request log.
-			 */
-			private String dateFormat = "dd/MMM/yyyy:HH:mm:ss Z";
-
-			/**
-			 * Locale of the request log.
-			 */
-			private Locale locale;
-
-			/**
-			 * Timezone of the request log.
-			 */
-			private TimeZone timeZone = TimeZone.getTimeZone("GMT");
-
-			/**
-			 * Enable logging of the request cookies.
-			 */
-			private boolean logCookies;
-
-			/**
-			 * Enable logging of the request hostname.
-			 */
-			private boolean logServer;
-
-			/**
-			 * Enable logging of request processing time.
-			 */
-			private boolean logLatency;
-
-			/**
-			 * Whether to log IP address from the "X-Forwarded-For" header rather than the
-			 * one from the connection.
-			 */
-			private boolean preferProxiedForAddress = false;
-
 			/**
 			 * Request paths that should not be logged.
 			 */
@@ -964,6 +932,22 @@ public class ServerProperties {
 				this.enabled = enabled;
 			}
 
+			public FORMAT getFormat() {
+				return this.format;
+			}
+
+			public void setFormat(FORMAT format) {
+				this.format = format;
+			}
+
+			public String getCustomFormat() {
+				return this.customFormat;
+			}
+
+			public void setCustomFormat(String customFormat) {
+				this.customFormat = customFormat;
+			}
+
 			public String getFilename() {
 				return this.filename;
 			}
@@ -996,76 +980,30 @@ public class ServerProperties {
 				this.append = append;
 			}
 
-			public boolean isExtendedFormat() {
-				return this.extendedFormat;
-			}
-
-			public void setExtendedFormat(boolean extendedFormat) {
-				this.extendedFormat = extendedFormat;
-			}
-
-			public String getDateFormat() {
-				return this.dateFormat;
-			}
-
-			public void setDateFormat(String dateFormat) {
-				this.dateFormat = dateFormat;
-			}
-
-			public Locale getLocale() {
-				return this.locale;
-			}
-
-			public void setLocale(Locale locale) {
-				this.locale = locale;
-			}
-
-			public TimeZone getTimeZone() {
-				return this.timeZone;
-			}
-
-			public void setTimeZone(TimeZone timeZone) {
-				this.timeZone = timeZone;
-			}
-
-			public boolean isLogCookies() {
-				return this.logCookies;
-			}
-
-			public void setLogCookies(boolean logCookies) {
-				this.logCookies = logCookies;
-			}
-
-			public boolean isLogServer() {
-				return this.logServer;
-			}
-
-			public void setLogServer(boolean logServer) {
-				this.logServer = logServer;
-			}
-
-			public boolean isLogLatency() {
-				return this.logLatency;
+			public List<String> getIgnorePaths() {
+				return this.ignorePaths;
 			}
 
-			public void setLogLatency(boolean logLatency) {
-				this.logLatency = logLatency;
+			public void setIgnorePaths(List<String> ignorePaths) {
+				this.ignorePaths = ignorePaths;
 			}
 
-			public boolean isPreferProxiedForAddress() {
-				return this.preferProxiedForAddress;
-			}
+			/**
+			 * Log format for Jetty access logs.
+			 */
+			public enum FORMAT {
 
-			public void setPreferProxiedForAddress(boolean preferProxiedForAddress) {
-				this.preferProxiedForAddress = preferProxiedForAddress;
-			}
+				/**
+				 * NCSA format, as defined in CustomRequestLog#NCSA_FORMAT.
+				 */
+				NCSA,
 
-			public List<String> getIgnorePaths() {
-				return this.ignorePaths;
-			}
+				/**
+				 * Extended NCSA format, as defined in
+				 * CustomRequestLog#EXTENDED_NCSA_FORMAT.
+				 */
+				EXTENDED_NCSA
 
-			public void setIgnorePaths(List<String> ignorePaths) {
-				this.ignorePaths = ignorePaths;
 			}
 
 		}

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizer.java

@@ -21,9 +21,10 @@ import java.util.Arrays;
 
 import org.eclipse.jetty.server.AbstractConnector;
 import org.eclipse.jetty.server.ConnectionFactory;
+import org.eclipse.jetty.server.CustomRequestLog;
 import org.eclipse.jetty.server.Handler;
 import org.eclipse.jetty.server.HttpConfiguration;
-import org.eclipse.jetty.server.NCSARequestLog;
+import org.eclipse.jetty.server.RequestLogWriter;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.handler.ContextHandler;
 import org.eclipse.jetty.server.handler.HandlerCollection;
@@ -149,36 +150,35 @@ public class JettyWebServerFactoryCustomizer implements
 	private void customizeAccessLog(ConfigurableJettyWebServerFactory factory,
 			ServerProperties.Jetty.Accesslog properties) {
 		factory.addServerCustomizers((server) -> {
-			NCSARequestLog log = new NCSARequestLog();
+			RequestLogWriter logWriter = new RequestLogWriter();
+			String format = getLogFormat(properties);
+			CustomRequestLog log = new CustomRequestLog(logWriter, format);
+			if (!CollectionUtils.isEmpty(properties.getIgnorePaths())) {
+				log.setIgnorePaths(properties.getIgnorePaths().toArray(new String[0]));
+			}
 			if (properties.getFilename() != null) {
-				log.setFilename(properties.getFilename());
+				logWriter.setFilename(properties.getFilename());
 			}
 			if (properties.getFileDateFormat() != null) {
-				log.setFilenameDateFormat(properties.getFileDateFormat());
-			}
-			log.setRetainDays(properties.getRetentionPeriod());
-			log.setAppend(properties.isAppend());
-			log.setExtended(properties.isExtendedFormat());
-			if (properties.getDateFormat() != null) {
-				log.setLogDateFormat(properties.getDateFormat());
-			}
-			if (properties.getLocale() != null) {
-				log.setLogLocale(properties.getLocale());
-			}
-			if (properties.getTimeZone() != null) {
-				log.setLogTimeZone(properties.getTimeZone().getID());
-			}
-			log.setLogCookies(properties.isLogCookies());
-			log.setLogServer(properties.isLogServer());
-			log.setLogLatency(properties.isLogLatency());
-			log.setPreferProxiedForAddress(properties.isPreferProxiedForAddress());
-			if (!CollectionUtils.isEmpty(properties.getIgnorePaths())) {
-				log.setIgnorePaths(properties.getIgnorePaths().toArray(new String[0]));
+				logWriter.setFilenameDateFormat(properties.getFileDateFormat());
 			}
+			logWriter.setRetainDays(properties.getRetentionPeriod());
+			logWriter.setAppend(properties.isAppend());
 			server.setRequestLog(log);
 		});
 	}
 
+	private String getLogFormat(ServerProperties.Jetty.Accesslog properties) {
+		if (properties.getCustomFormat() != null) {
+			return properties.getCustomFormat();
+		}
+		else if (ServerProperties.Jetty.Accesslog.FORMAT.EXTENDED_NCSA
+				.equals(properties.getFormat())) {
+			return CustomRequestLog.EXTENDED_NCSA_FORMAT;
+		}
+		return CustomRequestLog.NCSA_FORMAT;
+	}
+
 	private static class MaxHttpHeaderSizeCustomizer implements JettyServerCustomizer {
 
 		private final int maxHttpHeaderSize;

spring-boot-project/spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json

@@ -29,6 +29,55 @@
         "level": "error"
       }
     },
+    {
+      "name": "server.jetty.accesslog.date-format",
+      "deprecation": {
+        "replacement": "server.jetty.accesslog.custom-format",
+        "level": "error"
+      }
+    },
+    {
+      "name": "server.jetty.accesslog.extended-format",
+      "deprecation": {
+        "replacement": "server.jetty.accesslog.format",
+        "level": "error"
+      }
+    },
+    {
+      "name": "server.jetty.accesslog.locale",
+      "deprecation": {
+        "replacement": "server.jetty.accesslog.custom-format",
+        "level": "error"
+      }
+    },
+    {
+      "name": "server.jetty.accesslog.log-cookies",
+      "deprecation": {
+        "replacement": "server.jetty.accesslog.custom-format",
+        "level": "error"
+      }
+    },
+    {
+      "name": "server.jetty.accesslog.log-latency",
+      "deprecation": {
+        "replacement": "server.jetty.accesslog.custom-format",
+        "level": "error"
+      }
+    },
+    {
+      "name": "server.jetty.accesslog.log-server",
+      "deprecation": {
+        "replacement": "server.jetty.accesslog.custom-format",
+        "level": "error"
+      }
+    },
+    {
+      "name": "server.jetty.accesslog.time-zone",
+      "deprecation": {
+        "replacement": "server.jetty.accesslog.custom-format",
+        "level": "error"
+      }
+    },
     {
       "name": "server.compression.enabled",
       "description": "Whether response compression is enabled.",

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java

@@ -231,7 +231,7 @@ public class ServerPropertiesTests {
 		map.put("server.jetty.accesslog.file-date-format", "yyyymmdd");
 		map.put("server.jetty.accesslog.retention-period", "4");
 		map.put("server.jetty.accesslog.append", "true");
-		map.put("server.jetty.accesslog.prefer-proxied-for-address", "true");
+		map.put("server.jetty.accesslog.custom-format", "{client}a - %u %t \"%r\" %s %O");
 		map.put("server.jetty.accesslog.ignore-paths", "/a/path,/b/path");
 		bind(map);
 		ServerProperties.Jetty jetty = this.properties.getJetty();
@@ -240,7 +240,8 @@ public class ServerPropertiesTests {
 		assertThat(jetty.getAccesslog().getFileDateFormat()).isEqualTo("yyyymmdd");
 		assertThat(jetty.getAccesslog().getRetentionPeriod()).isEqualTo(4);
 		assertThat(jetty.getAccesslog().isAppend()).isTrue();
-		assertThat(jetty.getAccesslog().isPreferProxiedForAddress()).isTrue();
+		assertThat(jetty.getAccesslog().getCustomFormat())
+				.isEqualTo("{client}a - %u %t \"%r\" %s %O");
 		assertThat(jetty.getAccesslog().getIgnorePaths()).containsExactly("/a/path",
 				"/b/path");
 	}

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizerTests.java

@@ -20,14 +20,14 @@ import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Locale;
 import java.util.TimeZone;
 
 import org.eclipse.jetty.server.Connector;
+import org.eclipse.jetty.server.CustomRequestLog;
 import org.eclipse.jetty.server.HttpConfiguration;
 import org.eclipse.jetty.server.HttpConfiguration.ConnectionFactory;
-import org.eclipse.jetty.server.NCSARequestLog;
 import org.eclipse.jetty.server.RequestLog;
+import org.eclipse.jetty.server.RequestLogWriter;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -90,57 +90,48 @@ public class JettyWebServerFactoryCustomizerTests {
 		File logFile = File.createTempFile("jetty_log", ".log");
 		String timezone = TimeZone.getDefault().getID();
 		bind("server.jetty.accesslog.enabled=true",
+				"server.jetty.accesslog.format=extended_ncsa",
 				"server.jetty.accesslog.filename="
 						+ logFile.getAbsolutePath().replace("\\", "\\\\"),
 				"server.jetty.accesslog.file-date-format=yyyy-MM-dd",
 				"server.jetty.accesslog.retention-period=42",
 				"server.jetty.accesslog.append=true",
-				"server.jetty.accesslog.extended-format=true",
-				"server.jetty.accesslog.date-format=HH:mm:ss",
-				"server.jetty.accesslog.locale=en_BE",
-				"server.jetty.accesslog.time-zone=" + timezone,
-				"server.jetty.accesslog.log-cookies=true",
-				"server.jetty.accesslog.log-server=true",
-				"server.jetty.accesslog.log-latency=true",
-				"server.jetty.accesslog.prefer-proxied-for-address=true",
 				"server.jetty.accesslog.ignore-paths=/a/path,/b/path");
 		JettyWebServer server = customizeAndGetServer();
-		NCSARequestLog requestLog = getNCSARequestLog(server);
-		assertThat(requestLog.getFilename()).isEqualTo(logFile.getAbsolutePath());
-		assertThat(requestLog.getFilenameDateFormat()).isEqualTo("yyyy-MM-dd");
-		assertThat(requestLog.getRetainDays()).isEqualTo(42);
-		assertThat(requestLog.isAppend()).isTrue();
-		assertThat(requestLog.isExtended()).isTrue();
-		assertThat(requestLog.getLogDateFormat()).isEqualTo("HH:mm:ss");
-		assertThat(requestLog.getLogLocale()).isEqualTo(new Locale("en", "BE"));
-		assertThat(requestLog.getLogTimeZone()).isEqualTo(timezone);
-		assertThat(requestLog.getLogCookies()).isTrue();
-		assertThat(requestLog.getLogServer()).isTrue();
-		assertThat(requestLog.getLogLatency()).isTrue();
-		assertThat(requestLog.getPreferProxiedForAddress()).isTrue();
+		CustomRequestLog requestLog = getRequestLog(server);
+		assertThat(requestLog.getFormatString())
+				.isEqualTo(CustomRequestLog.EXTENDED_NCSA_FORMAT);
 		assertThat(requestLog.getIgnorePaths().length).isEqualTo(2);
 		assertThat(requestLog.getIgnorePaths()).containsExactly("/a/path", "/b/path");
+		RequestLogWriter logWriter = getLogWriter(requestLog);
+		assertThat(logWriter.getFileName()).isEqualTo(logFile.getAbsolutePath());
+		assertThat(logWriter.getFilenameDateFormat()).isEqualTo("yyyy-MM-dd");
+		assertThat(logWriter.getRetainDays()).isEqualTo(42);
+		assertThat(logWriter.isAppend()).isTrue();
 	}
 
 	@Test
 	public void accessLogCanBeEnabled() {
 		bind("server.jetty.accesslog.enabled=true");
 		JettyWebServer server = customizeAndGetServer();
-		NCSARequestLog requestLog = getNCSARequestLog(server);
-		assertThat(requestLog.getFilename()).isNull();
-		assertThat(requestLog.isAppend()).isFalse();
-		assertThat(requestLog.isExtended()).isFalse();
-		assertThat(requestLog.getLogCookies()).isFalse();
-		assertThat(requestLog.getLogServer()).isFalse();
-		assertThat(requestLog.getLogLatency()).isFalse();
-		assertThat(requestLog.getPreferProxiedForAddress()).isFalse();
+		CustomRequestLog requestLog = getRequestLog(server);
+		assertThat(requestLog.getFormatString()).isEqualTo(CustomRequestLog.NCSA_FORMAT);
 		assertThat(requestLog.getIgnorePaths()).isNull();
+		RequestLogWriter logWriter = getLogWriter(requestLog);
+		assertThat(logWriter.getFileName()).isNull();
+		assertThat(logWriter.isAppend()).isFalse();
 	}
 
-	private NCSARequestLog getNCSARequestLog(JettyWebServer server) {
+	private CustomRequestLog getRequestLog(JettyWebServer server) {
 		RequestLog requestLog = server.getServer().getRequestLog();
-		assertThat(requestLog).isInstanceOf(NCSARequestLog.class);
-		return (NCSARequestLog) requestLog;
+		assertThat(requestLog).isInstanceOf(CustomRequestLog.class);
+		return (CustomRequestLog) requestLog;
+	}
+
+	private RequestLogWriter getLogWriter(CustomRequestLog requestLog) {
+		RequestLog.Writer writer = requestLog.getWriter();
+		assertThat(writer).isInstanceOf(RequestLogWriter.class);
+		return (RequestLogWriter) requestLog.getWriter();
 	}
 
 	@Test