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.

Sign up for GitHub

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

Jump to bottom

Exception occours if client sends reset frame while the server is sending headers #6906

Closed
qwangseu opened this issue Jun 27, 2017 · 7 comments
Closed

Exception occours if client sends reset frame while the server is sending headers #6906

qwangseu opened this issue Jun 27, 2017 · 7 comments
Assignees
@Scottmitch
Labels
defect
Milestone
4.1.13.Final

Comments

@qwangseu
Copy link

qwangseu commented Jun 27, 2017 •
edited
Loading

when I use netty http2 to build a server, exception will occour if the client sents a reset frame to the server while the server is sending headers (netty version is 4.1.8.final),

io.netty.handler.codec.http2.Http2Exception: Request stream 2414839 is not correct for server connection
   at io.netty.handler.codec.http2.Http2Exception.connectionError(Http2Exception.java:85)
   at io.netty.handler.codec.http2.DefaultHttp2Connection$DefaultEndpoint.checkNewStreamAllowed(DefaultHttp2Connection.java:1095)
   at io.netty.handler.codec.http2.DefaultHttp2Connection$DefaultEndpoint.createStream(DefaultHttp2Connection.java:942)
   at io.netty.handler.codec.http2.DefaultHttp2Connection$DefaultEndpoint.createStream(DefaultHttp2Connection.java:960)
   at io.netty.handler.codec.http2.DefaultHttp2Connection$DefaultEndpoint.createStream(DefaultHttp2Connection.java:868)
   at io.netty.handler.codec.http2.DefaultHttp2ConnectionEncoder.writeHeaders(DefaultHttp2ConnectionEncoder.java:158)
   at io.netty.handler.codec.http2.DefaultHttp2ConnectionEncoder.writeHeaders(DefaultHttp2ConnectionEncoder.java:148)
   at com.c.t.h2c.netty.NettyServerHandler.sendResponseHeaders(NettyServerHandler.java:293)
   at com.c.t.h2c.netty.NettyServerHandler.write(NettyServerHandler.java:253)
   at io.netty.channel.AbstractChannelHandlerContext.invokeWrite0(AbstractChannelHandlerContext.java:739)
   at io.netty.channel.AbstractChannelHandlerContext.invokeWrite(AbstractChannelHutor$5.run(SingleThreadEventExecutor.java:858)
   at io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run(DefaultThreadFactory.java:144)
   at java.lang.Thread.run(Thread.java:745)

https://github.com/netty/netty/blob/4.1/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java#L156
when the server receives reset frame it will remove the stream from the streamMap, but it will create the stream when it writes headers if it finds the stream is null。

https://github.com/netty/netty/blob/4.1/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java#L687
here the netty will check whether the streamId is even, but the streamId from upstream is always odd.So is there any method to resolve this problem?
@twjitm
Copy link

twjitm commented Jun 27, 2017

so do I, How are you to solve it?

@normanmaurer
Copy link
Member

@ejona86 @nmittler @Scottmitch PTAL...

@Scottmitch
Copy link
Member

It looks like we are sending GO_AWAY in this scenario ... can you confirm this @qwangseu? If so this is a bug.

@ejona86 ejona86 mentioned this issue Jun 28, 2017
Closed
@Zeymo
Copy link

Zeymo commented Jun 28, 2017 •
edited
Loading

The same to me . I occurs when http2-client send RSTFrame(maybe time-out) as @qwangseu say

It seems http2-server must maintain stream status (like create,done) , when receive RSTFrame server must change status locked to done and don't remove from map until server write trailer(or not?) which alway responsed according to the status ,then remove stream .This delete action can and must only can do in server side after write trailer(or not?) rather the dependent client any control frame
/cc @Scottmitch @ejona86

@qwangseu
Copy link
Author

The client sends a request to the server but the server dose not response immediately，so the client sends a reset frame when timeout, if then the server response the request, it will trigger the exception because the stream has been deleted from the streamMap and the netty will try to create a new stream。
https://github.com/netty/netty/blob/4.1/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java#L155

@Scottmitch
Copy link
Member

@qwangseu - yes I understand the sequence of events ... I was just confirming that the issue is there is a GO_AWAY being sent.

@Scottmitch Scottmitch self-assigned this Jun 28, 2017
@Scottmitch Scottmitch added this to the 4.1.13.Final milestone Jun 28, 2017
@Scottmitch Scottmitch mentioned this issue Jun 28, 2017
Closed
@qwangseu
Copy link
Author

It seems like that the modification can resolve this problem.

Scottmitch added a commit to Scottmitch/netty that referenced this issue Jun 28, 2017
@Scottmitch
DefaultHttp2ConnectionEncoder#writeHeaders shouldn't send GO_AWAY if …
2c232cb 
…stream is closed

Motivation:
DefaultHttp2ConnectionEncoder#writeHeaders attempts to find a stream object, and if one doesn't exist it tries to create one. However in the event that the local endpoint has received a RST_STREAM frame before writing the response headers we attempt to create a stream. Since this stream ID is for the incorrect endpoint we then generate a GO_AWAY for what appears to be a protocol error, but can instead be failed locally.

Modifications:
- Just fail the local promise in the above situation instead of sending a GO_AWAY

Result:
Less severe consequences if the server asynchronously sends headers after a RST_STREAM has been received.
Fixes netty#6906.
@wmorgan wmorgan mentioned this issue Jun 29, 2017
Closed
liuzhengyang pushed a commit to liuzhengyang/netty that referenced this issue Sep 10, 2017
@Scottmitch
DefaultHttp2ConnectionEncoder#writeHeaders shouldn't send GO_AWAY if …
6d851ec 
…stream is closed

Motivation:
DefaultHttp2ConnectionEncoder#writeHeaders attempts to find a stream object, and if one doesn't exist it tries to create one. However in the event that the local endpoint has received a RST_STREAM frame before writing the response headers we attempt to create a stream. Since this stream ID is for the incorrect endpoint we then generate a GO_AWAY for what appears to be a protocol error, but can instead be failed locally.

Modifications:
- Just fail the local promise in the above situation instead of sending a GO_AWAY

Result:
Less severe consequences if the server asynchronously sends headers after a RST_STREAM has been received.
Fixes netty#6906.
kiril-me pushed a commit to kiril-me/netty that referenced this issue Feb 28, 2018
@Scottmitch @kiril-me
DefaultHttp2ConnectionEncoder#writeHeaders shouldn't send GO_AWAY if …
87f86e2 
…stream is closed

Motivation:
DefaultHttp2ConnectionEncoder#writeHeaders attempts to find a stream object, and if one doesn't exist it tries to create one. However in the event that the local endpoint has received a RST_STREAM frame before writing the response headers we attempt to create a stream. Since this stream ID is for the incorrect endpoint we then generate a GO_AWAY for what appears to be a protocol error, but can instead be failed locally.

Modifications:
- Just fail the local promise in the above situation instead of sending a GO_AWAY

Result:
Less severe consequences if the server asynchronously sends headers after a RST_STREAM has been received.
Fixes netty#6906.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Scottmitch Scottmitch

Labels
defect
Projects
None yet
Milestone
4.1.13.Final
Development

Successfully merging a pull request may close this issue.

DefaultHttp2ConnectionEncoder#writeHeaders shouldn't send GO_AWAY if … Scottmitch/netty
5 participants
@normanmaurer @Zeymo @Scottmitch @qwangseu @twjitm