garrettmills/commafeed
1
0
Fork 0
forked from Archives/Athou_commafeed
Code Pull Requests Activity

configurable http client timeouts

Browse Source
This commit is contained in:
Athou
2024-08-16 21:12:54 +02:00
parent 1b1a3f49c1
commit 012ce71134
10 changed files with 118 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files

65
commafeed-server/src/main/java/com/commafeed/CommaFeedConfiguration.java
View File

@@ -56,6 +56,11 @@ public interface CommaFeedConfiguration {
*/
Optional<String> googleAuthKey();
/**
* HTTP client configuration
*/
HttpClient httpClient();
/**
* Feed refresh engine settings.
*/
@@ -76,6 +81,55 @@ public interface CommaFeedConfiguration {
*/
Websocket websocket();
interface HttpClient {
/**
* User-Agent string that will be used by the http client, leave empty for the default one.
*/
Optional<String> userAgent();
/**
* Time to wait for a connection to be established.
*/
@WithDefault("5s")
Duration connectTimeout();
/**
* Time to wait for SSL handshake to complete.
*/
@WithDefault("5s")
Duration sslHandshakeTimeout();
/**
* Time to wait between two packets before timeout.
*/
@WithDefault("10s")
Duration socketTimeout();
/**
* Time to wait for the full response to be received.
*/
@WithDefault("10s")
Duration responseTimeout();
/**
* Time to live for a connection in the pool.
*/
@WithDefault("30s")
Duration connectionTimeToLive();
/**
* Time between eviction runs for idle connections.
*/
@WithDefault("1m")
Duration idleConnectionsEvictionInterval();
/**
* If a feed is larger than this, it will be discarded to prevent memory issues while parsing the feed.
*/
@WithDefault("5M")
MemorySize maxResponseSize();
}
interface FeedRefresh {
/**
* Amount of time CommaFeed will wait before refreshing the same feed.
@@ -104,12 +158,6 @@ public interface CommaFeedConfiguration {
@WithDefault("1")
int databaseThreads();
/**
* If a feed is larger than this, it will be discarded to prevent memory issues while parsing the feed.
*/
@WithDefault("5M")
MemorySize maxResponseSize();
/**
* Duration after which a user is considered inactive. Feeds for inactive users are not refreshed until they log in again.
*
@@ -117,11 +165,6 @@ public interface CommaFeedConfiguration {
*/
@WithDefault("0")
Duration userInactivityPeriod();
/**
* User-Agent string that will be used by the http client, leave empty for the default one.
*/
Optional<String> userAgent();
}
interface Database {

49
commafeed-server/src/main/java/com/commafeed/backend/HttpGetter.java
View File

@@ -3,10 +3,10 @@ package com.commafeed.backend;
import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import org.apache.commons.lang3.StringUtils;
import org.apache.hc.client5.http.config.ConnectionConfig;
@@ -36,7 +36,6 @@ import com.google.common.collect.Iterables;
import com.google.common.io.ByteStreams;
import com.google.common.net.HttpHeaders;
import io.quarkus.runtime.configuration.MemorySize;
import jakarta.inject.Singleton;
import lombok.Getter;
import lombok.RequiredArgsConstructor;
@@ -51,16 +50,18 @@ import nl.altindag.ssl.apache5.util.Apache5SslUtils;
@Slf4j
public class HttpGetter {
private final CommaFeedConfiguration config;
private final CloseableHttpClient client;
private final MemorySize maxResponseSize;
public HttpGetter(CommaFeedConfiguration config, CommaFeedVersion version, MetricRegistry metrics) {
PoolingHttpClientConnectionManager connectionManager = newConnectionManager(config.feedRefresh().httpThreads());
String userAgent = config.feedRefresh()
this.config = config;
PoolingHttpClientConnectionManager connectionManager = newConnectionManager(config);
String userAgent = config.httpClient()
.userAgent()
.orElseGet(() -> String.format("CommaFeed/%s (https://github.com/Athou/commafeed)", version.getVersion()));
this.client = newClient(connectionManager, userAgent);
this.maxResponseSize = config.feedRefresh().maxResponseSize();
this.client = newClient(connectionManager, userAgent, config.httpClient().idleConnectionsEvictionInterval());
metrics.registerGauge(MetricRegistry.name(getClass(), "pool", "max"), () -> connectionManager.getTotalStats().getMax());
metrics.registerGauge(MetricRegistry.name(getClass(), "pool", "size"),
@@ -69,8 +70,8 @@ public class HttpGetter {
metrics.registerGauge(MetricRegistry.name(getClass(), "pool", "pending"), () -> connectionManager.getTotalStats().getPending());
}
public HttpResult getBinary(String url, int timeout) throws IOException, NotModifiedException {
return getBinary(url, null, null, timeout);
public HttpResult getBinary(String url) throws IOException, NotModifiedException {
return getBinary(url, null, null);
}
/**
@@ -83,10 +84,9 @@ public class HttpGetter {
* @throws NotModifiedException
* if the url hasn't changed since we asked for it last time
*/
public HttpResult getBinary(String url, String lastModified, String eTag, int timeout) throws IOException, NotModifiedException {
public HttpResult getBinary(String url, String lastModified, String eTag) throws IOException, NotModifiedException {
log.debug("fetching {}", url);
long start = System.currentTimeMillis();
ClassicHttpRequest request = ClassicRequestBuilder.get(url).build();
if (lastModified != null) {
request.addHeader(HttpHeaders.IF_MODIFIED_SINCE, lastModified);
@@ -96,10 +96,10 @@ public class HttpGetter {
}
HttpClientContext context = HttpClientContext.create();
context.setRequestConfig(RequestConfig.custom().setResponseTimeout(timeout, TimeUnit.MILLISECONDS).build());
context.setRequestConfig(RequestConfig.custom().setResponseTimeout(Timeout.of(config.httpClient().responseTimeout())).build());
HttpResponse response = client.execute(request, context, resp -> {
byte[] content = resp.getEntity() == null ? null : toByteArray(resp.getEntity(), maxResponseSize.asLongValue());
byte[] content = resp.getEntity() == null ? null
: toByteArray(resp.getEntity(), config.httpClient().maxResponseSize().asLongValue());
int code = resp.getCode();
String lastModifiedHeader = Optional.ofNullable(resp.getFirstHeader(HttpHeaders.LAST_MODIFIED))
.map(NameValuePair::getValue)
@@ -137,8 +137,7 @@ public class HttpGetter {
throw new NotModifiedException("eTagHeader is the same");
}
long duration = System.currentTimeMillis() - start;
return new HttpResult(response.getContent(), response.getContentType(), lastModifiedHeader, eTagHeader, duration,
return new HttpResult(response.getContent(), response.getContentType(), lastModifiedHeader, eTagHeader,
response.getUrlAfterRedirect());
}
@@ -161,21 +160,26 @@ public class HttpGetter {
}
}
private static PoolingHttpClientConnectionManager newConnectionManager(int poolSize) {
private static PoolingHttpClientConnectionManager newConnectionManager(CommaFeedConfiguration config) {
SSLFactory sslFactory = SSLFactory.builder().withUnsafeTrustMaterial().withUnsafeHostnameVerifier().build();
int poolSize = config.feedRefresh().httpThreads();
return PoolingHttpClientConnectionManagerBuilder.create()
.setSSLSocketFactory(Apache5SslUtils.toSocketFactory(sslFactory))
.setDefaultConnectionConfig(
ConnectionConfig.custom().setConnectTimeout(Timeout.ofSeconds(5)).setTimeToLive(TimeValue.ofSeconds(30)).build())
.setDefaultTlsConfig(TlsConfig.custom().setHandshakeTimeout(Timeout.ofSeconds(5)).build())
.setDefaultConnectionConfig(ConnectionConfig.custom()
.setConnectTimeout(Timeout.of(config.httpClient().connectTimeout()))
.setSocketTimeout(Timeout.of(config.httpClient().socketTimeout()))
.setTimeToLive(Timeout.of(config.httpClient().connectionTimeToLive()))
.build())
.setDefaultTlsConfig(TlsConfig.custom().setHandshakeTimeout(Timeout.of(config.httpClient().sslHandshakeTimeout())).build())
.setMaxConnPerRoute(poolSize)
.setMaxConnTotal(poolSize)
.build();
}
private static CloseableHttpClient newClient(HttpClientConnectionManager connectionManager, String userAgent) {
private static CloseableHttpClient newClient(HttpClientConnectionManager connectionManager, String userAgent,
Duration idleConnectionsEvictionInterval) {
List<Header> headers = new ArrayList<>();
headers.add(new BasicHeader(HttpHeaders.ACCEPT_LANGUAGE, "en"));
headers.add(new BasicHeader(HttpHeaders.PRAGMA, "No-cache"));
@@ -189,7 +193,7 @@ public class HttpGetter {
.setDefaultHeaders(headers)
.setConnectionManager(connectionManager)
.evictExpiredConnections()
.evictIdleConnections(TimeValue.ofMinutes(1))
.evictIdleConnections(TimeValue.of(idleConnectionsEvictionInterval))
.build();
}
@@ -249,7 +253,6 @@ public class HttpGetter {
private final String contentType;
private final String lastModifiedSince;
private final String eTag;
private final long duration;
private final String urlAfterRedirect;
}

2
commafeed-server/src/main/java/com/commafeed/backend/favicon/AbstractFaviconFetcher.java
View File

@@ -12,8 +12,6 @@ import lombok.extern.slf4j.Slf4j;
@Slf4j
public abstract class AbstractFaviconFetcher {
protected static final int TIMEOUT = 4000;
private static final List<String> ICON_MIMETYPE_BLACKLIST = Arrays.asList("application/xml", "text/html");
private static final long MIN_ICON_LENGTH = 100;
private static final long MAX_ICON_LENGTH = 100000;

6
commafeed-server/src/main/java/com/commafeed/backend/favicon/DefaultFaviconFetcher.java
View File

@@ -69,7 +69,7 @@ public class DefaultFaviconFetcher extends AbstractFaviconFetcher {
try {
url = FeedUtils.removeTrailingSlash(url) + "/favicon.ico";
log.debug("getting root icon at {}", url);
HttpResult result = getter.getBinary(url, TIMEOUT);
HttpResult result = getter.getBinary(url);
bytes = result.getContent();
contentType = result.getContentType();
} catch (Exception e) {
@@ -87,7 +87,7 @@ public class DefaultFaviconFetcher extends AbstractFaviconFetcher {
Document doc;
try {
HttpResult result = getter.getBinary(url, TIMEOUT);
HttpResult result = getter.getBinary(url);
doc = Jsoup.parse(new String(result.getContent()), url);
} catch (Exception e) {
log.debug("Failed to retrieve page to find icon");
@@ -113,7 +113,7 @@ public class DefaultFaviconFetcher extends AbstractFaviconFetcher {
byte[] bytes;
String contentType;
try {
HttpResult result = getter.getBinary(href, TIMEOUT);
HttpResult result = getter.getBinary(href);
bytes = result.getContent();
contentType = result.getContentType();
} catch (Exception e) {

2
commafeed-server/src/main/java/com/commafeed/backend/favicon/FacebookFaviconFetcher.java
View File

@@ -43,7 +43,7 @@ public class FacebookFaviconFetcher extends AbstractFaviconFetcher {
try {
log.debug("Getting Facebook user's icon, {}", url);
HttpResult iconResult = getter.getBinary(iconUrl, TIMEOUT);
HttpResult iconResult = getter.getBinary(iconUrl);
bytes = iconResult.getContent();
contentType = iconResult.getContentType();
} catch (Exception e) {

2
commafeed-server/src/main/java/com/commafeed/backend/favicon/YoutubeFaviconFetcher.java
View File

@@ -80,7 +80,7 @@ public class YoutubeFaviconFetcher extends AbstractFaviconFetcher {
Thumbnail thumbnail = channel.getSnippet().getThumbnails().getDefault();
log.debug("fetching favicon");
HttpResult iconResult = getter.getBinary(thumbnail.getUrl(), TIMEOUT);
HttpResult iconResult = getter.getBinary(thumbnail.getUrl());
bytes = iconResult.getContent();
contentType = iconResult.getContentType();
} catch (Exception e) {

11
commafeed-server/src/main/java/com/commafeed/backend/feed/FeedFetcher.java
View File

@@ -40,9 +40,7 @@ public class FeedFetcher {
Instant lastPublishedDate, String lastContentHash) throws FeedException, IOException, NotModifiedException {
log.debug("Fetching feed {}", feedUrl);
int timeout = 20000;
HttpResult result = getter.getBinary(feedUrl, lastModified, eTag, timeout);
HttpResult result = getter.getBinary(feedUrl, lastModified, eTag);
byte[] content = result.getContent();
FeedParserResult parserResult;
@@ -54,7 +52,7 @@ public class FeedFetcher {
if (org.apache.commons.lang3.StringUtils.isNotBlank(extractedUrl)) {
feedUrl = extractedUrl;
result = getter.getBinary(extractedUrl, lastModified, eTag, timeout);
result = getter.getBinary(extractedUrl, lastModified, eTag);
content = result.getContent();
parserResult = parser.parse(result.getUrlAfterRedirect(), content);
} else {
@@ -87,8 +85,7 @@ public class FeedFetcher {
etagHeaderValueChanged ? result.getETag() : null);
}
return new FeedFetcherResult(parserResult, result.getUrlAfterRedirect(), result.getLastModifiedSince(), result.getETag(), hash,
result.getDuration());
return new FeedFetcherResult(parserResult, result.getUrlAfterRedirect(), result.getLastModifiedSince(), result.getETag(), hash);
}
private static String extractFeedUrl(List<FeedURLProvider> urlProviders, String url, String urlContent) {
@@ -103,7 +100,7 @@ public class FeedFetcher {
}
public record FeedFetcherResult(FeedParserResult feed, String urlAfterRedirect, String lastModifiedHeader, String lastETagHeader,
String contentHash, long fetchDuration) {
String contentHash) {
}
}

2
commafeed-server/src/main/java/com/commafeed/frontend/resource/ServerREST.java
View File

@@ -77,7 +77,7 @@ public class ServerREST {
url = FeedUtils.imageProxyDecoder(url);
try {
HttpResult result = httpGetter.getBinary(url, 20000);
HttpResult result = httpGetter.getBinary(url);
return Response.ok(result.getContent()).build();
} catch (Exception e) {
return Response.status(Status.SERVICE_UNAVAILABLE).entity(e.getMessage()).build();

48
commafeed-server/src/test/java/com/commafeed/backend/HttpGetterTest.java
View File

@@ -3,6 +3,7 @@ package com.commafeed.backend;
import java.io.IOException;
import java.math.BigInteger;
import java.net.SocketTimeoutException;
import java.time.Duration;
import java.util.Arrays;
import java.util.Objects;
import java.util.Optional;
@@ -39,11 +40,11 @@ import io.quarkus.runtime.configuration.MemorySize;
@ExtendWith(MockServerExtension.class)
class HttpGetterTest {
private static final int TIMEOUT = 10000;
private MockServerClient mockServerClient;
private String feedUrl;
private byte[] feedContent;
private CommaFeedConfiguration config;
private HttpGetter getter;
@BeforeEach
@@ -53,10 +54,15 @@ class HttpGetterTest {
this.feedUrl = "http://localhost:" + this.mockServerClient.getPort() + "/";
this.feedContent = IOUtils.toByteArray(Objects.requireNonNull(getClass().getResource("/feed/rss.xml")));
CommaFeedConfiguration config = Mockito.mock(CommaFeedConfiguration.class, Mockito.RETURNS_DEEP_STUBS);
Mockito.when(config.feedRefresh().userAgent()).thenReturn(Optional.of("http-getter-test"));
this.config = Mockito.mock(CommaFeedConfiguration.class, Mockito.RETURNS_DEEP_STUBS);
Mockito.when(config.httpClient().userAgent()).thenReturn(Optional.of("http-getter-test"));
Mockito.when(config.httpClient().connectTimeout()).thenReturn(Duration.ofMillis(300));
Mockito.when(config.httpClient().sslHandshakeTimeout()).thenReturn(Duration.ofSeconds(5));
Mockito.when(config.httpClient().socketTimeout()).thenReturn(Duration.ofMillis(300));
Mockito.when(config.httpClient().responseTimeout()).thenReturn(Duration.ofMillis(300));
Mockito.when(config.httpClient().connectionTimeToLive()).thenReturn(Duration.ofSeconds(30));
Mockito.when(config.httpClient().maxResponseSize()).thenReturn(new MemorySize(new BigInteger("10000")));
Mockito.when(config.feedRefresh().httpThreads()).thenReturn(3);
Mockito.when(config.feedRefresh().maxResponseSize()).thenReturn(new MemorySize(new BigInteger("10000")));
this.getter = new HttpGetter(config, Mockito.mock(CommaFeedVersion.class), Mockito.mock(MetricRegistry.class));
}
@@ -67,7 +73,7 @@ class HttpGetterTest {
void errorCodes(int code) {
this.mockServerClient.when(HttpRequest.request().withMethod("GET")).respond(HttpResponse.response().withStatusCode(code));
HttpResponseException e = Assertions.assertThrows(HttpResponseException.class, () -> getter.getBinary(this.feedUrl, TIMEOUT));
HttpResponseException e = Assertions.assertThrows(HttpResponseException.class, () -> getter.getBinary(this.feedUrl));
Assertions.assertEquals(code, e.getCode());
}
@@ -80,12 +86,11 @@ class HttpGetterTest {
.withHeader(HttpHeaders.LAST_MODIFIED, "123456")
.withHeader(HttpHeaders.ETAG, "78910"));
HttpResult result = getter.getBinary(this.feedUrl, TIMEOUT);
HttpResult result = getter.getBinary(this.feedUrl);
Assertions.assertArrayEquals(feedContent, result.getContent());
Assertions.assertEquals(MediaType.APPLICATION_ATOM_XML.toString(), result.getContentType());
Assertions.assertEquals("123456", result.getLastModifiedSince());
Assertions.assertEquals("78910", result.getETag());
Assertions.assertTrue(result.getDuration() >= 0);
Assertions.assertEquals(this.feedUrl, result.getUrlAfterRedirect());
}
@@ -110,24 +115,23 @@ class HttpGetterTest {
this.mockServerClient.when(HttpRequest.request().withMethod("GET").withPath("/redirected-2"))
.respond(HttpResponse.response().withBody(feedContent).withContentType(MediaType.APPLICATION_ATOM_XML));
HttpResult result = getter.getBinary(this.feedUrl, TIMEOUT);
HttpResult result = getter.getBinary(this.feedUrl);
Assertions.assertEquals("http://localhost:" + this.mockServerClient.getPort() + "/redirected-2", result.getUrlAfterRedirect());
}
@Test
void dataTimeout() {
int smallTimeout = 500;
this.mockServerClient.when(HttpRequest.request().withMethod("GET"))
.respond(HttpResponse.response().withDelay(Delay.milliseconds(smallTimeout * 2)));
.respond(HttpResponse.response().withDelay(Delay.milliseconds(1000)));
Assertions.assertThrows(SocketTimeoutException.class, () -> getter.getBinary(this.feedUrl, smallTimeout));
Assertions.assertThrows(SocketTimeoutException.class, () -> getter.getBinary(this.feedUrl));
}
@Test
void connectTimeout() {
// try to connect to a non-routable address
// https://stackoverflow.com/a/904609
Assertions.assertThrows(ConnectTimeoutException.class, () -> getter.getBinary("http://10.255.255.1", 500));
Assertions.assertThrows(ConnectTimeoutException.class, () -> getter.getBinary("http://10.255.255.1"));
}
@Test
@@ -135,7 +139,7 @@ class HttpGetterTest {
this.mockServerClient.when(HttpRequest.request().withMethod("GET").withHeader(HttpHeaders.USER_AGENT, "http-getter-test"))
.respond(HttpResponse.response().withBody("ok"));
HttpResult result = getter.getBinary(this.feedUrl, TIMEOUT);
HttpResult result = getter.getBinary(this.feedUrl);
Assertions.assertEquals("ok", new String(result.getContent()));
}
@@ -144,7 +148,7 @@ class HttpGetterTest {
this.mockServerClient.when(HttpRequest.request().withMethod("GET").withHeader(HttpHeaders.IF_MODIFIED_SINCE, "123456"))
.respond(HttpResponse.response().withStatusCode(HttpStatus.SC_NOT_MODIFIED));
Assertions.assertThrows(NotModifiedException.class, () -> getter.getBinary(this.feedUrl, "123456", null, TIMEOUT));
Assertions.assertThrows(NotModifiedException.class, () -> getter.getBinary(this.feedUrl, "123456", null));
}
@Test
@@ -152,7 +156,7 @@ class HttpGetterTest {
this.mockServerClient.when(HttpRequest.request().withMethod("GET").withHeader(HttpHeaders.IF_NONE_MATCH, "78910"))
.respond(HttpResponse.response().withStatusCode(HttpStatus.SC_NOT_MODIFIED));
Assertions.assertThrows(NotModifiedException.class, () -> getter.getBinary(this.feedUrl, null, "78910", TIMEOUT));
Assertions.assertThrows(NotModifiedException.class, () -> getter.getBinary(this.feedUrl, null, "78910"));
}
@Test
@@ -169,8 +173,8 @@ class HttpGetterTest {
return HttpResponse.response().withBody("ok").withHeader(HttpHeaders.SET_COOKIE, "foo=bar");
});
Assertions.assertDoesNotThrow(() -> getter.getBinary(this.feedUrl, TIMEOUT));
Assertions.assertDoesNotThrow(() -> getter.getBinary(this.feedUrl, TIMEOUT));
Assertions.assertDoesNotThrow(() -> getter.getBinary(this.feedUrl));
Assertions.assertDoesNotThrow(() -> getter.getBinary(this.feedUrl));
Assertions.assertEquals(2, calls.get());
}
@@ -188,7 +192,7 @@ class HttpGetterTest {
return HttpResponse.response().withBody("ok");
});
Assertions.assertDoesNotThrow(() -> getter.getBinary(this.feedUrl, TIMEOUT));
Assertions.assertDoesNotThrow(() -> getter.getBinary(this.feedUrl));
}
@Test
@@ -197,7 +201,7 @@ class HttpGetterTest {
Arrays.fill(bytes, (byte) 1);
this.mockServerClient.when(HttpRequest.request().withMethod("GET")).respond(HttpResponse.response().withBody(bytes));
IOException e = Assertions.assertThrows(IOException.class, () -> getter.getBinary(this.feedUrl, TIMEOUT));
IOException e = Assertions.assertThrows(IOException.class, () -> getter.getBinary(this.feedUrl));
Assertions.assertEquals("Response size (100000 bytes) exceeds the maximum allowed size (10000 bytes)", e.getMessage());
}
@@ -210,7 +214,7 @@ class HttpGetterTest {
.withBody(bytes)
.withConnectionOptions(ConnectionOptions.connectionOptions().withSuppressContentLengthHeader(true)));
IOException e = Assertions.assertThrows(IOException.class, () -> getter.getBinary(this.feedUrl, TIMEOUT));
IOException e = Assertions.assertThrows(IOException.class, () -> getter.getBinary(this.feedUrl));
Assertions.assertEquals("Response size exceeds the maximum allowed size (10000 bytes)", e.getMessage());
}
@@ -218,7 +222,7 @@ class HttpGetterTest {
void ignoreInvalidSsl() throws Exception {
this.mockServerClient.when(HttpRequest.request().withMethod("GET")).respond(HttpResponse.response().withBody("ok"));
HttpResult result = getter.getBinary("https://localhost:" + this.mockServerClient.getPort(), TIMEOUT);
HttpResult result = getter.getBinary("https://localhost:" + this.mockServerClient.getPort());
Assertions.assertEquals("ok", new String(result.getContent()));
}

4
commafeed-server/src/test/java/com/commafeed/backend/feed/FeedFetcherTest.java
View File

@@ -45,8 +45,8 @@ class FeedFetcherTest {
byte[] content = "content".getBytes();
String lastContentHash = Hashing.sha1().hashBytes(content).toString();
Mockito.when(getter.getBinary(url, lastModified, etag, 20000))
.thenReturn(new HttpResult(content, "content-type", "last-modified-2", "etag-2", 20, null));
Mockito.when(getter.getBinary(url, lastModified, etag))
.thenReturn(new HttpResult(content, "content-type", "last-modified-2", "etag-2", null));
NotModifiedException e = Assertions.assertThrows(NotModifiedException.class,
() -> fetcher.fetch(url, false, lastModified, etag, Instant.now(), lastContentHash));