Skip to content

Commit

Permalink
more comments + remove 'unchecked'
Browse files Browse the repository at this point in the history
  • Loading branch information
antonfirsov committed Dec 2, 2020
1 parent 94bc901 commit 67f53a0
Showing 1 changed file with 5 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -963,9 +963,9 @@ private async Task StartRequestAsync(WinHttpRequestState state)
// call of the response body data. WINHTTP_OPTION_RECEIVE_TIMEOUT sets a timeout on each
// lower layer winsock read.
// Timeout.InfiniteTimeSpan will be converted to uint.MaxValue milliseconds (~ 50 days).
// The result a of double->uint cast is unspecified and may differ on ARM.
// The result a of double->uint cast is unspecified and may differ on ARM, returning 0 instead of uint.MaxValue.
// To handle Timeout.InfiniteTimespan correctly, we need to cast to Int32 first.
uint optionData = unchecked((uint)(int)_receiveDataTimeout.TotalMilliseconds);
uint optionData = (uint)(int)_receiveDataTimeout.TotalMilliseconds;

This comment has been minimized.

Copy link
@jkotas

jkotas Dec 2, 2020

Member

Are there other corner cases where this can overflow in unspecified way? E.g. Can TotalMilliseconds be be 100 days?

This comment has been minimized.

Copy link
@antonfirsov

antonfirsov Dec 2, 2020

Author Member

There is a guard in each individual property setter that will throw an exception for TotalMilliseconds > int.MaxValue.

Later, when we forward the property values to WinHttp, we do unchecked casts everywhere in the WinHttpHandler library.

SetWinHttpOption(state.RequestHandle, Interop.WinHttp.WINHTTP_OPTION_RECEIVE_TIMEOUT, ref optionData);

HttpResponseMessage responseMessage =
Expand Down Expand Up @@ -1010,10 +1010,10 @@ private unsafe void SetTcpKeepalive(SafeWinHttpHandle sessionHandle)
onoff = 1,

// Timeout.InfiniteTimeSpan will be converted to uint.MaxValue milliseconds (~ 50 days)
// The result a of double->uint cast is unspecified and may differ on ARM.
// The result a of double->uint cast is unspecified and may differ on ARM, returning 0 instead of uint.MaxValue.
// To handle Timeout.InfiniteTimespan correctly, we need to cast to Int32 first.
keepaliveinterval = unchecked((uint)(int)_tcpKeepAliveInterval.TotalMilliseconds),
keepalivetime = unchecked((uint)(int)_tcpKeepAliveTime.TotalMilliseconds)
keepaliveinterval = (uint)(int)_tcpKeepAliveInterval.TotalMilliseconds,
keepalivetime = (uint)(int)_tcpKeepAliveTime.TotalMilliseconds
};

SetWinHttpOption(
Expand Down

0 comments on commit 67f53a0

Please sign in to comment.