Do not throw from HttpChannelState.read() method #11369
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes #11363 by ensuring that read never throws, but instead returns an Error chunk.
joakime
reviewed
Feb 2, 2024
jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java
Show resolved
Hide resolved
joakime
changed the title
sbordet
requested changes
Feb 5, 2024
| } | ||
| chunk = stream.read(); | ||
| } | ||
| catch (Throwable t) |
There was a problem hiding this comment.
I would extend the try to the whole method.
The lines below should not throw, but neither the above should have, so I'd extend the scope of the try to the whole method to state the intention that the whole method must not throw, not only certain parts of the method.
sbordet
requested changes
Feb 5, 2024
| @@ -867,31 +867,38 @@ public long getLength() | |||
| @Override | |||
| public Content.Chunk read() | |||
| { | |||
| HttpStream stream; | |||
| try (AutoLock ignored = _lock.lock()) | |||
| Content.Chunk chunk; | |||
There was a problem hiding this comment.
This line can be moved down now, as it is only used inside the try.
sbordet
approved these changes
Feb 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #11363 by ensuring that read never throws, but instead returns an Error chunk.