Skip to content

Commit

Permalink
Make connection errors during notifications silent (logged only) (#52)
Browse files Browse the repository at this point in the history
Also scrub apikey from any future log statements (not aware that it was
logged prior, but with this new change it would of been without the
scrub) + Test cases
  • Loading branch information
shayaantx authored Jan 24, 2022
1 parent 62e851a commit 4e4be4d
Show file tree
Hide file tree
Showing 9 changed files with 160 additions and 14 deletions.
12 changes: 12 additions & 0 deletions src/main/java/com/botdarr/api/DownloadsStrategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
import com.google.gson.JsonArray;
import com.google.gson.JsonElement;
import com.google.gson.JsonParser;
import org.apache.http.conn.HttpHostConnectException;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -38,6 +41,14 @@ public List<ChatClientResponse> downloads() {

public List<ChatClientResponse> getContentDownloads() {
return ConnectionHelper.makeGetRequest(this.api, this.url, new ConnectionHelper.SimpleMessageEmbedResponseHandler(chatClientResponseBuilder) {

@Override
public List<ChatClientResponse> onConnectException(HttpHostConnectException e) {
String message = "Error trying to connect to " + DownloadsStrategy.this.api.getApiUrl(DownloadsStrategy.this.url);
LOGGER.error(message);
return Collections.emptyList();
}

@Override
public List<ChatClientResponse> onSuccess(String response) {
return parseContent(response);
Expand Down Expand Up @@ -69,4 +80,5 @@ public List<ChatClientResponse> parseContent(String response) {
private final ChatClientResponseBuilder<? extends ChatClientResponse> chatClientResponseBuilder;
private final int MAX_DOWNLOADS_TO_SHOW = new ApiRequests().getMaxDownloadsToShow();
private final ContentType contentType;
private static Logger LOGGER = LogManager.getLogger();
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.botdarr.connections.ConnectionHelper;
import com.google.gson.Gson;
import org.apache.http.client.methods.*;
import org.apache.http.conn.HttpHostConnectException;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.apache.logging.log4j.LogManager;
Expand Down Expand Up @@ -291,6 +292,11 @@ public T onException(Exception e) {
LOGGER.error("Error trying to make matrix request", e);
return null;
}

@Override
public T onConnectException(HttpHostConnectException e) {
return onException(e);
}
}

protected static class Constants {
Expand Down
20 changes: 18 additions & 2 deletions src/main/java/com/botdarr/connections/ConnectionHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.botdarr.clients.ChatClientResponseBuilder;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.*;
import org.apache.http.conn.HttpHostConnectException;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.util.EntityUtils;
Expand Down Expand Up @@ -59,17 +60,25 @@ public static <T> T makeRequest(RequestHandler requestHandler, ResponseHandler<T
} else {
return responseHandler.onFailure(statusCode, response.getStatusLine().getReasonPhrase());
}
} catch (HttpHostConnectException e) {
LOGGER.error("Error trying to connect during request", e);
return responseHandler.onConnectException(e);
} catch (Exception e) {
LOGGER.error("Error trying to execute connection during post request", e);
LOGGER.error("Error trying to execute connection during request", e);
return responseHandler.onException(e);
}
} catch (IOException e) {
LOGGER.error("Error trying to make connection during post request", e);
LOGGER.error("Error trying to make connection during request", e);
return responseHandler.onException(e);
}
}

public static abstract class SimpleMessageEmbedResponseHandler implements ResponseHandler<List<ChatClientResponse>> {
@Override
public List<ChatClientResponse> onConnectException(HttpHostConnectException e) {
return onException(e);
}

public SimpleMessageEmbedResponseHandler(ChatClientResponseBuilder<? extends ChatClientResponse> chatClientResponseBuilder) {
this.chatClientResponseBuilder = chatClientResponseBuilder;
}
Expand Down Expand Up @@ -97,6 +106,11 @@ public T onFailure(int statusCode, String reason) {
public T onException(Exception e) {
return null;
}

@Override
public T onConnectException(HttpHostConnectException e) {
return null;
}
}

public interface RequestHandler {
Expand All @@ -112,6 +126,8 @@ public interface ResponseHandler<T> {
T onFailure(int statusCode, String reason);

T onException(Exception e);

T onConnectException(HttpHostConnectException e);
}

private static final Logger LOGGER = LogManager.getLogger();
Expand Down
42 changes: 42 additions & 0 deletions src/main/java/com/botdarr/utilities/Log4jSensitiveDataPattern.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.botdarr.utilities;

import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.config.plugins.Plugin;
import org.apache.logging.log4j.core.pattern.ConverterKeys;
import org.apache.logging.log4j.core.pattern.LogEventPatternConverter;

import java.util.regex.Matcher;
import java.util.regex.Pattern;

@Plugin(name="LogMaskingConverter", category = "Converter")
@ConverterKeys({"scrubbedMsg"})
public class Log4jSensitiveDataPattern extends LogEventPatternConverter {
private static final Pattern API_KEY_PATTERN = Pattern.compile("apikey=([0-9a-zA-z]+)");

protected Log4jSensitiveDataPattern(String name, String style) {
super(name, style);
}
public static Log4jSensitiveDataPattern newInstance(String[] options) {
return new Log4jSensitiveDataPattern("scrubbedMsg", Thread.currentThread().getName());
}

@Override
public void format(LogEvent event, StringBuilder toAppendTo) {
String message = event.getMessage().getFormattedMessage();
String maskedMessage = message;
try {
StringBuffer modifiedBuffer = new StringBuffer();
Matcher matcher = API_KEY_PATTERN.matcher(message);
while(matcher.find()) {
matcher.appendReplacement(modifiedBuffer, "apikey=****");
}
if (modifiedBuffer.length() > 0) {
maskedMessage = modifiedBuffer.toString();
}
} catch (Exception e) {
System.out.println("Error trying to mask message, message = " + e.getMessage());
maskedMessage = message;
}
toAppendTo.append(maskedMessage);
}
}
20 changes: 10 additions & 10 deletions src/main/resources/log4j2.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
<Appenders>
<!-- Console Appender -->
<Console name="Console" target="SYSTEM_OUT">
<PatternLayout pattern="%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %msg%n" />
<PatternLayout pattern="%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %scrubbedMsg%n" />
</Console>
<RollingFile name="BotLog">
<FileName>logs/bot.log</FileName>
<FilePattern>logs/%d{yyyy-MM-dd-hh-mm}.log.zip</FilePattern>
<PatternLayout>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %msg%n</Pattern>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %scrubbedMsg%n</Pattern>
</PatternLayout>
<Policies>
<SizeBasedTriggeringPolicy size="10 MB"/>
Expand All @@ -21,7 +21,7 @@
<FileName>logs/audit.log</FileName>
<FilePattern>logs/%d{yyyy-MM-dd-hh-mm}.audit.zip</FilePattern>
<PatternLayout>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %msg%n</Pattern>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %scrubbedMsg%n</Pattern>
</PatternLayout>
<Policies>
<SizeBasedTriggeringPolicy size="10 MB"/>
Expand All @@ -33,7 +33,7 @@
<FileName>logs/radarr.log</FileName>
<FilePattern>logs/%d{yyyy-MM-dd-hh-mm}.log.zip</FilePattern>
<PatternLayout>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %msg%n</Pattern>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %scrubbedMsg%n</Pattern>
</PatternLayout>
<Policies>
<SizeBasedTriggeringPolicy size="10 MB"/>
Expand All @@ -45,7 +45,7 @@
<FileName>logs/sonarr.log</FileName>
<FilePattern>logs/%d{yyyy-MM-dd-hh-mm}.log.zip</FilePattern>
<PatternLayout>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %msg%n</Pattern>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %scrubbedMsg%n</Pattern>
</PatternLayout>
<Policies>
<SizeBasedTriggeringPolicy size="10 MB"/>
Expand All @@ -57,7 +57,7 @@
<FileName>logs/network.log</FileName>
<FilePattern>logs/%d{yyyy-MM-dd-hh-mm}.log.zip</FilePattern>
<PatternLayout>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %msg%n</Pattern>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %scrubbedMsg%n</Pattern>
</PatternLayout>
<Policies>
<SizeBasedTriggeringPolicy size="10 MB"/>
Expand All @@ -69,7 +69,7 @@
<FileName>logs/slack.log</FileName>
<FilePattern>logs/%d{yyyy-MM-dd-hh-mm}.log.zip</FilePattern>
<PatternLayout>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %msg%n</Pattern>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %scrubbedMsg%n</Pattern>
</PatternLayout>
<Policies>
<SizeBasedTriggeringPolicy size="10 MB"/>
Expand All @@ -81,7 +81,7 @@
<FileName>logs/discord.log</FileName>
<FilePattern>logs/%d{yyyy-MM-dd-hh-mm}.log.zip</FilePattern>
<PatternLayout>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %msg%n</Pattern>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %scrubbedMsg%n</Pattern>
</PatternLayout>
<Policies>
<SizeBasedTriggeringPolicy size="10 MB"/>
Expand All @@ -93,7 +93,7 @@
<FileName>logs/matrix.log</FileName>
<FilePattern>logs/%d{yyyy-MM-dd-hh-mm}.log.zip</FilePattern>
<PatternLayout>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %msg%n</Pattern>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %scrubbedMsg%n</Pattern>
</PatternLayout>
<Policies>
<SizeBasedTriggeringPolicy size="10 MB"/>
Expand All @@ -105,7 +105,7 @@
<FileName>logs/telegram.log</FileName>
<FilePattern>logs/%d{yyyy-MM-dd-hh-mm}.log.zip</FilePattern>
<PatternLayout>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %msg%n</Pattern>
<Pattern>%d{yyyy-MMM-dd HH:mm:ss a} [%t] %-5level %logger{36} - %scrubbedMsg%n</Pattern>
</PatternLayout>
<Policies>
<SizeBasedTriggeringPolicy size="10 MB"/>
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
5.1.14
5.1.15
32 changes: 32 additions & 0 deletions src/test/java/com/botdarr/api/DownloadsStrategyTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,18 @@
import com.botdarr.TestResponse;
import com.botdarr.clients.ChatClientResponse;
import com.botdarr.clients.ChatClientResponseBuilder;
import com.google.gson.Gson;
import com.google.gson.JsonElement;
import mockit.*;
import org.apache.logging.log4j.Logger;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Rule;
import org.junit.Test;
import org.mockserver.junit.MockServerRule;
import org.mockserver.model.HttpRequest;
import org.mockserver.model.HttpResponse;
import org.mockserver.model.MediaType;

import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -61,12 +68,37 @@ public void parseContent_tooManyDownloads_infoMessageIncluded() {
Assert.assertEquals(expectedInfoResponse, responses.get(0));
}

@Test
public void getContentDownloads_endpointUnavailable_emptyResponse() {
DownloadsStrategy downloadsStrategy = new DownloadsStrategy(api, "", chatClientResponseBuilder, ContentType.MOVIE) {
@Override
public ChatClientResponse getResponse(JsonElement rawElement) {
return null;
}
};
Deencapsulation.setField(downloadsStrategy, "LOGGER", logger);
new Expectations(downloadsStrategy) {{
api.getApiUrl(""); result = "http://localhost"; times = 2;
api.getApiToken(); result = "token1"; times = 1;
logger.error("Error trying to connect to http://localhost"); times = 1;
}};

//confirm even tho we failed to connect we don't return error notifications that could flood chat clients
Assert.assertTrue(downloadsStrategy.getContentDownloads().isEmpty());
}

private DownloadsStrategy getMockDownloadsStrategy(int maxDownloadsToShow) {
DownloadsStrategy mockDownloadsStrategy = new MockDownloadsStrategy(api, "", chatClientResponseBuilder, ContentType.MOVIE);
Deencapsulation.setField(mockDownloadsStrategy, "MAX_DOWNLOADS_TO_SHOW", maxDownloadsToShow);
return mockDownloadsStrategy;
}

@Rule
public MockServerRule mockServerRule = new MockServerRule(this);

@Mocked
private Logger logger;

@Injectable
private Api api;

Expand Down
1 change: 0 additions & 1 deletion src/test/java/com/botdarr/api/radarr/RadarrApiTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.botdarr.Config;
import com.botdarr.TestResponse;
import com.botdarr.TestResponseBuilder;
import com.botdarr.api.radarr.*;
import com.botdarr.commands.CommandResponse;
import com.google.gson.Gson;
import mockit.Deencapsulation;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.botdarr.utilities;

import mockit.Expectations;
import mockit.Mocked;
import org.apache.logging.log4j.core.impl.Log4jLogEvent;
import org.apache.logging.log4j.core.pattern.LogEventPatternConverter;
import org.junit.Assert;
import org.junit.Test;

public class Log4jSensitiveDataPatternTests {
@Test
public void format_noApiKeyInString_noMaskedText() {
String input = "http://localhost";
LogEventPatternConverter patternConverter = new Log4jSensitiveDataPattern("test", "style");
StringBuilder output = new StringBuilder();
new Expectations() {{
mockedEvent.getMessage().getFormattedMessage(); result = input;
}};
patternConverter.format(mockedEvent, output);
//input should remain unchanged
Assert.assertEquals(input, output.toString());
}

@Test
public void format_apiKeyInString_noMaskedText() {
String input = "http://localhost?apikey=fdskjkjfd";
LogEventPatternConverter patternConverter = new Log4jSensitiveDataPattern("test", "style");
StringBuilder output = new StringBuilder();
new Expectations() {{
mockedEvent.getMessage().getFormattedMessage(); result = input;
}};
patternConverter.format(mockedEvent, output);
//input should remain unchanged
Assert.assertEquals("http://localhost?apikey=****", output.toString());
}

@Mocked
private Log4jLogEvent mockedEvent;
}

0 comments on commit 4e4be4d

Please sign in to comment.