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

bug(NetHttp): delete requests are silently retired if a socket is reset before getting the response #1471

Closed
BenWhitehead opened this issue Oct 5, 2021 · 0 comments · Fixed by #1472
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@BenWhitehead
Copy link
Contributor

HttpURLConnection will silently retry DELETE requests. This behavior is similar to other existing JDK bugs (JDK-6382788, JDK-6427251). google-http-java-client already contains a workaround (NetHttpRequest.java#L108-L112) for those requests which have a body, but does not account for DELETE with an empty body.

For Google Cloud Storage, while developing a conformance test suite for retry handling I ran into this bug and will submit a fix.

Environment details

  1. NetHttp
  2. OS type and version: Debian Linux
  3. Java version:
    openjdk version "1.8.0_292"
    OpenJDK Runtime Environment (AdoptOpenJDK)(build 1.8.0_292-b10)
    OpenJDK 64-Bit Server VM (AdoptOpenJDK)(build 25.292-b10, mixed mode)
    
  4. google-http-client version(s): 1.40.0 and older

Steps to reproduce

  1. start a "server" which closes the connection without sending a response
    socat tcp4-listen:8080,fork,reuseaddr system:"sleep 1"\!\!stdout
    
  2. run the following code and observe two requests in the server output
    URL url = new URL("http://localhost:8080");
    HttpURLConnection conn = (HttpURLConnection) url.openConnection();
    conn.setRequestMethod("DELETE");
    
    conn.getResponseCode();
    

Code example

See rerpo step 3

Stack trace

No relevant stack trace

External references such as API reference guides

@BenWhitehead BenWhitehead self-assigned this Oct 5, 2021
@chanseokoh chanseokoh added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Oct 6, 2021
gcf-merge-on-green bot pushed a commit that referenced this issue Oct 7, 2021
…#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
gcf-merge-on-green bot pushed a commit that referenced this issue Oct 7, 2021
🤖 I have created a release \*beep\* \*boop\*
---
### [1.40.1](https://www.github.com/googleapis/google-http-java-client/compare/v1.40.0...v1.40.1) (2021-10-07)


### Bug Fixes

* add used packages to OSGI manifest again ([#1439](https://www.github.com/googleapis/google-http-java-client/issues/1439)) ([#1440](https://www.github.com/googleapis/google-http-java-client/issues/1440)) ([59fc8b0](https://www.github.com/googleapis/google-http-java-client/commit/59fc8b03e5518864c60ce4dd47664e8935da343b))
* update NetHttpRequest to prevent silent retry of DELETE requests ([#1472](https://www.github.com/googleapis/google-http-java-client/issues/1472)) ([57ef11a](https://www.github.com/googleapis/google-http-java-client/commit/57ef11a0e1904bb932e5493a30f0a2ca2a2798cf)), closes [#1471](https://www.github.com/googleapis/google-http-java-client/issues/1471)


### Dependencies

* update dependency com.fasterxml.jackson.core:jackson-core to v2.12.5 ([#1437](https://www.github.com/googleapis/google-http-java-client/issues/1437)) ([0ce8467](https://www.github.com/googleapis/google-http-java-client/commit/0ce84676bfbe4cc8e237d5e33dfaa532b13e798c))
* update dependency com.fasterxml.jackson.core:jackson-core to v2.13.0 ([#1469](https://www.github.com/googleapis/google-http-java-client/issues/1469)) ([7d9a042](https://www.github.com/googleapis/google-http-java-client/commit/7d9a042110b8879b592d7e80bd73e77c7a84d8b7))
* update dependency com.google.protobuf:protobuf-java to v3.18.0 ([#1454](https://www.github.com/googleapis/google-http-java-client/issues/1454)) ([cc63e41](https://www.github.com/googleapis/google-http-java-client/commit/cc63e41fac8295c7fea751191a6fe9537c1f70e3))
* update dependency com.google.protobuf:protobuf-java to v3.18.1 ([#1470](https://www.github.com/googleapis/google-http-java-client/issues/1470)) ([c36637a](https://www.github.com/googleapis/google-http-java-client/commit/c36637acbca536992349970664026cf145ad8964))
* update dependency com.puppycrawl.tools:checkstyle to v9 ([#1441](https://www.github.com/googleapis/google-http-java-client/issues/1441)) ([a95cd97](https://www.github.com/googleapis/google-http-java-client/commit/a95cd9717fc8accd80252b12357971cb71887d90))
* update project.appengine.version to v1.9.91 ([#1287](https://www.github.com/googleapis/google-http-java-client/issues/1287)) ([09ebf8d](https://www.github.com/googleapis/google-http-java-client/commit/09ebf8d7e3860f2b94a6fea0ef134c93904d4ed1))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants