-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Make the Content.Chunk.isTerminal()
contract stricter
#8994
Conversation
Content.Chunk.isTerminal()
contract stricter
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the change. But it raises a question about calling succeeded twice?
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ChunksContentSource.java
Show resolved
Hide resolved
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ByteBufferContentSource.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java
Outdated
Show resolved
Hide resolved
@gregw I answered your question w.r.t succeeding twice the callback in your code comment. In a nutshell, it's never succeeded twice and my original change introduced a bug which made it never be succeeded in certain cases. This is now fixed with some comments explaining the somewhat baroque logic. |
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ChunksContentSource.java
Show resolved
Hide resolved
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should rely on Chunk.from
to correctly choose from EOF
, EMPTY
or a new chunk, rather than have that same logic scatter around the code base.
jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartTest.java
Show resolved
Hide resolved
...t/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpReceiverOverHTTP2.java
Outdated
Show resolved
Hide resolved
...-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java
Outdated
Show resolved
Hide resolved
...t/src/main/java/org/eclipse/jetty/http3/client/transport/internal/HttpReceiverOverHTTP3.java
Outdated
Show resolved
Hide resolved
...-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ByteBufferContentSource.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/AsyncContentSourceTest.java
Show resolved
Hide resolved
jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MockHttpStream.java
Show resolved
Hide resolved
jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/MockHttpStream.java
Outdated
Show resolved
Hide resolved
...etty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletMultiPartFormData.java
Outdated
Show resolved
Hide resolved
84eddd6
to
cc5dedc
Compare
… Chunk.isTerminal() contract stricter Signed-off-by: Ludovic Orban <lorban@bitronix.be>
cc5dedc
to
f510986
Compare
Make the
Content.Chunk.isTerminal()
contract stricter by explicitly disallowing the creation of new last chunks with an emptyByteBuffer
.Also replace non-last empty chunks with EMPTY where it makes sense.
Closes #8993