From 57ef11a0e1904bb932e5493a30f0a2ca2a2798cf Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Thu, 7 Oct 2021 15:02:49 -0400 Subject: [PATCH] fix: update NetHttpRequest to prevent silent retry of DELETE requests (#1472) HttpURLConnection will silently retry `DELETE` requests. This behavior is similar to other existing JDK bugs (JDK-6382788[1], JDK-6427251[2]). google-http-java-client already contains a workaround for POST and PUT requests NetHttpRequest.java#L108-L112, but does not account for `DELETE` with an empty body. This change adds handling for DELETE to leverage the same workaround as POST and PUT. [1] https://bugs.java.com/bugdatabase/view_bug.do?bug_id=6382788 [2] https://bugs.java.com/bugdatabase/view_bug.do?bug_id=6427251 Fixes #1471 --- .../client/http/javanet/NetHttpRequest.java | 3 +++ .../http/javanet/MockHttpURLConnection.java | 24 +++++++++++++++++++ .../http/javanet/NetHttpRequestTest.java | 13 ++++++++++ 3 files changed, 40 insertions(+) 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 1d043472b..fa201b06f 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 @@ -141,6 +141,9 @@ LowLevelHttpResponse execute(final OutputWriter outputWriter) throws IOException Preconditions.checkArgument( contentLength == 0, "%s with non-zero content length is not supported", requestMethod); } + } else if ("DELETE".equals(connection.getRequestMethod())) { + connection.setDoOutput(true); + connection.setFixedLengthStreamingMode(0L); } // connect boolean successfulConnection = false; 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 2fba28b48..ba6f2f539 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 @@ -42,6 +42,10 @@ public class MockHttpURLConnection extends HttpURLConnection { /** Whether {@link #doOutput} was called. */ private boolean doOutputCalled; + /** Whether {@link #setFixedLengthStreamingMode(int)} was called. */ + private boolean setFixedLengthStreamingModeIntCalled = false; + /** Whether {@link #setFixedLengthStreamingMode(long)} was called. */ + private boolean setFixedLengthStreamingModeLongCalled = false; /** * Output stream or {@code null} to throw an {@link UnknownServiceException} when {@link @@ -205,4 +209,24 @@ public String getHeaderField(String name) { public int getChunkLength() { return chunkLength; } + + @Override + public void setFixedLengthStreamingMode(int contentLength) { + this.setFixedLengthStreamingModeIntCalled = true; + super.setFixedLengthStreamingMode(contentLength); + } + + @Override + public void setFixedLengthStreamingMode(long contentLength) { + this.setFixedLengthStreamingModeLongCalled = true; + super.setFixedLengthStreamingMode(contentLength); + } + + public boolean isSetFixedLengthStreamingModeIntCalled() { + return setFixedLengthStreamingModeIntCalled; + } + + public boolean isSetFixedLengthStreamingModeLongCalled() { + return setFixedLengthStreamingModeLongCalled; + } } 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 ae3606ca5..754ae8fad 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 @@ -233,4 +233,17 @@ public void testChunkedLengthNotSet() throws Exception { assertEquals(connection.getChunkLength(), -1); assertEquals("6", request.getRequestProperty("Content-Length")); } + + // see https://github.com/googleapis/google-http-java-client/issues/1471 for more details + @Test + public void testDeleteSetsContentLengthToZeroWithoutContent() throws Exception { + MockHttpURLConnection connection = new MockHttpURLConnection(new URL(HttpTesting.SIMPLE_URL)); + connection.setRequestMethod("DELETE"); + NetHttpRequest request = new NetHttpRequest(connection); + request.execute(); + + assertTrue(connection.doOutputCalled()); + assertTrue(connection.isSetFixedLengthStreamingModeLongCalled()); + assertFalse(connection.isSetFixedLengthStreamingModeIntCalled()); + } }