-
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
Implements client-side handling of RST_STREAM(NO_ERROR) in HTTP/2. #11054
base: jetty-12.0.x
Are you sure you want to change the base?
Conversation
HTTP/3 could have the same handling, but apparently there is no way to receive a RESET_STREAM or STOP_SENDING frame in Quiche. Now when receiving a RST_STREAM(NO_ERROR), it is not treated as a failure. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
…e completed successfully. Also, disposing the stream if the request failed, as we cannot rely on the RST_STREAM sent by the server. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
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.
just some initial comments/questions at this stage.
@@ -775,6 +775,12 @@ public void close() | |||
} | |||
} | |||
|
|||
public boolean dispose() |
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.
javadoc please.
Does it need to be public?
@@ -494,16 +494,17 @@ public void succeeded() | |||
{ | |||
if (LOG.isDebugEnabled()) | |||
LOG.debug("HTTP3 Response #{}/{}: unconsumed request content, resetting stream", stream.getId(), Integer.toHexString(stream.getSession().hashCode())); | |||
stream.reset(HTTP3ErrorCode.REQUEST_CANCELLED_ERROR.code(), new IOException("unconsumed content")); | |||
stream.reset(HTTP3ErrorCode.NO_ERROR.code(), new IOException("unconsumed content")); |
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.
why not use CONTENT_NOT_CONSUMED
?
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.
Server side looks OK... but some questions.
Will need to work out why it is hanging in CI
@sbordet how about in this release cycle we only change the server side to send the reset(NO_ERROR) and leave the client side as is. |
rebase against 12.1.x |
HTTP/3 could have the same handling, but apparently there is no way to receive a RESET_STREAM or STOP_SENDING frame in Quiche.
Now when receiving a RST_STREAM(NO_ERROR), it is not treated as a failure.