Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MJ: Fix #75 bug with openssl 3.0.10 #76

Merged
merged 2 commits into from
Aug 17, 2023
Merged

MJ: Fix #75 bug with openssl 3.0.10 #76

merged 2 commits into from
Aug 17, 2023

Conversation

BibMartin
Copy link
Contributor

This PR fixes the regex that breaks oscrypto with openssl 3.0.10 (#75 ).
In adding a + in the regex, we allow version numbers of the form D.D.DD.
Tested at home, it seems enough to solve the problem.

This would be the clean solution to snowpark-python's problem (snowflakedb/snowpark-python#992)

@vcunat
Copy link

vcunat commented Aug 10, 2023

Unfortunately, this does not seem to be the only place in need of fixes.

@vcunat
Copy link

vcunat commented Aug 10, 2023

Tests pass for me if I also add

--- a/oscrypto/_openssl/_libcrypto_ctypes.py
+++ b/oscrypto/_openssl/_libcrypto_ctypes.py
@@ -40,7 +40,7 @@ except (AttributeError):
 
 is_libressl = 'LibreSSL' in version_string
 
-version_match = re.search('\\b(\\d\\.\\d\\.\\d[a-z]*)\\b', version_string)
+version_match = re.search('\\b(\\d\\.\\d\\.\\d+[a-z]*)\\b', version_string)
 if not version_match:
     version_match = re.search('(?<=LibreSSL )(\\d\\.\\d(\\.\\d)?)\\b', version_string)
 if not version_match:

@rubickcz
Copy link

rubickcz commented Aug 14, 2023

@wbond Hello, this is blocker issue, can we get this merged soon please? 🙏

@wbond
Copy link
Owner

wbond commented Aug 16, 2023

Can you rebase this on master please @BibMartin? I've done a bit of CI work over the past couple of days, so things should be in a good place now.

@wbond wbond merged commit ebbc944 into wbond:master Aug 17, 2023
@wbond
Copy link
Owner

wbond commented Aug 17, 2023

I've merged this for now and added some fixes on master since I didn't have permission to push to your branch @BibMartin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants