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

BUG: use uuid4 instead of md5 for fake column in cross merge #48029

Merged
merged 3 commits into from
Aug 11, 2022

Conversation

phofl
Copy link
Member

@phofl phofl commented Aug 10, 2022

cc @mroeschke Any idea how to test this? Did not add it for the test, do you think thats necessary?

@phofl phofl added the Reshaping Concat, Merge/Join, Stack/Unstack, Explode label Aug 10, 2022
@mroeschke mroeschke added this to the 1.5 milestone Aug 11, 2022
@mroeschke mroeschke changed the title BUG: use usedforsecurity for md5 in cross merge BUG: use uuid4 instead of md5 for fake column in cross merge Aug 11, 2022
@mroeschke
Copy link
Member

@szelenka, could you confirm using uuid4 would be a sufficient replacement for this use case (if you can test locally)?

From what I am seeing online, I don't see a really a convenient way of testing this in our CI system

@szelenka
Copy link

@mroeschke yes this seems to work okay:

>>> ssl.FIPS_mode_set(0)
>>> ssl.FIPS_mode_set(1)

# Old method
>>> cross_col = f"_cross_{hashlib.md5().hexdigest()}"
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: [digital envelope routines] disabled for fips

# New method
>>> cross_col = f"_cross_{uuid.uuid4()}"
>>> cross_col
'_cross_0a42418d-46db-40c9-8cbf-72d672d9863a'

@mroeschke mroeschke merged commit 926b9ce into pandas-dev:main Aug 11, 2022
@mroeschke
Copy link
Member

Thanks @phofl and @szelenka for confirming.

@phofl phofl deleted the 48024 branch August 11, 2022 10:30
noatamir pushed a commit to noatamir/pandas that referenced this pull request Nov 9, 2022
…dev#48029)

* BUG: use usedforsecurity for md5 in cross merge

* BUG: use usedforsecurity for md5 in cross merge

* Use uuid
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Merge "cross" will assert on systems with FIPS enforcement
3 participants