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

feat: support chunked transfer encoding #910

Merged
merged 6 commits into from
Dec 17, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
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(
codyoss marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright 2019 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/

package com.google.api.client.http.apache.v2;
codyoss marked this conversation as resolved.
Show resolved Hide resolved

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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package com.google.api.client.http;

import com.google.api.client.util.Beta;
import com.google.api.client.util.IOUtils;
import com.google.api.client.util.LoggingStreamingContent;
import com.google.api.client.util.ObjectParser;
import com.google.api.client.util.Preconditions;
Expand Down Expand Up @@ -141,7 +140,7 @@ public final class HttpRequest {

/** HTTP request URL. */
private GenericUrl url;

/** Timeout in milliseconds to establish a connection or {@code 0} for an infinite timeout. */
private int connectTimeout = 20 * 1000;

Expand Down Expand Up @@ -172,7 +171,7 @@ public final class HttpRequest {
/** The {@link BackOffPolicy} to use between retry attempts or {@code null} for none. */
@Deprecated @Beta private BackOffPolicy backOffPolicy;

/** Whether to automatically follow redirects ({@code true} by default). */
/** Whether to automatically follow redirects ({@code true} by default). */
private boolean followRedirects = true;

/** Whether to use raw redirect URLs ({@code false} by default). */
Expand Down Expand Up @@ -698,15 +697,13 @@ public HttpRequest setFollowRedirects(boolean followRedirects) {
return this;
}

/**
* Return whether to use raw redirect URLs.
*/
/** Return whether to use raw redirect URLs. */
public boolean getUseRawRedirectUrls() {
return useRawRedirectUrls;
}

/**
* Sets whether to use raw redirect URLs.
* Sets whether to use raw redirect URLs.
*
* <p>The default value is {@code false}.
*/
Expand Down Expand Up @@ -938,7 +935,7 @@ public HttpResponse execute() throws IOException {
final boolean contentRetrySupported = streamingContent == null || content.retrySupported();
if (streamingContent != null) {
final String contentEncoding;
final long contentLength;
long contentLength = -1;
final String contentType = content.getType();
// log content
if (loggable) {
Expand All @@ -953,7 +950,6 @@ public HttpResponse execute() throws IOException {
} else {
contentEncoding = encoding.getName();
streamingContent = new HttpEncodingStreamingContent(streamingContent, encoding);
contentLength = contentRetrySupported ? IOUtils.computeLength(streamingContent) : -1;
}
// append content headers to log buffer
if (loggable) {
Expand Down Expand Up @@ -1222,13 +1218,14 @@ private static void addSpanAttribute(Span span, String key, String value) {
span.putAttribute(key, AttributeValue.stringAttributeValue(value));
}
}

private static String getVersion() {
String version = HttpRequest.class.getPackage().getImplementationVersion();
// in a non-packaged environment (local), there's no implementation version to read
if (version == null) {
// fall back to reading from a properties file - note this value is expected to be cached
try (InputStream inputStream = HttpRequest.class.getResourceAsStream("/google-http-client.properties")) {
try (InputStream inputStream =
HttpRequest.class.getResourceAsStream("/google-http-client.properties")) {
if (inputStream != null) {
Properties properties = new Properties();
properties.load(inputStream);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@
import org.apache.http.params.HttpConnectionParams;
import org.apache.http.params.HttpParams;

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

Expand All @@ -54,12 +52,16 @@ public void setTimeout(int connectTimeout, int readTimeout) throws IOException {
@Override
public LowLevelHttpResponse execute() throws IOException {
if (getStreamingContent() != null) {
Preconditions.checkArgument(request instanceof HttpEntityEnclosingRequest,
Preconditions.checkArgument(
request instanceof HttpEntityEnclosingRequest,
codyoss marked this conversation as resolved.
Show resolved Hide resolved
"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);
}
return new ApacheHttpResponse(request, httpClient.execute(request));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ public void addHeader(String name, String value) {
connection.addRequestProperty(name, value);
}

@VisibleForTesting
String getRequestProperty(String name) {
return connection.getRequestProperty(name);
}

@Override
public void setTimeout(int connectTimeout, int readTimeout) {
connection.setReadTimeout(readTimeout);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,4 +201,8 @@ public String getHeaderField(String name) {
List<String> values = headers.get(name);
return values == null ? null : values.get(0);
}

public int getChunkLength() {
return chunkLength;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;

import java.util.regex.Pattern;
import junit.framework.TestCase;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.util.Arrays;
Expand All @@ -45,7 +41,8 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.logging.Level;

import java.util.regex.Pattern;
import junit.framework.TestCase;
import org.junit.Assert;

/**
Expand Down Expand Up @@ -990,7 +987,7 @@ public LowLevelHttpResponse execute() throws IOException {
if (expectGZip) {
assertEquals(HttpEncodingStreamingContent.class, getStreamingContent().getClass());
assertEquals("gzip", getContentEncoding());
assertEquals(25, getContentLength());
assertEquals(-1, getContentLength());
} else {
assertFalse(
getStreamingContent().getClass().equals(HttpEncodingStreamingContent.class));
Expand Down Expand Up @@ -1230,7 +1227,9 @@ public void testExecute_curlLogger() throws Exception {
if (message.startsWith("curl")) {
found = true;
assertTrue(message.contains("curl -v --compressed -H 'Accept-Encoding: gzip'"));
assertTrue(message.contains("-H 'User-Agent: Google-HTTP-Java-Client/" + HttpRequest.VERSION + " (gzip)'"));
assertTrue(
message.contains(
"-H 'User-Agent: Google-HTTP-Java-Client/" + HttpRequest.VERSION + " (gzip)'"));
assertTrue(message.contains("' -- 'http://google.com/#q=a'\"'\"'b'\"'\"'c'"));
}
}
Expand Down Expand Up @@ -1261,7 +1260,9 @@ public void testExecute_curlLoggerWithContentEncoding() throws Exception {
found = true;
assertTrue(message.contains("curl -v --compressed -X POST -H 'Accept-Encoding: gzip'"));
assertTrue(message.contains("-H 'User-Agent: " + HttpRequest.USER_AGENT_SUFFIX + "'"));
assertTrue(message.contains("-H 'Content-Type: text/plain; charset=UTF-8' -H 'Content-Encoding: gzip'"));
assertTrue(
message.contains(
"-H 'Content-Type: text/plain; charset=UTF-8' -H 'Content-Encoding: gzip'"));
assertTrue(message.contains("-d '@-' -- 'http://google.com/#q=a'\"'\"'b'\"'\"'c' << $$$"));
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
package com.google.api.client.http.javanet;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.junit.Assert.*;

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.http.LowLevelHttpResponse;
Expand All @@ -15,6 +14,7 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.concurrent.TimeoutException;
import org.junit.Test;

Expand Down Expand Up @@ -203,4 +203,34 @@ public void close() throws IOException {
assertEquals("Error during close", e.getMessage());
}
}

@Test
public void testChunkedLengthSet() throws Exception {
MockHttpURLConnection connection = new MockHttpURLConnection(new URL(HttpTesting.SIMPLE_URL));
connection.setRequestMethod("POST");
NetHttpRequest request = new NetHttpRequest(connection);
InputStream is = NetHttpRequestTest.class.getClassLoader().getResourceAsStream("file.txt");
HttpContent content = new InputStreamContent("text/plain", is);
request.setStreamingContent(content);
request.setContentEncoding("gzip");
request.execute();

assertEquals(connection.getChunkLength(), 4096);
codyoss marked this conversation as resolved.
Show resolved Hide resolved
assertNull(request.getRequestProperty("Content-Length"));
}

@Test
public void testChunkedLengthNotSet() throws Exception {
MockHttpURLConnection connection = new MockHttpURLConnection(new URL(HttpTesting.SIMPLE_URL));
connection.setRequestMethod("POST");
NetHttpRequest request = new NetHttpRequest(connection);
HttpContent content =
new ByteArrayContent("text/plain", "sample".getBytes(StandardCharsets.UTF_8));
request.setStreamingContent(content);
request.setContentLength(content.getLength());
request.execute();

assertEquals(connection.getChunkLength(), -1);
assertEquals("6", request.getRequestProperty("Content-Length"));
}
}