From 08fb3aa8797719109d5ba9c4897c6e0a6ae8eff4 Mon Sep 17 00:00:00 2001 From: Kirill Goncharov Date: Mon, 13 May 2019 15:46:34 +0300 Subject: [PATCH] Prevent generation of very long names for lock file (#97) --- HISTORY.rst | 13 ++++++++++++- celery_once/__init__.py | 2 +- celery_once/backends/file.py | 16 +++++++++++++++- tests/integration/backends/test_file.py | 2 +- tests/unit/backends/test_file.py | 18 ++++++++++++++---- 5 files changed, 43 insertions(+), 8 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index afb221a..af36b54 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -1,11 +1,22 @@ History ======= +3.0.0 +----- +2019-05-13 + +Fixed an issue where large/long arguments could cause ``OSError Filename too long`` with the file backend (see #96). +Keys generated for file backend, are now hashed and limited to 50 characters in length. +*Due to this, it is not backwards compatible with existing keys from the file backend, so any pending locks from previous version will be ignored.* +The Redis backend is unchanged, and thus fully compatible. + +Credit for fix to @xuhcc. + 2.1.2 ----- 2019-05-13 -- Add support for 'rediss'. Thanks @gustavoalmeida +- Add support for ``rediss``. Thanks @gustavoalmeida 2.1.1 ----- diff --git a/celery_once/__init__.py b/celery_once/__init__.py index 95bac35..dbf6b8d 100755 --- a/celery_once/__init__.py +++ b/celery_once/__init__.py @@ -2,7 +2,7 @@ __author__ = 'Cameron Maske' __email__ = 'cameronmaske@gmail.com' -__version__ = '2.1.2' +__version__ = '3.0.0' from .tasks import QueueOnce, AlreadyQueued diff --git a/celery_once/backends/file.py b/celery_once/backends/file.py index c9ea5c1..3b9ed6a 100644 --- a/celery_once/backends/file.py +++ b/celery_once/backends/file.py @@ -1,14 +1,27 @@ """ Definition of the file locking backend. """ +import hashlib import errno import os import tempfile import time +import six + from celery_once.tasks import AlreadyQueued +def key_to_lock_name(key): + """ + Combine part of a key with its hash to prevent very long filenames + """ + MAX_LENGTH = 50 + key_hash = hashlib.md5(six.b(key)).hexdigest() + lock_name = key[:MAX_LENGTH - len(key_hash) - 1] + '_' + key_hash + return lock_name + + class File(object): """ File locking backend. @@ -27,7 +40,8 @@ def __init__(self, settings): raise def _get_lock_path(self, key): - return os.path.join(self.location, key) + lock_name = key_to_lock_name(key) + return os.path.join(self.location, lock_name) def raise_or_lock(self, key, timeout): """ diff --git a/tests/integration/backends/test_file.py b/tests/integration/backends/test_file.py index 5e488a5..7037ff0 100644 --- a/tests/integration/backends/test_file.py +++ b/tests/integration/backends/test_file.py @@ -23,7 +23,7 @@ def example(a=1): @pytest.fixture() def lock_path(): - path = '/tmp/celery_once/qo_example_a-1' + path = '/tmp/celery_once/qo_example_a-1_b7f89d8561e5788a3e7687c6ede93bcd' yield path os.remove(path) # Remove file after test function runs. diff --git a/tests/unit/backends/test_file.py b/tests/unit/backends/test_file.py index 966ce9b..89895cd 100644 --- a/tests/unit/backends/test_file.py +++ b/tests/unit/backends/test_file.py @@ -5,10 +5,17 @@ import pytest -from celery_once.backends.file import File +from celery_once.backends.file import key_to_lock_name, File from celery_once.tasks import AlreadyQueued +def test_key_to_lock_name(): + assert key_to_lock_name('qo_test') == \ + 'qo_test_999f583e69db6a0c04b86beeebb2b631' + assert key_to_lock_name('qo_looooooong_task_name') == \ + 'qo_looooooong_tas_6626e5965e549303044d5a7f4fdc3c6b' + + def test_file_init(mocker): makedirs_mock = mocker.patch('celery_once.backends.file.os.makedirs') location = '/home/test' @@ -55,7 +62,8 @@ def test_file_create_lock(backend, mocker): mtime_mock = mocker.patch('celery_once.backends.file.os.path.getmtime') utime_mock = mocker.patch('celery_once.backends.file.os.utime') close_mock = mocker.patch('celery_once.backends.file.os.close') - expected_lock_path = os.path.join(TEST_LOCATION, key) + expected_lock_path = os.path.join(TEST_LOCATION, + key_to_lock_name(key)) ret = backend.raise_or_lock(key, timeout) assert open_mock.call_count == 1 @@ -103,7 +111,8 @@ def test_file_lock_timeout(backend, mocker): return_value=1550156000.0) utime_mock = mocker.patch('celery_once.backends.file.os.utime') close_mock = mocker.patch('celery_once.backends.file.os.close') - expected_lock_path = os.path.join(TEST_LOCATION, key) + expected_lock_path = os.path.join(TEST_LOCATION, + key_to_lock_name(key)) ret = backend.raise_or_lock(key, timeout) assert open_mock.call_count == 1 @@ -115,7 +124,8 @@ def test_file_lock_timeout(backend, mocker): def test_file_clear_lock(backend, mocker): key = 'test.task.key' remove_mock = mocker.patch('celery_once.backends.file.os.remove') - expected_lock_path = os.path.join(TEST_LOCATION, key) + expected_lock_path = os.path.join(TEST_LOCATION, + key_to_lock_name(key)) ret = backend.clear_lock(key) assert remove_mock.call_count == 1