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

4.x: Return 500 when 204 with entity is sent from routing. #8357

Merged
merged 5 commits into from Feb 13, 2024

Conversation

tomas-langer
Copy link
Member

@tomas-langer tomas-langer commented Feb 9, 2024

Resolves #8348

Description

Fixed a problem when user returned 204 with an entity in JAX-RS.
With the default connector, the next (!) request would fail with Status code -1 (caused by the client ignoring the entity according to the specification, but then the connection was broken, and the next response started reading the previous entity).

The client could handle this more gracefully (i.e. close the connection, report the problem), but it is the default one in Java...

Updated solution:

  • if an attempt is done to write entity with 204 status, the status is changed to 500, and everything else carries out as usual (as entity is allowed with 500). The server logs an error explaining the problem.

@tomas-langer tomas-langer added webserver 4.x Version 4.x labels Feb 9, 2024
@tomas-langer tomas-langer self-assigned this Feb 9, 2024
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Feb 9, 2024
@tomas-langer tomas-langer added this to the 4.0.6 milestone Feb 9, 2024
romain-grecourt
romain-grecourt previously approved these changes Feb 9, 2024
danielkec
danielkec previously approved these changes Feb 12, 2024
@tomas-langer
Copy link
Member Author

I have validated the implementation in HttpUrlConnection - it expects that there is NOT an entity (it does some side check with Content-Length header, which is not used in our case, we just use chunked encoding).
So we must resolve this in Helidon.

@jbescos
Copy link
Member

jbescos commented Feb 13, 2024

I have validated the implementation in HttpUrlConnection - it expects that there is NOT an entity (it does some side check with Content-Length header, which is not used in our case, we just use chunked encoding). So we must resolve this in Helidon.

Find here my findings about this: jakartaee/rest#1199 (comment)

You would need to read the whole inputstream when you receive 204, 205 or 304 and ignore the entity if exists. This is in the ideal case that the stream is flushed with headers and entity together.

Copy link
Member

@jbescos jbescos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am reviewing this further and I am having questions. Lets try to clarify that.

Is Http1ServerResponse#nonEntityBytes always invoked?. I think it is. When there is 204, 205 or 304 with an entity we should log a warning and don't write the entity rather than failing with 500 error in my opinion.

Also the helidon client will not be able to handle chunked 204, 205 or 304 with entity. This is the comment wrote here: #8357 (comment)

@tomas-langer tomas-langer merged commit 509ec52 into helidon-io:main Feb 13, 2024
12 checks passed
@tomas-langer tomas-langer deleted the 8348-no-content branch February 13, 2024 11:07
hrstoyanov pushed a commit to hrstoyanov/helidon that referenced this pull request Feb 23, 2024
…o#8357)

* Return 500 when 204 with entity is sent from routing.
* Never send entity with status 204 (from our own code)
* Updated to include 205 and 304 statuses
tvallin pushed a commit to tvallin/helidon that referenced this pull request Feb 28, 2024
…o#8357)

* Return 500 when 204 with entity is sent from routing.
* Never send entity with status 204 (from our own code)
* Updated to include 205 and 304 statuses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x Version 4.x OCA Verified All contributors have signed the Oracle Contributor Agreement. webserver
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ResponseProcessing Failed to convert a response into an exception
4 participants