Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[BUG][Java][client] Not possible to send payloads on DELETE requests #3719

Closed
3 of 6 tasks
bkabrda opened this issue Aug 21, 2019 · 2 comments · Fixed by #3703
Closed
3 of 6 tasks

[BUG][Java][client] Not possible to send payloads on DELETE requests #3719

bkabrda opened this issue Aug 21, 2019 · 2 comments · Fixed by #3703

Comments

@bkabrda
Copy link
Contributor

bkabrda commented Aug 21, 2019

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • What's the version of OpenAPI Generator used?
  • Have you search for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Bounty to sponsor the fix (example)
Description

I've just hit an issue with Jersey client which doesn't make it possible at all to send payloads with DELETE requests. While the HTTP spec says that semantics of DELETE payload is undefined, there is nothing that strictly prohibits sending payloads with DELETE requests (and I'm actually generating a client for API that needs payloads on DELETE :/ ).

I think this should be allowed in the following way:

  • If the body passed in is null, no payload will be sent (not even empty object).
  • If the body passed in is not null, payload will be sent in the same way as it is for e.g. POST and other methods.

Does that sound like a good compromise?

openapi-generator version

4.1.0 (but seems to be problem in all versions AFAICS)

OpenAPI declaration file content or url
Command line used for generation
Steps to reproduce

Generate a jersey2 client from OpenAPI spec that defines a json payload on DELETE request, try to run invoke the method for deletion with a non-null payload - no payload is sent.

Related issues/PRs

#3276

Suggest a fix

I'm using the following patch to make this work. If it looks ok, I can submit it as a PR (probably after #3703 is merged, since it touches same parts of code and my patch might need to change depending on the final changes in #3703).

diff --git a/Java/libraries/jersey2/ApiClient.mustache b/Java/libraries/jersey2/ApiClient.mustache
index ab6a4a1353..579dc1ae1c 100644
--- a/Java/libraries/jersey2/ApiClient.mustache
+++ b/Java/libraries/jersey2/ApiClient.mustache
@@ -716,7 +716,12 @@ public class ApiClient {
       } else if ("PUT".equals(method)) {
         response = invocationBuilder.put(entity);
       } else if ("DELETE".equals(method)) {
-        response = invocationBuilder.delete();
+        // if body is null or Object, no entity was passed in => call delete without payload
+        if (body == null || body.getClass() == java.lang.Object.class) {
+          response = invocationBuilder.delete();
+        } else {
+          response = invocationBuilder.method("DELETE", entity);
+        }
       } else if ("PATCH".equals(method)) {
         response = invocationBuilder.method("PATCH", entity);
       } else if ("HEAD".equals(method)) {
@@ -776,6 +781,8 @@ public class ApiClient {
     clientConfig.register(json);
     clientConfig.register(JacksonFeature.class);
     clientConfig.property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true);
+    // turn off compliance validation to be able to send payloads with DELETE calls
+    clientConfig.property(ClientProperties.SUPPRESS_HTTP_COMPLIANCE_VALIDATION, true);
     if (debugging) {
 {{^supportJava6}}
       clientConfig.register(new LoggingFeature(java.util.logging.Logger.getLogger(LoggingFeature.DEFAULT_LOGGER_NAME), java.util.logging.Level.INFO, LoggingFeature.Verbosity.PAYLOAD_ANY, 1024*50 /* Log payloads up to 50K */));
@@ -786,6 +793,9 @@ public class ApiClient {
 {{#supportJava6}}
       clientConfig.register(new LoggingFilter(java.util.logging.Logger.getLogger(LoggingFilter.class.getName()), true));
 {{/supportJava6}}
+    } else {
+      // suppress warnings for payloads with DELETE calls
+      java.util.logging.Logger.getLogger("org.glassfish.jersey").setLevel(java.util.logging.Level.SEVERE);
     }
     performAdditionalClientConfiguration(clientConfig);
     return ClientBuilder.newClient(clientConfig);
@jmini
Copy link
Member

jmini commented Aug 21, 2019

My test suite (see #689 (comment)) did not catch this case, probably because of mock-server/mockserver#663


Just using invocationBuilder.method("DELETE", entity); is sufficient in my opinion after my fixes in #3703


// turn off compliance validation to be able to send payloads with DELETE calls
clientConfig.property(ClientProperties.SUPPRESS_HTTP_COMPLIANCE_VALIDATION, true);

Is necessary to not have this stacktrace:

java.lang.IllegalStateException: Entity must be null for http method DELETE.
	at org.glassfish.jersey.client.JerseyInvocation.validateHttpMethodAndEntity(JerseyInvocation.java:157)
	at org.glassfish.jersey.client.JerseyInvocation.<init>(JerseyInvocation.java:112)
	at org.glassfish.jersey.client.JerseyInvocation.<init>(JerseyInvocation.java:108)
	at org.glassfish.jersey.client.JerseyInvocation.<init>(JerseyInvocation.java:99)
	at org.glassfish.jersey.client.JerseyInvocation$Builder.method(JerseyInvocation.java:445)

// suppress warnings for payloads with DELETE calls:
java.util.logging.Logger.getLogger("org.glassfish.jersey.client").setLevel(java.util.logging.Level.SEVERE);

Is necessary to remove

AM org.glassfish.jersey.client.JerseyInvocation validateHttpMethodAndEntity
WARNING: Entity must be null for http method DELETE.

jmini pushed a commit to jmini/openapi-generator that referenced this issue Aug 21, 2019
jmini added a commit to jmini/openapi-generator that referenced this issue Aug 21, 2019
@jmini
Copy link
Member

jmini commented Aug 21, 2019

As discussed with @bkabrda, I am integrating this patch in #3703
354b62b


resteasy has the same issue also fixed with #3703

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
2 participants