Skip to content

Commit 8c0e3c6

Browse files
Suvrat1629ppkarwasz
Suvrat1629
and
ppkarwasz
authored
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 <[email protected]>
1 parent c59fdd4 commit 8c0e3c6

File tree

3 files changed

+155
-7
lines changed

3 files changed

+155
-7
lines changed

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/HttpAppenderBuilderTest.java

+130
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.core.appender;
18+
19+
import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
20+
import static org.junit.jupiter.api.Assertions.assertNotNull;
21+
import static org.junit.jupiter.api.Assertions.assertThrows;
22+
23+
import java.net.MalformedURLException;
24+
import java.net.URL;
25+
import org.apache.logging.log4j.Level;
26+
import org.apache.logging.log4j.core.Layout;
27+
import org.apache.logging.log4j.core.config.Configuration;
28+
import org.apache.logging.log4j.core.config.DefaultConfiguration;
29+
import org.apache.logging.log4j.core.config.Property;
30+
import org.apache.logging.log4j.core.layout.JsonLayout;
31+
import org.apache.logging.log4j.core.net.ssl.SslConfiguration;
32+
import org.apache.logging.log4j.test.ListStatusListener;
33+
import org.apache.logging.log4j.test.junit.UsingStatusListener;
34+
import org.junit.jupiter.api.Test;
35+
36+
class HttpAppenderBuilderTest {
37+
38+
private HttpAppender.Builder<?> getBuilder() {
39+
Configuration mockConfig = new DefaultConfiguration();
40+
return HttpAppender.newBuilder().setConfiguration(mockConfig).setName("TestHttpAppender"); // Name is required
41+
}
42+
43+
@Test
44+
@UsingStatusListener
45+
void testBuilderWithoutUrl(final ListStatusListener listener) throws Exception {
46+
HttpAppender appender = HttpAppender.newBuilder()
47+
.setConfiguration(new DefaultConfiguration())
48+
.setName("TestAppender")
49+
.setLayout(JsonLayout.createDefaultLayout()) // Providing a layout here
50+
.build();
51+
52+
assertThat(listener.findStatusData(Level.ERROR))
53+
.anyMatch(statusData ->
54+
statusData.getMessage().getFormattedMessage().contains("HttpAppender requires URL to be set."));
55+
}
56+
57+
@Test
58+
@UsingStatusListener
59+
void testBuilderWithUrlAndWithoutLayout(final ListStatusListener listener) throws Exception {
60+
HttpAppender appender = HttpAppender.newBuilder()
61+
.setConfiguration(new DefaultConfiguration())
62+
.setName("TestAppender")
63+
.setUrl(new URL("http://localhost:8080/logs"))
64+
.build();
65+
66+
assertThat(listener.findStatusData(Level.ERROR)).anyMatch(statusData -> statusData
67+
.getMessage()
68+
.getFormattedMessage()
69+
.contains("HttpAppender requires a layout to be set."));
70+
}
71+
72+
@Test
73+
void testBuilderWithValidConfiguration() throws Exception {
74+
URL url = new URL("http://example.com");
75+
Layout<?> layout = JsonLayout.createDefaultLayout();
76+
77+
HttpAppender.Builder<?> builder = getBuilder().setUrl(url).setLayout(layout);
78+
79+
HttpAppender appender = builder.build();
80+
assertNotNull(appender, "HttpAppender should be created with valid configuration.");
81+
}
82+
83+
@Test
84+
void testBuilderWithCustomMethod() throws Exception {
85+
URL url = new URL("http://example.com");
86+
Layout<?> layout = JsonLayout.createDefaultLayout();
87+
String customMethod = "PUT";
88+
89+
HttpAppender.Builder<?> builder =
90+
getBuilder().setUrl(url).setLayout(layout).setMethod(customMethod);
91+
92+
HttpAppender appender = builder.build();
93+
assertNotNull(appender, "HttpAppender should be created with a custom HTTP method.");
94+
}
95+
96+
@Test
97+
void testBuilderWithHeaders() throws Exception {
98+
URL url = new URL("http://example.com");
99+
Layout<?> layout = JsonLayout.createDefaultLayout();
100+
Property[] headers = new Property[] {
101+
Property.createProperty("Header1", "Value1"), Property.createProperty("Header2", "Value2")
102+
};
103+
104+
HttpAppender.Builder<?> builder =
105+
getBuilder().setUrl(url).setLayout(layout).setHeaders(headers);
106+
107+
HttpAppender appender = builder.build();
108+
assertNotNull(appender, "HttpAppender should be created with headers.");
109+
}
110+
111+
@Test
112+
void testBuilderWithSslConfiguration() throws Exception {
113+
URL url = new URL("https://example.com");
114+
Layout<?> layout = JsonLayout.createDefaultLayout();
115+
116+
// Use real SslConfiguration instead of Mockito mock
117+
SslConfiguration sslConfig = SslConfiguration.createSSLConfiguration(null, null, null, false);
118+
119+
HttpAppender.Builder<?> builder =
120+
getBuilder().setUrl(url).setLayout(layout).setSslConfiguration(sslConfig);
121+
122+
HttpAppender appender = builder.build();
123+
assertNotNull(appender, "HttpAppender should be created with SSL configuration.");
124+
}
125+
126+
@Test
127+
void testBuilderWithInvalidUrl() {
128+
assertThrows(MalformedURLException.class, () -> new URL("invalid-url"));
129+
}
130+
}

