Skip to content

Commit

Permalink
fix: update NetHttpRequest to prevent silent retry of DELETE requests (
Browse files Browse the repository at this point in the history
…#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
  • Loading branch information
BenWhitehead committed Oct 7, 2021
1 parent 135bd25 commit 57ef11a
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 0 deletions.
Expand Up @@ -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;
Expand Down
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}
Expand Up @@ -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());
}
}

0 comments on commit 57ef11a

Please sign in to comment.