eclipse-ee4j / eclipse-ee4j/jersey

Http compliance of Jersey Microprofile RestClient - Issue for DELETE request with Entity

Open
#5,253 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
730
Forks
382
PR merge metrics
No merged PRs in 30d

Description

The Jersey Microprofile RestClient only partially takes the property `ClientProperties.SUPPRESS_HTTP_COMPLIANCE_VALIDATION` into account.

Using this flag enables for instance to send a PUT request without any payload.

But it seems that the jersey's Microprofile implementation of the RestClient has hardcoded discarding the Entity body in case of a `GET` and a `DELETE` request. (see `org.glassfish.jersey.microprofile.restclient.MethodModel::synchronousCall()` for instance)
The compliance property flag is ignored although there are some discussion around the validity (or not) of providing an entity payload along DELETE requests (https://stackoverflow.com/questions/299628/is-an-entity-body-allowed-for-an-http-delete-request#299696)
Seems like the RFC says that an Entity **should not** be provided along a DELETE request as it has no defined semantic. But is not strictly forbidden. (https://www.rfc-editor.org/rfc/rfc9110.html#name-delete)

EDIT :
I could trace the origin of the `SUPPRESS_HTTP_COMPLIANCE_VALIDATION` property that was introduced to workaround an issue of `HttpUrlConnection` that was preventing to add a payload to `DELETE` requests :
- Original issue https://github.com/javaee/jersey/issues/1983
- JDK fix in java 8 of HttpUrlConnection https://bugs.openjdk.org/browse/JDK-7157360)
- Addition of SUPPRESS_HTTP_COMPLIANCE_VALIDATION flag https://github.com/javaee/jersey/commit/f56211a1d6c93215729b6884d530f02edd1b64f0

In a general manner I don't see the point of having jersey client enforcing HTTP compliance of requests. This should be the work of the underlying `HttpUrlConnection` if needed.

**As a fix I would suggest to remove completely this flag and the compliance checks**

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.