‎log4j-core/src/main/java/org/apache/logging/log4j/core/appender/HttpAppender.java

+17-7
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,15 @@
3232
import org.apache.logging.log4j.core.config.plugins.PluginElement;
3333
import org.apache.logging.log4j.core.config.plugins.validation.constraints.Required;
3434
import org.apache.logging.log4j.core.net.ssl.SslConfiguration;
35+
import org.apache.logging.log4j.status.StatusLogger;
3536

36-
/**
37-
* Sends log events over HTTP.
38-
*/
3937
@Plugin(name = "Http", category = Node.CATEGORY, elementType = Appender.ELEMENT_TYPE, printObject = true)
4038
public final class HttpAppender extends AbstractAppender {
4139

40+
private static final StatusLogger LOGGER = StatusLogger.getLogger();
41+
4242
/**
4343
* Builds HttpAppender instances.
44-
* @param <B> The type to build
4544
*/
4645
public static class Builder<B extends Builder<B>> extends AbstractAppender.Builder<B>
4746
implements org.apache.logging.log4j.core.util.Builder<HttpAppender> {
@@ -70,6 +69,18 @@ public static class Builder<B extends Builder<B>> extends AbstractAppender.Build
7069

7170
@Override
7271
public HttpAppender build() {
72+
// Validate URL presence
73+
if (url == null) {
74+
LOGGER.error("HttpAppender requires URL to be set.");
75+
return null; // Return null if URL is missing
76+
}
77+
78+
// Validate layout presence
79+
if (getLayout() == null) {
80+
LOGGER.error("HttpAppender requires a layout to be set.");
81+
return null; // Return null if layout is missing
82+
}
83+
7384
final HttpManager httpManager = new HttpURLConnectionManager(
7485
getConfiguration(),
7586
getConfiguration().getLoggerContext(),
@@ -81,10 +92,12 @@ public HttpAppender build() {
8192
headers,
8293
sslConfiguration,
8394
verifyHostname);
95+
8496
return new HttpAppender(
8597
getName(), getLayout(), getFilter(), isIgnoreExceptions(), httpManager, getPropertyArray());
8698
}
8799

100+
// Getter and Setter methods
88101
public URL getUrl() {
89102
return url;
90103
}
@@ -149,9 +162,6 @@ public B setVerifyHostname(final boolean verifyHostname) {
149162
}
150163
}
151164

152-
/**
153-
* @return a builder for a HttpAppender.
154-
*/
155165
@PluginBuilderFactory
156166
public static <B extends Builder<B>> B newBuilder() {
157167
return new Builder<B>().asBuilder();

‎src/changelog/.2.x.x/3011_http_appender_validation.xml

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
3+
xmlns="https://logging.apache.org/xml/ns"
4+
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
5+
type="fixed">
6+
<issue id="3011" link="https://github.com/apache/logging-log4j2/issues/3011"/>
7+
<description format="asciidoc">Improves validation of HTTP Appender.</description>
8+
</entry>

0 commit comments

Comments
 (0)