This is an automated email from the ASF dual-hosted git repository.
mattsicker pushed a commit to branch fix/3.x/port-3438
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 1b1b79e5d865e0e498144889ed16078ffed499d2
Author: Suvrat Acharya <140749446+suvrat1...@users.noreply.github.com>
AuthorDate: Sun Feb 16 13:59:16 2025 +0530
Improve configuration error handling of HttpAppender (#3438)
This PR introduces improvements to `HttpAppender` and adds a new test
class, `HttpAppenderBuilderTest`, to enhance test coverage.
The changes include:
* Updating `HttpAppender` to improve validating behavior.
* Adding HttpAppenderBuilderTest.java to verify the builder logic for
HttpAppender.
Ensuring that missing configurations (e.g., URL, Layout) correctly log
errors.
Co-authored-by: Piotr P. Karwasz <piotr.git...@karwasz.org>
---
.../core/appender/HttpAppenderBuilderTest.java | 130 +++++++++++++++++++++
.../logging/log4j/core/appender/HttpAppender.java | 24 ++--
2 files changed, 147 insertions(+), 7 deletions(-)
diff --git
a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/HttpAppenderBuilderTest.java
b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/HttpAppenderBuilderTest.java
new file mode 100644
index 0000000000..2680961d19
--- /dev/null
+++
b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/HttpAppenderBuilderTest.java
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.core.appender;
+
+import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.net.MalformedURLException;
+import java.net.URL;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.Layout;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.DefaultConfiguration;
+import org.apache.logging.log4j.core.config.Property;
+import org.apache.logging.log4j.core.layout.JsonLayout;
+import org.apache.logging.log4j.core.net.ssl.SslConfiguration;
+import org.apache.logging.log4j.test.ListStatusListener;
+import org.apache.logging.log4j.test.junit.UsingStatusListener;
+import org.junit.jupiter.api.Test;
+
+class HttpAppenderBuilderTest {
+
+ private HttpAppender.Builder<?> getBuilder() {
+ Configuration mockConfig = new DefaultConfiguration();
+ return
HttpAppender.newBuilder().setConfiguration(mockConfig).setName("TestHttpAppender");
// Name is required
+ }
+
+ @Test
+ @UsingStatusListener
+ void testBuilderWithoutUrl(final ListStatusListener listener) throws
Exception {
+ HttpAppender appender = HttpAppender.newBuilder()
+ .setConfiguration(new DefaultConfiguration())
+ .setName("TestAppender")
+ .setLayout(JsonLayout.createDefaultLayout()) // Providing a
layout here
+ .build();
+
+ assertThat(listener.findStatusData(Level.ERROR))
+ .anyMatch(statusData ->
+
statusData.getMessage().getFormattedMessage().contains("HttpAppender requires
URL to be set."));
+ }
+
+ @Test
+ @UsingStatusListener
+ void testBuilderWithUrlAndWithoutLayout(final ListStatusListener listener)
throws Exception {
+ HttpAppender appender = HttpAppender.newBuilder()
+ .setConfiguration(new DefaultConfiguration())
+ .setName("TestAppender")
+ .setUrl(new URL("http://localhost:8080/logs";))
+ .build();
+
+ assertThat(listener.findStatusData(Level.ERROR)).anyMatch(statusData
-> statusData
+ .getMessage()
+ .getFormattedMessage()
+ .contains("HttpAppender requires a layout to be set."));
+ }
+
+ @Test
+ void testBuilderWithValidConfiguration() throws Exception {
+ URL url = new URL("http://example.com";);
+ Layout<?> layout = JsonLayout.createDefaultLayout();
+
+ HttpAppender.Builder<?> builder =
getBuilder().setUrl(url).setLayout(layout);
+
+ HttpAppender appender = builder.build();
+ assertNotNull(appender, "HttpAppender should be created with valid
configuration.");
+ }
+
+ @Test
+ void testBuilderWithCustomMethod() throws Exception {
+ URL url = new URL("http://example.com";);
+ Layout<?> layout = JsonLayout.createDefaultLayout();
+ String customMethod = "PUT";
+
+ HttpAppender.Builder<?> builder =
+
getBuilder().setUrl(url).setLayout(layout).setMethod(customMethod);
+
+ HttpAppender appender = builder.build();
+ assertNotNull(appender, "HttpAppender should be created with a custom
HTTP method.");
+ }
+
+ @Test
+ void testBuilderWithHeaders() throws Exception {
+ URL url = new URL("http://example.com";);
+ Layout<?> layout = JsonLayout.createDefaultLayout();
+ Property[] headers = new Property[] {
+ Property.createProperty("Header1", "Value1"),
Property.createProperty("Header2", "Value2")
+ };
+
+ HttpAppender.Builder<?> builder =
+ getBuilder().setUrl(url).setLayout(layout).setHeaders(headers);
+
+ HttpAppender appender = builder.build();
+ assertNotNull(appender, "HttpAppender should be created with
headers.");
+ }
+
+ @Test
+ void testBuilderWithSslConfiguration() throws Exception {
+ URL url = new URL("https://example.com";);
+ Layout<?> layout = JsonLayout.createDefaultLayout();
+
+ // Use real SslConfiguration instead of Mockito mock
+ SslConfiguration sslConfig =
SslConfiguration.createSSLConfiguration(null, null, null, false);
+
+ HttpAppender.Builder<?> builder =
+
getBuilder().setUrl(url).setLayout(layout).setSslConfiguration(sslConfig);
+
+ HttpAppender appender = builder.build();
+ assertNotNull(appender, "HttpAppender should be created with SSL
configuration.");
+ }
+
+ @Test
+ void testBuilderWithInvalidUrl() {
+ assertThrows(MalformedURLException.class, () -> new
URL("invalid-url"));
+ }
+}
diff --git
a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/HttpAppender.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/HttpAppender.java
index e33de5cc66..b34b546f9d 100644
---
a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/HttpAppender.java
+++
b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/HttpAppender.java
@@ -31,17 +31,16 @@ import
org.apache.logging.log4j.plugins.PluginBuilderAttribute;
import org.apache.logging.log4j.plugins.PluginElement;
import org.apache.logging.log4j.plugins.PluginFactory;
import org.apache.logging.log4j.plugins.validation.constraints.Required;
+import org.apache.logging.log4j.status.StatusLogger;
-/**
- * Sends log events over HTTP.
- */
@Configurable(elementType = Appender.ELEMENT_TYPE, printObject = true)
@Plugin("Http")
public final class HttpAppender extends AbstractAppender {
+ private static final StatusLogger LOGGER = StatusLogger.getLogger();
+
/**
* Builds HttpAppender instances.
- * @param <B> The type to build
*/
public static class Builder<B extends Builder<B>> extends
AbstractAppender.Builder<B>
implements
org.apache.logging.log4j.plugins.util.Builder<HttpAppender> {
@@ -70,6 +69,18 @@ public final class HttpAppender extends AbstractAppender {
@Override
public HttpAppender build() {
+ // Validate URL presence
+ if (url == null) {
+ LOGGER.error("HttpAppender requires URL to be set.");
+ return null; // Return null if URL is missing
+ }
+
+ // Validate layout presence
+ if (getLayout() == null) {
+ LOGGER.error("HttpAppender requires a layout to be set.");
+ return null; // Return null if layout is missing
+ }
+
final HttpManager httpManager = new HttpURLConnectionManager(
getConfiguration(),
getConfiguration().getLoggerContext(),
@@ -81,10 +92,12 @@ public final class HttpAppender extends AbstractAppender {
headers,
sslConfiguration,
verifyHostname);
+
return new HttpAppender(
getName(), getLayout(), getFilter(), isIgnoreExceptions(),
httpManager, getPropertyArray());
}
+ // Getter and Setter methods
public URL getUrl() {
return url;
}
@@ -149,9 +162,6 @@ public final class HttpAppender extends AbstractAppender {
}
}
- /**
- * @return a builder for a HttpAppender.
- */
@PluginFactory
public static <B extends Builder<B>> B newBuilder() {
return new Builder<B>().asBuilder();
