Skip to content

Commit

Permalink
feat: support chunked transfer encoding
Browse files Browse the repository at this point in the history
Currently any time HttpRequest works with encoded data it encodes
the data twice. Once for the actaul stream and once for checking
the length of the stream. Instead, when there is encoding just don't
set the content length. This will cause the underlying transports,
with a few tweaks, to use Transfer-Encoding: chunked.

Fixes googleapis#648
  • Loading branch information
codyoss committed Dec 10, 2019
1 parent 53c50d2 commit 74556be
Show file tree
Hide file tree
Showing 32 changed files with 387 additions and 307 deletions.
Expand Up @@ -23,9 +23,7 @@
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.HttpRequestBase;

/**
* @author Yaniv Inbar
*/
/** @author Yaniv Inbar */
final class ApacheHttpRequest extends LowLevelHttpRequest {
private final HttpClient httpClient;

Expand All @@ -38,11 +36,12 @@ final class ApacheHttpRequest extends LowLevelHttpRequest {
this.httpClient = httpClient;
this.request = request;
// disable redirects as google-http-client handles redirects
this.requestConfig = RequestConfig.custom()
.setRedirectsEnabled(false)
.setNormalizeUri(false)
// TODO(chingor): configure in HttpClientBuilder when available
.setStaleConnectionCheckEnabled(false);
this.requestConfig =
RequestConfig.custom()
.setRedirectsEnabled(false)
.setNormalizeUri(false)
// TODO(chingor): configure in HttpClientBuilder when available
.setStaleConnectionCheckEnabled(false);
}

@Override
Expand All @@ -52,19 +51,22 @@ public void addHeader(String name, String value) {

@Override
public void setTimeout(int connectTimeout, int readTimeout) throws IOException {
requestConfig.setConnectTimeout(connectTimeout)
.setSocketTimeout(readTimeout);
requestConfig.setConnectTimeout(connectTimeout).setSocketTimeout(readTimeout);
}

@Override
public LowLevelHttpResponse execute() throws IOException {
if (getStreamingContent() != null) {
Preconditions.checkArgument(request instanceof HttpEntityEnclosingRequest,
Preconditions.checkArgument(
request instanceof HttpEntityEnclosingRequest,
"Apache HTTP client does not support %s requests with content.",
request.getRequestLine().getMethod());
ContentEntity entity = new ContentEntity(getContentLength(), getStreamingContent());
entity.setContentEncoding(getContentEncoding());
entity.setContentType(getContentType());
if (getContentLength() == -1) {
entity.setChunked(true);
}
((HttpEntityEnclosingRequest) request).setEntity(entity);
}
request.setConfig(requestConfig.build());
Expand Down
Expand Up @@ -38,19 +38,15 @@
/**
* Thread-safe HTTP transport based on the Apache HTTP Client library.
*
* <p>
* Implementation is thread-safe, as long as any parameter modification to the
* {@link #getHttpClient() Apache HTTP Client} is only done at initialization time. For maximum
* efficiency, applications should use a single globally-shared instance of the HTTP transport.
* </p>
* <p>Implementation is thread-safe, as long as any parameter modification to the {@link
* #getHttpClient() Apache HTTP Client} is only done at initialization time. For maximum efficiency,
* applications should use a single globally-shared instance of the HTTP transport.
*
* <p>
* Default settings are specified in {@link #newDefaultHttpClient()}. Use the
* {@link #ApacheHttpTransport(HttpClient)} constructor to override the Apache HTTP Client used.
* Please read the <a
* <p>Default settings are specified in {@link #newDefaultHttpClient()}. Use the {@link
* #ApacheHttpTransport(HttpClient)} constructor to override the Apache HTTP Client used. Please
* read the <a
* href="http://hc.apache.org/httpcomponents-client-ga/tutorial/html/connmgmt.html">Apache HTTP
* Client connection management tutorial</a> for more complex configuration options.
* </p>
*
* @since 1.30
* @author Yaniv Inbar
Expand All @@ -72,41 +68,38 @@ public ApacheHttpTransport() {
/**
* Constructor that allows an alternative Apache HTTP client to be used.
*
* <p>
* Note that in the previous version, we overrode several settings. However, we are no longer able
* to do so.
* </p>
* <p>Note that in the previous version, we overrode several settings. However, we are no longer
* able to do so.
*
* <p>If you choose to provide your own Apache HttpClient implementation, be sure that
*
* <p>If you choose to provide your own Apache HttpClient implementation, be sure that</p>
* <ul>
* <li>HTTP version is set to 1.1.</li>
* <li>Redirects are disabled (google-http-client handles redirects).</li>
* <li>Retries are disabled (google-http-client handles retries).</li>
* <li>HTTP version is set to 1.1.
* <li>Redirects are disabled (google-http-client handles redirects).
* <li>Retries are disabled (google-http-client handles retries).
* </ul>
*
* @param httpClient Apache HTTP client to use
*
* @since 1.30
*/
public ApacheHttpTransport(HttpClient httpClient) {
this.httpClient = httpClient;
}

/**
* Creates a new instance of the Apache HTTP client that is used by the
* {@link #ApacheHttpTransport()} constructor.
* Creates a new instance of the Apache HTTP client that is used by the {@link
* #ApacheHttpTransport()} constructor.
*
* <p>Settings:
*
* <p>
* Settings:
* </p>
* <ul>
* <li>The client connection manager is set to {@link PoolingHttpClientConnectionManager}.</li>
* <li><The retry mechanism is turned off using
* {@link HttpClientBuilder#disableRedirectHandling}.</li>
* <li>The route planner uses {@link SystemDefaultRoutePlanner} with
* {@link ProxySelector#getDefault()}, which uses the proxy settings from <a
* href="https://docs.oracle.com/javase/7/docs/api/java/net/doc-files/net-properties.html">system
* properties</a>.</li>
* <li>The client connection manager is set to {@link PoolingHttpClientConnectionManager}.
* <li><The retry mechanism is turned off using {@link
* HttpClientBuilder#disableRedirectHandling}.
* <li>The route planner uses {@link SystemDefaultRoutePlanner} with {@link
* ProxySelector#getDefault()}, which uses the proxy settings from <a
* href="https://docs.oracle.com/javase/7/docs/api/java/net/doc-files/net-properties.html">system
* properties</a>.
* </ul>
*
* @return new instance of the Apache HTTP client
Expand All @@ -117,20 +110,19 @@ public static HttpClient newDefaultHttpClient() {
}

/**
* Creates a new Apache HTTP client builder that is used by the
* {@link #ApacheHttpTransport()} constructor.
* Creates a new Apache HTTP client builder that is used by the {@link #ApacheHttpTransport()}
* constructor.
*
* <p>Settings:
*
* <p>
* Settings:
* </p>
* <ul>
* <li>The client connection manager is set to {@link PoolingHttpClientConnectionManager}.</li>
* <li><The retry mechanism is turned off using
* {@link HttpClientBuilder#disableRedirectHandling}.</li>
* <li>The route planner uses {@link SystemDefaultRoutePlanner} with
* {@link ProxySelector#getDefault()}, which uses the proxy settings from <a
* href="http://docs.oracle.com/javase/7/docs/api/java/net/doc-files/net-properties.html">system
* properties</a>.</li>
* <li>The client connection manager is set to {@link PoolingHttpClientConnectionManager}.
* <li><The retry mechanism is turned off using {@link
* HttpClientBuilder#disableRedirectHandling}.
* <li>The route planner uses {@link SystemDefaultRoutePlanner} with {@link
* ProxySelector#getDefault()}, which uses the proxy settings from <a
* href="http://docs.oracle.com/javase/7/docs/api/java/net/doc-files/net-properties.html">system
* properties</a>.
* </ul>
*
* @return new instance of the Apache HTTP client
Expand All @@ -139,14 +131,14 @@ public static HttpClient newDefaultHttpClient() {
public static HttpClientBuilder newDefaultHttpClientBuilder() {

return HttpClientBuilder.create()
.useSystemProperties()
.setSSLSocketFactory(SSLConnectionSocketFactory.getSocketFactory())
.setMaxConnTotal(200)
.setMaxConnPerRoute(20)
.setConnectionTimeToLive(-1, TimeUnit.MILLISECONDS)
.setRoutePlanner(new SystemDefaultRoutePlanner(ProxySelector.getDefault()))
.disableRedirectHandling()
.disableAutomaticRetries();
.useSystemProperties()
.setSSLSocketFactory(SSLConnectionSocketFactory.getSocketFactory())
.setMaxConnTotal(200)
.setMaxConnPerRoute(20)
.setConnectionTimeToLive(-1, TimeUnit.MILLISECONDS)
.setRoutePlanner(new SystemDefaultRoutePlanner(ProxySelector.getDefault()))
.disableRedirectHandling()
.disableAutomaticRetries();
}

@Override
Expand Down
Expand Up @@ -21,9 +21,7 @@
import java.io.OutputStream;
import org.apache.http.entity.AbstractHttpEntity;

/**
* @author Yaniv Inbar
*/
/** @author Yaniv Inbar */
final class ContentEntity extends AbstractHttpEntity {

/** Content length or less than zero if not known. */
Expand Down
Expand Up @@ -18,6 +18,4 @@
* @since 1.30
* @author Yaniv Inbar
*/

package com.google.api.client.http.apache.v2;

@@ -0,0 +1,45 @@
package com.google.api.client.http.apache.v2;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import com.google.api.client.http.ByteArrayContent;
import com.google.api.client.http.HttpContent;
import com.google.api.client.http.InputStreamContent;
import com.google.api.client.testing.http.apache.MockHttpClient;
import java.io.ByteArrayInputStream;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import org.junit.Test;

public class ApacheHttpRequestTest {

@Test
public void testContentLengthSet() throws Exception {
HttpExtensionMethod base = new HttpExtensionMethod("POST", "http://www.google.com");
ApacheHttpRequest request = new ApacheHttpRequest(new MockHttpClient(), base);
HttpContent content =
new ByteArrayContent("text/plain", "sample".getBytes(StandardCharsets.UTF_8));
request.setStreamingContent(content);
request.setContentLength(content.getLength());
request.execute();

assertFalse(base.getEntity().isChunked());
assertEquals(6, base.getEntity().getContentLength());
}

@Test
public void testChunked() throws Exception {
byte[] buf = new byte[300];
Arrays.fill(buf, (byte) ' ');
HttpExtensionMethod base = new HttpExtensionMethod("POST", "http://www.google.com");
ApacheHttpRequest request = new ApacheHttpRequest(new MockHttpClient(), base);
HttpContent content = new InputStreamContent("text/plain", new ByteArrayInputStream(buf));
request.setStreamingContent(content);
request.execute();

assertTrue(base.getEntity().isChunked());
assertEquals(-1, base.getEntity().getContentLength());
}
}
Expand Up @@ -34,7 +34,6 @@
import java.io.OutputStream;
import java.net.InetSocketAddress;
import java.nio.charset.StandardCharsets;
import java.util.Random;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import org.apache.http.Header;
Expand Down Expand Up @@ -126,7 +125,8 @@ private void subtestUnsupportedRequestsWithContent(ApacheHttpRequest request, St
fail("expected " + IllegalArgumentException.class);
} catch (IllegalArgumentException e) {
// expected
assertEquals(e.getMessage(),
assertEquals(
e.getMessage(),
"Apache HTTP client does not support " + method + " requests with content.");
}
}
Expand All @@ -142,16 +142,18 @@ private void execute(ApacheHttpRequest request) throws IOException {
@Test
public void testRequestShouldNotFollowRedirects() throws IOException {
final AtomicInteger requestsAttempted = new AtomicInteger(0);
HttpRequestExecutor requestExecutor = new HttpRequestExecutor() {
@Override
public HttpResponse execute(HttpRequest request, HttpClientConnection connection,
HttpContext context) throws IOException, HttpException {
HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 302, null);
response.addHeader("location", "https://google.com/path");
requestsAttempted.incrementAndGet();
return response;
}
};
HttpRequestExecutor requestExecutor =
new HttpRequestExecutor() {
@Override
public HttpResponse execute(
HttpRequest request, HttpClientConnection connection, HttpContext context)
throws IOException, HttpException {
HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 302, null);
response.addHeader("location", "https://google.com/path");
requestsAttempted.incrementAndGet();
return response;
}
};
HttpClient client = HttpClients.custom().setRequestExecutor(requestExecutor).build();
ApacheHttpTransport transport = new ApacheHttpTransport(client);
ApacheHttpRequest request = transport.buildRequest("GET", "https://google.com");
Expand All @@ -163,17 +165,21 @@ public HttpResponse execute(HttpRequest request, HttpClientConnection connection
@Test
public void testRequestCanSetHeaders() {
final AtomicBoolean interceptorCalled = new AtomicBoolean(false);
HttpClient client = HttpClients.custom().addInterceptorFirst(new HttpRequestInterceptor() {
@Override
public void process(HttpRequest request, HttpContext context)
throws HttpException, IOException {
Header header = request.getFirstHeader("foo");
assertNotNull("Should have found header", header);
assertEquals("bar", header.getValue());
interceptorCalled.set(true);
throw new IOException("cancelling request");
}
}).build();
HttpClient client =
HttpClients.custom()
.addInterceptorFirst(
new HttpRequestInterceptor() {
@Override
public void process(HttpRequest request, HttpContext context)
throws HttpException, IOException {
Header header = request.getFirstHeader("foo");
assertNotNull("Should have found header", header);
assertEquals("bar", header.getValue());
interceptorCalled.set(true);
throw new IOException("cancelling request");
}
})
.build();

ApacheHttpTransport transport = new ApacheHttpTransport(client);
ApacheHttpRequest request = transport.buildRequest("GET", "https://google.com");
Expand Down Expand Up @@ -225,10 +231,7 @@ public void handle(HttpExchange httpExchange) throws IOException {
GenericUrl testUrl = new GenericUrl("http://localhost/foo//bar");
testUrl.setPort(server.getAddress().getPort());
com.google.api.client.http.HttpResponse response =
transport
.createRequestFactory()
.buildGetRequest(testUrl)
.execute();
transport.createRequestFactory().buildGetRequest(testUrl).execute();
assertEquals(200, response.getStatusCode());
assertEquals("/foo//bar", response.parseAsString());
}
Expand Down
Expand Up @@ -15,28 +15,26 @@
/**
* Findbugs package which supports custom Google APIs Client library findbugs Plugins.
*
* Usage on pom.xml:
* <p>Usage on pom.xml:
*
* <pre>
&lt;plugin&gt;
&lt;groupId>org.codehaus.mojo&lt;/groupId&gt;
&lt;artifactId>findbugs-maven-plugin&lt;/artifactId&gt;
...
&lt;configuration&gt;
&lt;plugins&gt;
&lt;plugin&gt;
&lt;groupId&gt;com.google.http-client&lt;/groupId&gt;
&lt;artifactId&gt;google-http-client-findbugs&lt;/artifactId&gt;
&lt;version&gt;${project.http.version}&lt;/version&gt;
&lt;/plugin&gt;
&lt;/plugins&gt;
&lt;/configuration&gt;
...
&lt;/plugin&gt;
* &lt;plugin&gt;
* &lt;groupId>org.codehaus.mojo&lt;/groupId&gt;
* &lt;artifactId>findbugs-maven-plugin&lt;/artifactId&gt;
* ...
* &lt;configuration&gt;
* &lt;plugins&gt;
* &lt;plugin&gt;
* &lt;groupId&gt;com.google.http-client&lt;/groupId&gt;
* &lt;artifactId&gt;google-http-client-findbugs&lt;/artifactId&gt;
* &lt;version&gt;${project.http.version}&lt;/version&gt;
* &lt;/plugin&gt;
* &lt;/plugins&gt;
* &lt;/configuration&gt;
* ...
* &lt;/plugin&gt;
* </pre>
*
* @author Eyal Peled
*/

package com.google.api.client.findbugs;

0 comments on commit 74556be

Please sign in to comment.