Closed graebm closed 2 months ago
Attention: Patch coverage is 66.66667%
with 2 lines
in your changes missing coverage. Please review.
Project coverage is 80.22%. Comparing base (
9072f86
) to head (5720360
).
Files | Patch % | Lines |
---|---|---|
source/channel.c | 66.66% | 1 Missing :warning: |
source/s2n/s2n_tls_channel_handler.c | 50.00% | 1 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Issue:
A C++ SDK user found a memory leak in the Windows TLS channel-handler code and submitted the following PR: https://github.com/awslabs/aws-c-io/pull/651
In evaluating this 2-line fix, I noticed that it sat in the middle of some (preexisting) sloppy error-handling.
Description of changes:
FreeContextBuffer()
.FreeContextBuffer()
after calling toAcceptSecurityContext()
/InitializeSecurityContextA()
. Previously we'd only free the buffer in certain circumstances, and were probably leaking.aws_io_message
. Officially declare that message allocations can't fail.NULL
. They deal with OOM by immediately terminating the program. This was done because error-handling code for allocation failure makes everything so much more complex, and it's seldom tested. We tried for years to handle it. It's not worth the effort.aws_io_message
after checking its capacityAWS_ASSERT()
about buffers being large enough withAWS_FATAL_ASSERT()
. Similarly, add anAWS_FATAL_ASSERT()
in one place that never checked or asserted before doing the copy.Co-authored-by: @normanade
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.