Revert "Replace CONF's internal representation with something more typesafe" #1986
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…pesafe" This reverts commit cc932cf.
samuel40791765
force-pushed
the
revert-conf-change
branch
from
9b31a61
to
6fadb50
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1986 +/- ##
==========================================
- Coverage 78.86% 78.86% -0.01%
==========================================
Files 593 593
Lines 101933 101936 +3
Branches 14450 14453 +3
==========================================
+ Hits 80386 80388 +2
Misses 20903 20903
- Partials 644 645 +1 ☔ View full report in Codecov by Sentry. |
WillChilds-Klein
approved these changes
Nov 14, 2024
justsmth
approved these changes
Nov 14, 2024
samuel40791765
added a commit
that referenced
this pull request
Nov 16, 2024
This extends upon #1986. We do have all the necessary support for "LHASH", it's just been renamed and made internal by upstream: b8441ac. Ruby's CONF module happens to have a dependency on a couple of LHASH macros OpenSSL created and the non-opaqueness of CONF. Although we aren't likely to be supporting CONF files for the time being, this change seems to reasonable for enough us to maintain so we can get a better build/patch for Ruby. We still fail most CONF related tests in Ruby, so we'll have to work around those. But this commit lets us gracefully build with the underlying C code for Ruby CONF without any AWS-LC specific logic. ### Testing: Verified that this passes the build for Ruby By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts commit cc932cf.
Issues:
Addresses
CryptoAlg-2449Description of changes:
OpenSSL's historically made the
CONFstruct non-opaque. This hasn't changed in OpenSSL 3 either although there is documentation that indicates they wish to do so in the long run. It's likely going to be some time before this is actually done, but in the meantime Ruby has a dependency onCONF's internals.This commit is reverting some of the original upstream work done that changes the internal variables of
CONF. There will be a follow up PR to make the structure non-opaque.Call-outs:
N/A
Testing:
Original tests
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.