honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Watcher: Fix proxy scheme to default to HTTP (elastic/x-pack-elasticsearch#3844) 

This fixes a regression introduced in Elasticsearch 6.0, when switching
from HttpURLConnection to the Apache HTTP Client.

In the old implementation there was no way to specify if you wanted to use HTTP
or HTTPS for your proxy, only HTTP. If people needed to use HTTPs, they
could just use the CONNECT feature of the proxy.

The new implementation used the scheme of the request that was about to
be sent out as the proxy scheme to be used. So if the request was HTTPS
but the proxy server was HTTP this created a problem.

This commit changes the default scheme to be just HTTP, so that then the
standard CONNECT procecure is taken care off.

Without a real proxy server this is super hard to test. I have verified
this with the following test against a tinyproxy running on port 8888,
but I do not have a great idea how to test this in a unit testable way using a real proxy.

Original commit: elastic/x-pack-elasticsearch@f68e72d8f1
This commit is contained in:
Alexander Reelsen 2018-02-12 13:12:38 +01:00 committed by GitHub
parent 8e501b6a7e
commit 48f6a752cb
2 changed files with 84 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/http/HttpClient.java
View File

@ -45,7 +45,6 @@ import org.elasticsearch.xpack.watcher.common.http.auth.ApplicableHttpAuth;
import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuthRegistry; import org.elasticsearch.xpack.watcher.common.http.auth.HttpAuthRegistry;
import javax.net.ssl.HostnameVerifier; import javax.net.ssl.HostnameVerifier;
import java.io.ByteArrayOutputStream; import java.io.ByteArrayOutputStream;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
@ -63,9 +62,7 @@ public class HttpClient extends AbstractComponent {
private final HttpAuthRegistry httpAuthRegistry; private final HttpAuthRegistry httpAuthRegistry;
private final CloseableHttpClient client; private final CloseableHttpClient client;
private final Integer proxyPort; private final HttpProxy settingsProxy;
private final String proxyHost;
private final String proxyScheme;
private final TimeValue defaultConnectionTimeout; private final TimeValue defaultConnectionTimeout;
private final TimeValue defaultReadTimeout; private final TimeValue defaultReadTimeout;
private final ByteSizeValue maxResponseSize; private final ByteSizeValue maxResponseSize;
@ -76,17 +73,7 @@ public class HttpClient extends AbstractComponent {
this.defaultConnectionTimeout = HttpSettings.CONNECTION_TIMEOUT.get(settings); this.defaultConnectionTimeout = HttpSettings.CONNECTION_TIMEOUT.get(settings);
this.defaultReadTimeout = HttpSettings.READ_TIMEOUT.get(settings); this.defaultReadTimeout = HttpSettings.READ_TIMEOUT.get(settings);
this.maxResponseSize = HttpSettings.MAX_HTTP_RESPONSE_SIZE.get(settings); this.maxResponseSize = HttpSettings.MAX_HTTP_RESPONSE_SIZE.get(settings);
this.settingsProxy = getProxyFromSettings();
// proxy setup
this.proxyHost = HttpSettings.PROXY_HOST.get(settings);
this.proxyScheme = HttpSettings.PROXY_SCHEME.exists(settings) ? HttpSettings.PROXY_SCHEME.get(settings) : null;
this.proxyPort = HttpSettings.PROXY_PORT.get(settings);
if (proxyPort != 0 && Strings.hasText(proxyHost)) {
logger.info("Using default proxy for http input and slack/hipchat/pagerduty/webhook actions [{}:{}]", proxyHost, proxyPort);
} else if (proxyPort != 0 ^ Strings.hasText(proxyHost)) {
throw new IllegalArgumentException("HTTP proxy requires both settings: [" + HttpSettings.PROXY_HOST.getKey() + "] and [" +
HttpSettings.PROXY_PORT.getKey() + "]");
}
HttpClientBuilder clientBuilder = HttpClientBuilder.create(); HttpClientBuilder clientBuilder = HttpClientBuilder.create();
@ -122,8 +109,6 @@ public class HttpClient extends AbstractComponent {
} }
internalRequest.setHeader(HttpHeaders.ACCEPT_CHARSET, StandardCharsets.UTF_8.name()); internalRequest.setHeader(HttpHeaders.ACCEPT_CHARSET, StandardCharsets.UTF_8.name());
RequestConfig.Builder config = RequestConfig.custom();
// headers // headers
if (request.headers().isEmpty() == false) { if (request.headers().isEmpty() == false) {
for (Map.Entry<String, String> entry : request.headers.entrySet()) { for (Map.Entry<String, String> entry : request.headers.entrySet()) {
@ -139,19 +124,8 @@ public class HttpClient extends AbstractComponent {
} }
} }
// proxy RequestConfig.Builder config = RequestConfig.custom();
if (request.proxy != null && request.proxy.equals(HttpProxy.NO_PROXY) == false) { setProxy(config, request, settingsProxy);
// if a proxy scheme is configured use this, but fall back to the same than the request in case there was no special
// configuration given
String scheme = request.proxy.getScheme() != null ? request.proxy.getScheme().scheme() : request.scheme.scheme();
HttpHost proxy = new HttpHost(request.proxy.getHost(), request.proxy.getPort(), scheme);
config.setProxy(proxy);
} else if (proxyPort != null && Strings.hasText(proxyHost)) {
String scheme = proxyScheme != null ? proxyScheme : request.scheme.scheme();
HttpHost proxy = new HttpHost(proxyHost, proxyPort, scheme);
config.setProxy(proxy);
}
HttpClientContext localContext = HttpClientContext.create(); HttpClientContext localContext = HttpClientContext.create();
// auth // auth
if (request.auth() != null) { if (request.auth() != null) {
@ -218,6 +192,49 @@ public class HttpClient extends AbstractComponent {
} }
} }
/**
* Enriches the config object optionally with proxy information
*
* @param config The request builder config object
* @param request The request parsed into the HTTP client
*/
static void setProxy(RequestConfig.Builder config, HttpRequest request, HttpProxy configuredProxy) {
if (request.proxy != null && request.proxy.equals(HttpProxy.NO_PROXY) == false) {
// if a proxy scheme is configured use this, but fall back to the same than the request in case there was no special
// configuration given
String scheme = request.proxy.getScheme() != null ? request.proxy.getScheme().scheme() : Scheme.HTTP.scheme();
HttpHost proxy = new HttpHost(request.proxy.getHost(), request.proxy.getPort(), scheme);
config.setProxy(proxy);
} else if (HttpProxy.NO_PROXY.equals(configuredProxy) == false) {
HttpHost proxy = new HttpHost(configuredProxy.getHost(), configuredProxy.getPort(), configuredProxy.getScheme().scheme());
config.setProxy(proxy);
}
}
/**
* Creates a HTTP proxy from the system wide settings
*
* @return A http proxy instance, if no settings are configured this will be a HttpProxy.NO_PROXY instance
*/
private HttpProxy getProxyFromSettings() {
String proxyHost = HttpSettings.PROXY_HOST.get(settings);
Scheme proxyScheme = HttpSettings.PROXY_SCHEME.exists(settings) ?
Scheme.parse(HttpSettings.PROXY_SCHEME.get(settings)) : Scheme.HTTP;
int proxyPort = HttpSettings.PROXY_PORT.get(settings);
if (proxyPort != 0 && Strings.hasText(proxyHost)) {
logger.info("Using default proxy for http input and slack/hipchat/pagerduty/webhook actions [{}:{}]", proxyHost, proxyPort);
} else if (proxyPort != 0 ^ Strings.hasText(proxyHost)) {
throw new IllegalArgumentException("HTTP proxy requires both settings: [" + HttpSettings.PROXY_HOST.getKey() + "] and [" +
HttpSettings.PROXY_PORT.getKey() + "]");
}
if (proxyPort > 0 && Strings.hasText(proxyHost)) {
return new HttpProxy(proxyHost, proxyPort, proxyScheme);
}
return HttpProxy.NO_PROXY;
}
private URI createURI(HttpRequest request) { private URI createURI(HttpRequest request) {
// this could be really simple, as the apache http client has a UriBuilder class, however this class is always doing // this could be really simple, as the apache http client has a UriBuilder class, however this class is always doing
// url path escaping, and we have done this already, so this would result in double escaping // url path escaping, and we have done this already, so this would result in double escaping

38
plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/common/http/HttpClientTests.java
View File

@ -8,6 +8,7 @@ package org.elasticsearch.xpack.watcher.common.http;
import com.carrotsearch.randomizedtesting.generators.RandomStrings; import com.carrotsearch.randomizedtesting.generators.RandomStrings;
import org.apache.http.HttpHeaders; import org.apache.http.HttpHeaders;
import org.apache.http.client.ClientProtocolException; import org.apache.http.client.ClientProtocolException;
import org.apache.http.client.config.RequestConfig;
import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier; import org.apache.logging.log4j.util.Supplier;
import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.MockSecureSettings;
@ -33,7 +34,6 @@ import org.junit.After;
import org.junit.Before; import org.junit.Before;
import javax.net.ssl.SSLContext; import javax.net.ssl.SSLContext;
import java.io.BufferedReader; import java.io.BufferedReader;
import java.io.IOException; import java.io.IOException;
import java.io.InputStreamReader; import java.io.InputStreamReader;
@ -323,6 +323,42 @@ public class HttpClientTests extends ESTestCase {
} }
} }
public void testSetProxy() throws Exception {
HttpProxy localhostHttpProxy = new HttpProxy("localhost", 1234, Scheme.HTTP);
RequestConfig.Builder config = RequestConfig.custom();
// no proxy configured at all
HttpClient.setProxy(config, HttpRequest.builder().fromUrl("https://elastic.co").build(), HttpProxy.NO_PROXY);
assertThat(config.build().getProxy(), is(nullValue()));
// no system wide proxy configured, proxy in request
config = RequestConfig.custom();
HttpClient.setProxy(config,
HttpRequest.builder().fromUrl("https://elastic.co").proxy(new HttpProxy("localhost", 23456)).build(),
HttpProxy.NO_PROXY);
assertThat(config.build().getProxy().toString(), is("http://localhost:23456"));
// system wide proxy configured, no proxy in request
config = RequestConfig.custom();
HttpClient.setProxy(config, HttpRequest.builder().fromUrl("https://elastic.co").build(),
localhostHttpProxy);
assertThat(config.build().getProxy().toString(), is("http://localhost:1234"));
// proxy in request, no system wide proxy configured. request
config = RequestConfig.custom();
HttpClient.setProxy(config,
HttpRequest.builder().fromUrl("https://elastic.co").proxy(new HttpProxy("localhost", 23456, Scheme.HTTP)).build(),
HttpProxy.NO_PROXY);
assertThat(config.build().getProxy().toString(), is("http://localhost:23456"));
// proxy in request, system wide proxy configured. request wins
config = RequestConfig.custom();
HttpClient.setProxy(config,
HttpRequest.builder().fromUrl("http://elastic.co").proxy(new HttpProxy("localhost", 23456, Scheme.HTTPS)).build(),
localhostHttpProxy);
assertThat(config.build().getProxy().toString(), is("https://localhost:23456"));
}
public void testProxyCanHaveDifferentSchemeThanRequest() throws Exception { public void testProxyCanHaveDifferentSchemeThanRequest() throws Exception {
// this test fakes a proxy server that sends a response instead of forwarding it to the mock web server // this test fakes a proxy server that sends a response instead of forwarding it to the mock web server
// on top of that the proxy request is HTTPS but the real request is HTTP only // on top of that the proxy request is HTTPS but the real request is HTTP only