OctopusDeploy / Halibut

| Public | A secure communication stack for .NET using JSON-RPC over SSL.
Other
12 stars 44 forks source link

HTTPProxy client no longer requires all headers to arrive together (async only) #468

Closed LukeButters closed 1 year ago

LukeButters commented 1 year ago

Background

Fixes a bug in HTTPProxyClient in which it would leave parts of the HTTP header in the stream, which would then be passed to the SSL stream which would throw.

The problem is the existing implementation would assume that it is done getting bytes when the network stream happened to have no bytes available at the time it checked. This doesn't make any sense since bytes of the HTTP header can arrive with any timing. When a delay occurs, the HTTPProxyClient would assume it has no more bytes and proceed to checking what it has and passing the stream to the SslStream.

The fix is to simply keep reading until the end of the HTTP headers is signalled which per the spec (citation needed) is when an empty line is received ie when the last 4 bytes are \r\n\r\n.

Related to https://github.com/OctopusDeploy/Issues/issues/8266

Results

The Halibut http proxy test library is updated to have the ability to trigger the exact issue. When proxy tests are run with that enabled, they fail. With the fix they pass. Note that only latest client and latest service both in async has the fix and so this new feature of the HTTP proxy is only used in that case.

How to review this PR

Quality :heavy_check_mark:

Pre-requisites

nathanwoctopusdeploy commented 1 year ago

[sc-57553]

shortcut-integration[bot] commented 1 year ago

This pull request has been linked to Shortcut Story #57553: HttpProxyClient may not read all HTTP headers..