HTTP/2: implement 100-continue - #2106
Conversation
|
|
||
| protected override void OnReadStarting() | ||
| { | ||
| TryProduceContinue(); |
There was a problem hiding this comment.
It looks to me like the 100-continue response is being sent every time the body of an "Expect: 100-continue" request is read.
Shouldn't this instead behave like HTTP/1.1 where we do not send the 100-continue response if the request body is already available when the app starts reading it?
5fd8067 to
dd5718c
Compare
dd5718c to
ef2d60f
Compare
|
Forgot to comment that I've updated this to have the same behavior as we have with HTTP/1.x. |
|
Ping on this @halter73 . If it looks good to you, I'll get it merged. Somewhat deferring to you here though since I haven't had a chance to catch up on this code and I don't want to hold this PR open any longer :). |
| { | ||
| var awaitable = _context.RequestBodyPipe.Reader.ReadAsync(); | ||
|
|
||
| if (!awaitable.IsCompleted) |
There was a problem hiding this comment.
@anurse Ideally we would only check this once per-request like we do for HTTP/1.1 instead of per-read like is done here. This might not be so easy with multiple streams per connection. I'll look into merging this tomorrow.
There was a problem hiding this comment.
You mean checking _send100Continue I assume?
|
@anurse Can you review? I updated this to check _send100Continue less. This should also prevent the PR from hurting HTTP/1.1 perf. |
4 participants
No description provided.