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

HeaderDelimitedMessageHandler may write only header but not the body when the request is cancelled #55

Closed
IlyaBiryukov opened this issue Aug 30, 2017 · 1 comment
Assignees
Labels
Milestone

Comments

@IlyaBiryukov
Copy link
Contributor

Repro:
Invoke a method on a server and then cancel the cancellation token passed to the method. Depending on your luck, the cancellation may happen so that the header is already written to the underlying stream here, but the content is not, and it got cancelled here. Sending another message on that JsonRPC makes it send the header and the content. The receiving end will get the header from the first (cancelled) message, and then the header and the content from the next message. It'll try to use the first header to read the whole message, and will read malformed json (it will contain the header and part of the body of the second message). Then it'll close the stream.

Expected:
Sending a JSON RPC message by the message handler should be atomic. We should write header and content together, or don't write anything at all.

@IlyaBiryukov IlyaBiryukov self-assigned this Aug 30, 2017
@IlyaBiryukov IlyaBiryukov added this to the VS 15.3 milestone Aug 30, 2017
@AArnott AArnott modified the milestones: v1.1, VS 15.3 Aug 30, 2017
@AArnott
Copy link
Member

AArnott commented Aug 30, 2017

@IlyaBiryukov VS 15.3 is equivalent to the existing v1.1 milestone. So I've moved to that one.

IlyaBiryukov pushed a commit that referenced this issue Aug 31, 2017
HeaderDelimitedMessageHandler accepts cancellation token from the call
to JsonRpc.InvokeAsync. If the caller cancels the token at the right
moment, it may happen when the header is already written to the stream,
but the message content is not. If this happens, on the next call the
reciever will get a malformed message - a header from the first message,
a header and content from the next message. This will cause JsonRpc to
error out, close the stream, and cancel the next method.

This fix makes sure that writing in HeaderDelimiteMessageHandler occurs
atomically and cannot be interrupted by cancelled cancellation token.

Fix for #55.
@AArnott AArnott closed this as completed Sep 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants