Populate DataHash key size in SslConnectionInfo#4169
Conversation
shrutigarg
force-pushed
the
ssl_new1
branch
3 times, most recently
from
25133a1 to
a8716e4
Compare
There was a problem hiding this comment.
If we're no longer returning an int, any reason not to change the out parameter to be the return value?
There was a problem hiding this comment.
This is a PAL function, so the signature needs to match Windows. (Windows currently has a void return). If we did this we would need to update the Windows PAL function and the call site. (Not saying that we can't do that though...)
There was a problem hiding this comment.
That's why I like PAL interfaces vs PAL static implicit call conventions, personally.
There was a problem hiding this comment.
We should add a null check for this new parameter before dereferencing it.
There was a problem hiding this comment.
I don't see this check in the latest code. All that is needed is to change line 459 from
if (!ssl || !dataCipherAlg || !keyExchangeAlg || !dataHashAlg || !dataKeySize)
to
if (!ssl || !dataCipherAlg || !keyExchangeAlg || !dataHashAlg || !dataKeySize || !hashKeySize)
|
LGTM |
shrutigarg
force-pushed
the
ssl_new1
branch
4 times, most recently
from
8c10ea0 to
12e22c7
Compare
|
@dotnet-bot test this please |
shrutigarg
force-pushed
the
ssl_new1
branch
5 times, most recently
from
474ec26 to
92ec4d8
Compare
Populate DataHash key size in SslConnectionInfo
7 participants
this is an update on PR #3916 on top of Eric's shimming changes.
It includes one extra bug fix of updating signature change of QueryContextConnectionInfo.