diff --git a/google-http-client-apache-v2/src/main/java/com/google/api/client/http/apache/v2/ApacheHttpRequest.java b/google-http-client-apache-v2/src/main/java/com/google/api/client/http/apache/v2/ApacheHttpRequest.java index 29bd61ee7..ae2aac763 100644 --- a/google-http-client-apache-v2/src/main/java/com/google/api/client/http/apache/v2/ApacheHttpRequest.java +++ b/google-http-client-apache-v2/src/main/java/com/google/api/client/http/apache/v2/ApacheHttpRequest.java @@ -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; @@ -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 @@ -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.checkState( + 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()); diff --git a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpRequestTest.java b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpRequestTest.java new file mode 100644 index 000000000..74c97244a --- /dev/null +++ b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpRequestTest.java @@ -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; + +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()); + } +} diff --git a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java index e6ca850ce..e9b93e9be 100644 --- a/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java +++ b/google-http-client-apache-v2/src/test/java/com/google/api/client/http/apache/v2/ApacheHttpTransportTest.java @@ -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; @@ -123,8 +122,8 @@ private void subtestUnsupportedRequestsWithContent(ApacheHttpRequest request, St throws IOException { try { execute(request); - fail("expected " + IllegalArgumentException.class); - } catch (IllegalArgumentException e) { + fail("expected " + IllegalStateException.class); + } catch (IllegalStateException e) { // expected assertEquals(e.getMessage(), "Apache HTTP client does not support " + method + " requests with content."); diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java b/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java index 23ec3e168..9124e4906 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java @@ -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; @@ -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; @@ -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). */ @@ -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. * *

The default value is {@code false}. */ @@ -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) { @@ -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) { @@ -1222,7 +1218,7 @@ private static void addSpanAttribute(Span span, String key, String value) { span.putAttribute(key, AttributeValue.stringAttributeValue(value)); } } - + private static String getVersion() { // attempt to read the library's version from a properties file generated during the build // this value should be read and cached for later use diff --git a/google-http-client/src/main/java/com/google/api/client/http/apache/ApacheHttpRequest.java b/google-http-client/src/main/java/com/google/api/client/http/apache/ApacheHttpRequest.java index b31b20594..355acfcd0 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/apache/ApacheHttpRequest.java +++ b/google-http-client/src/main/java/com/google/api/client/http/apache/ApacheHttpRequest.java @@ -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; @@ -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.checkState( + 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); } return new ApacheHttpResponse(request, httpClient.execute(request)); diff --git a/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpRequest.java b/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpRequest.java index aa0d8e3e4..1d043472b 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpRequest.java +++ b/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpRequest.java @@ -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); diff --git a/google-http-client/src/main/java/com/google/api/client/testing/http/javanet/MockHttpURLConnection.java b/google-http-client/src/main/java/com/google/api/client/testing/http/javanet/MockHttpURLConnection.java index 467f495f4..2fba28b48 100644 --- a/google-http-client/src/main/java/com/google/api/client/testing/http/javanet/MockHttpURLConnection.java +++ b/google-http-client/src/main/java/com/google/api/client/testing/http/javanet/MockHttpURLConnection.java @@ -201,4 +201,8 @@ public String getHeaderField(String name) { List values = headers.get(name); return values == null ? null : values.get(0); } + + public int getChunkLength() { + return chunkLength; + } } diff --git a/google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java b/google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java index 154f3af16..bb47e98d7 100644 --- a/google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java +++ b/google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java @@ -987,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)); @@ -1227,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'")); } } @@ -1258,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' << $$$")); } } diff --git a/google-http-client/src/test/java/com/google/api/client/http/javanet/NetHttpRequestTest.java b/google-http-client/src/test/java/com/google/api/client/http/javanet/NetHttpRequestTest.java index d4118328e..ae3606ca5 100644 --- a/google-http-client/src/test/java/com/google/api/client/http/javanet/NetHttpRequestTest.java +++ b/google-http-client/src/test/java/com/google/api/client/http/javanet/NetHttpRequestTest.java @@ -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; @@ -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; @@ -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(4096, connection.getChunkLength()); + 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")); + } }