mirror of
https://github.com/gristlabs/grist-core.git
synced 2024-10-27 20:44:07 +00:00
(core) Use standard library uuid.uuidv4
when possible for better randomness
Summary: Using the `random` module in the Grist `UUID()` function is not cryptographically secure, and is only necessary for the old pynbox (Python 2) sandbox which doesn't support `os.urandom`. This diff uses the `uuid.uuidv4()` function from the Python standard library when possible, which is more secure, only falling back to the old implementation when necessary. Test Plan: Added Python unit tests to check both implementations. Reviewers: dsagal Subscribers: paulfitz, dsagal Differential Revision: https://phab.getgrist.com/D3578
This commit is contained in:
parent
3ad78590c2
commit
31f54065f5
@ -872,7 +872,14 @@ def UUID():
|
||||
This would only calculate UUID() once and freeze the calculated value. By contrast, a regular formula
|
||||
may get recalculated any time the document is reloaded, producing a different value for UUID() each time.
|
||||
"""
|
||||
try:
|
||||
uid = uuid.uuid4()
|
||||
except Exception:
|
||||
# Pynbox doesn't support the above because it doesn't support `os.urandom()`.
|
||||
# Using the `random` module is less secure but should be OK.
|
||||
if six.PY2:
|
||||
return str(uuid.UUID(bytes=[chr(random.randrange(0, 256)) for _ in xrange(0, 16)], version=4))
|
||||
byts = [chr(random.randrange(0, 256)) for _ in xrange(0, 16)]
|
||||
else:
|
||||
return str(uuid.UUID(bytes=bytes([random.randrange(0, 256) for _ in range(0, 16)]), version=4))
|
||||
byts = bytes([random.randrange(0, 256) for _ in range(0, 16)])
|
||||
uid = uuid.UUID(bytes=byts, version=4)
|
||||
return str(uid)
|
||||
|
@ -1,6 +1,8 @@
|
||||
import doctest
|
||||
import os
|
||||
import random
|
||||
import re
|
||||
import unittest
|
||||
|
||||
import six
|
||||
|
||||
@ -41,3 +43,40 @@ def load_tests(loader, tests, ignore):
|
||||
setUp = date_setUp, tearDown = date_tearDown))
|
||||
tests.addTests(doctest.DocTestSuite(functions.lookup, checker=Py23DocChecker()))
|
||||
return tests
|
||||
|
||||
|
||||
class TestUuid(unittest.TestCase):
|
||||
def check_uuids(self, expected_unique):
|
||||
uuids = set()
|
||||
for _ in range(100):
|
||||
random.seed(0) # should make only 'fallback' UUIDs all the same
|
||||
uuids.add(functions.UUID())
|
||||
|
||||
self.assertEqual(len(uuids), expected_unique)
|
||||
for uid in uuids:
|
||||
match = re.match(r'^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$', uid)
|
||||
self.assertIsNotNone(match, uid)
|
||||
|
||||
def test_standard_uuid(self):
|
||||
# Test that uuid.uuid4() is used correctly.
|
||||
# uuid.uuid4() shouldn't be affected by random.seed().
|
||||
# Depending on the test environment, uuid.uuid4() may or may not actually be available.
|
||||
try:
|
||||
os.urandom(1)
|
||||
except NotImplementedError:
|
||||
expected_unique = 1
|
||||
else:
|
||||
expected_unique = 100
|
||||
|
||||
self.check_uuids(expected_unique)
|
||||
|
||||
def test_fallback_uuid(self):
|
||||
# Test that our custom implementation with the `random` module works
|
||||
# and is used when uuid.uuid4() is not available.
|
||||
import uuid
|
||||
v4 = uuid.uuid4
|
||||
del uuid.uuid4
|
||||
try:
|
||||
self.check_uuids(1) # because of the `random.seed(0)` in `check_uuids()`
|
||||
finally:
|
||||
uuid.uuid4 = v4
|
||||
|
Loading…
Reference in New Issue
Block a user