Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SITES-19780 - Embed component should have more aggressive timeouts #2723

Merged
merged 9 commits into from
Apr 19, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,6 @@ public class OEmbedClientImpl implements OEmbedClient {
@Reference
private HttpClientBuilderFactory httpClientBuilderFactory;

/**
* Socket timeout.
*/
private int soTimeout = 60000;

/**
* Connection timeout.
*/
private int connectionTimeout = 5000;

private static final Logger LOGGER = LoggerFactory.getLogger(OEmbedClientImpl.class);

private Map<String, OEmbedClientImplConfigurationFactory.Config> configs = new HashMap<>();
Expand Down Expand Up @@ -103,7 +93,7 @@ public OEmbedResponse getResponse(String url) {
return null;
}
OEmbedResponse.Format format = OEmbedResponse.Format.fromString(config.format());
try (CloseableHttpClient httpClient = getHttpClient()) {
try (CloseableHttpClient httpClient = getHttpClient(config)) {
if (OEmbedResponse.Format.JSON == format) {
String jsonURL = buildURL(config.endpoint(), url, OEmbedResponse.Format.JSON.getValue(), null, null);
try (InputStream jsonStream = getDataStream(jsonURL, httpClient)) {
Expand All @@ -112,6 +102,7 @@ public OEmbedResponse getResponse(String url) {
} else if (jaxbContext != null && OEmbedResponse.Format.XML == format) {
String xmlURL = buildURL(config.endpoint(), url, OEmbedResponse.Format.XML.getValue(), null, null);
try (InputStream xmlStream = getDataStream(xmlURL, httpClient)) {

//Disable XXE
SAXParserFactory spf = SAXParserFactory.newInstance();
spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
Expand Down Expand Up @@ -154,8 +145,8 @@ protected OEmbedClientImplConfigurationFactory.Config getConfiguration(String ur
return null;
}

protected CloseableHttpClient getHttpClient() {
RequestConfig rc = RequestConfig.custom().setConnectTimeout(connectionTimeout).setSocketTimeout(soTimeout)
protected CloseableHttpClient getHttpClient(OEmbedClientImplConfigurationFactory.Config config) {
RequestConfig rc = RequestConfig.custom().setConnectTimeout(config.connectionTimeout()).setSocketTimeout(config.socketTimeout())
.build();
if (httpClientBuilderFactory != null && httpClientBuilderFactory.newBuilder() != null) {
return httpClientBuilderFactory.newBuilder().setDefaultRequestConfig(rc).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ public class OEmbedClientImplConfigurationFactory {
description = "Configuration for defining oEmbed endpoints."
)
public @interface Config {

int DEFAULT_CONNECTION_TIMEOUT = 2000, DEFAULT_SOCKET_TIMEOUT=5000;

@AttributeDefinition(
name = "Provider Name",
description = "Name of the oEmbed provider."
Expand Down Expand Up @@ -77,6 +80,18 @@ public class OEmbedClientImplConfigurationFactory {
description = "Describes whether the provider response HTML is allowed to be displayed in an unsafe context."
)
boolean unsafeContext() default false;

@AttributeDefinition(
name = "Socket Timeout",
description = "The time waiting for data – after establishing the connection; maximum time of inactivity between two data packets."
)
int socketTimeout() default DEFAULT_SOCKET_TIMEOUT;

@AttributeDefinition(
name = "Connection Timeout",
description = "The time to establish the connection with the remote host."
)
int connectionTimeout() default DEFAULT_CONNECTION_TIMEOUT;
}

@Activate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ private List getListUnderTest(String resourcePath) {
Utils.enableDataLayer(context, true);
Resource resource = context.resourceResolver().getResource(resourcePath);
if (resource == null) {
throw new IllegalStateException("Did you forget to defines test resource " + resourcePath + "?");
throw new IllegalStateException("Did you forget to define test resource " + resourcePath + "?");
}
context.request().setContextPath(CONTEXT_PATH);
context.currentResource(resource);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,16 @@ public String[] scheme() {
public boolean unsafeContext() {
return false;
}

@Override
public int socketTimeout() {
return 1000;
}

@Override
public int connectionTimeout() {
return 1000;
}
});

client.bindOEmbedClientImplConfigurationFactory(configurationFactory, new HashMap<>());
Expand Down Expand Up @@ -155,6 +165,16 @@ public String[] scheme() {
public boolean unsafeContext() {
return false;
}

@Override
public int socketTimeout() {
return 1000;
}

@Override
public int connectionTimeout() {
return 1000;
}
});

client.bindOEmbedClientImplConfigurationFactory(configurationFactory, new HashMap<>());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,5 @@ scheme=["https?://www\\.facebook\\.com/.*/posts/.*",
"https?://www\\.facebook\\.com/photo\\.php.*",
"https?://www\\.facebook\\.com/photo\\.php"]
unsafeContext="true"
socketTimeout="5000"
connectionTimeout="2000"
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@ endpoint="https://www.facebook.com/plugins/video/oembed.json"
format="json"
scheme=["https?://www\\.facebook\\.com/.*/videos/.*","https?://www\\.facebook\\.com/video\\.php.*"]
unsafeContext="true"
socketTimeout="5000"
connectionTimeout="2000"
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@ endpoint="https://www.flickr.com/services/oembed/"
format="xml"
scheme=["https?://.*\.flickr\\.com/photos/.*","https?://flic\\.kr/p/.*"]
unsafeContext="true"
socketTimeout="5000"
connectionTimeout="2000"
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@ endpoint="https://api.instagram.com/oembed"
format="json"
scheme=["https?://(www\\.)?instagram\\.com/p/.*","https?://(www\\.)?instagr\\.am/p/.*"]
unsafeContext="true"
socketTimeout="5000"
connectionTimeout="2000"
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@ endpoint="https://soundcloud.com/oembed"
format="json"
scheme=["https?://soundcloud\\.com/.*"]
unsafeContext="true"
socketTimeout="5000"
connectionTimeout="2000"
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@ endpoint="https://publish.twitter.com/oembed"
format="json"
scheme=["https?://(.*\.)?twitter\.com/.*"]
unsafeContext="true"
socketTimeout="5000"
connectionTimeout="2000"
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@ endpoint="https://www.youtube.com/oembed"
format="json"
scheme=["https://.*\.youtube.com/watch.*","https://.*\.youtube.com/v/.*","https://youtu\.be/.*"]
unsafeContext="true"
socketTimeout="5000"
connectionTimeout="2000"
5 changes: 5 additions & 0 deletions content/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@
</dependencies>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-antrun-plugin</artifactId>
<version>1.3</version>
</plugin>
<plugin>
<groupId>com.day.jcr.vault</groupId>
<artifactId>content-package-maven-plugin</artifactId>
Expand Down
Loading