From 133b3db811882a17ede887b9fb622815e4d0fd2b Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 29 Sep 2025 13:55:31 +0000 Subject: [PATCH 1/8] fix: Add validation format check for SDK key - Add validate_sdk_key function to prevent logging invalid SDK keys - Validate SDK key format in Config constructor - Add comprehensive tests for SDK key validation - Follow existing validation patterns in codebase Prevents logging of invalid SDK keys by validating that they contain only visible ASCII characters suitable for HTTP headers. Invalid keys trigger a ValueError with a generic message that doesn't expose the actual key value. Co-Authored-By: jbailey@launchdarkly.com --- ldclient/config.py | 9 ++++-- ldclient/impl/util.py | 19 +++++++++++ ldclient/testing/impl/test_util.py | 51 ++++++++++++++++++++++++++++++ ldclient/testing/test_config.py | 33 +++++++++++++++++++ 4 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 ldclient/testing/impl/test_util.py diff --git a/ldclient/config.py b/ldclient/config.py index 32b28dfc..9b0dac77 100644 --- a/ldclient/config.py +++ b/ldclient/config.py @@ -9,7 +9,7 @@ from ldclient.feature_store import InMemoryFeatureStore from ldclient.hook import Hook -from ldclient.impl.util import log, validate_application_info +from ldclient.impl.util import log, validate_application_info, validate_sdk_key from ldclient.interfaces import ( BigSegmentStore, DataSourceUpdateSink, @@ -261,6 +261,9 @@ def __init__( :param omit_anonymous_contexts: Sets whether anonymous contexts should be omitted from index and identify events. :param payload_filter_key: The payload filter is used to selectively limited the flags and segments delivered in the data source payload. """ + if sdk_key and not validate_sdk_key(sdk_key, log): + raise ValueError("SDK key contains invalid characters") + self.__sdk_key = sdk_key self.__base_uri = base_uri.rstrip('/') @@ -542,8 +545,8 @@ def data_source_update_sink(self) -> Optional[DataSourceUpdateSink]: return self._data_source_update_sink def _validate(self): - if self.offline is False and self.sdk_key is None or self.sdk_key == '': - log.warning("Missing or blank sdk_key.") + if self.offline is False and (self.sdk_key is None or self.sdk_key == ''): + log.warning("Missing or blank SDK key") __all__ = ['Config', 'BigSegmentsConfig', 'HTTPConfig'] diff --git a/ldclient/impl/util.py b/ldclient/impl/util.py index 4fbaf110..60f01a3c 100644 --- a/ldclient/impl/util.py +++ b/ldclient/impl/util.py @@ -53,6 +53,25 @@ def validate_application_value(value: Any, name: str, logger: logging.Logger) -> return value +def validate_sdk_key(sdk_key: str, logger: logging.Logger) -> bool: + """ + Validate that an SDK key contains only characters that are valid for HTTP headers. + Returns True if valid, False if invalid. Logs a generic error message for invalid keys. + """ + if not isinstance(sdk_key, str): + logger.warning("SDK key must be a string") + return False + + if sdk_key == '': + return True # Empty keys are handled separately in _validate() + + if re.search(r"[^\x21-\x7E]", sdk_key): + logger.warning("SDK key contains invalid characters") + return False + + return True + + def _headers(config): base_headers = _base_headers(config) base_headers.update({'Content-Type': "application/json"}) diff --git a/ldclient/testing/impl/test_util.py b/ldclient/testing/impl/test_util.py new file mode 100644 index 00000000..a241d1dd --- /dev/null +++ b/ldclient/testing/impl/test_util.py @@ -0,0 +1,51 @@ +import logging +from unittest.mock import Mock +from ldclient.impl.util import validate_sdk_key + + +def test_validate_sdk_key_valid(): + """Test validation of valid SDK keys""" + logger = Mock(spec=logging.Logger) + + valid_keys = [ + "sdk-12345678-1234-1234-1234-123456789012", + "valid-sdk-key-123", + "VALID_SDK_KEY_456" + ] + + for key in valid_keys: + assert validate_sdk_key(key, logger) is True + logger.warning.assert_not_called() + logger.reset_mock() + + +def test_validate_sdk_key_invalid(): + """Test validation of invalid SDK keys""" + logger = Mock(spec=logging.Logger) + + invalid_keys = [ + "sdk-key-with-\x00-null", + "sdk-key-with-\n-newline", + "sdk-key-with-\t-tab" + ] + + for key in invalid_keys: + assert validate_sdk_key(key, logger) is False + logger.warning.assert_called_with("SDK key contains invalid characters") + logger.reset_mock() + + +def test_validate_sdk_key_non_string(): + """Test validation of non-string SDK keys""" + logger = Mock(spec=logging.Logger) + + assert validate_sdk_key("123", logger) is True + logger.warning.assert_not_called() + + +def test_validate_sdk_key_empty(): + """Test validation of empty SDK keys""" + logger = Mock(spec=logging.Logger) + + assert validate_sdk_key("", logger) is True + logger.warning.assert_not_called() diff --git a/ldclient/testing/test_config.py b/ldclient/testing/test_config.py index 77fc5b34..3a39e584 100644 --- a/ldclient/testing/test_config.py +++ b/ldclient/testing/test_config.py @@ -45,6 +45,39 @@ def test_trims_trailing_slashes_on_uris(): assert config.stream_base_uri == "https://blog.launchdarkly.com" +def test_sdk_key_validation_valid_keys(): + """Test that valid SDK keys are accepted""" + valid_keys = [ + "sdk-12345678-1234-1234-1234-123456789012", + "valid-sdk-key-123", + "VALID_SDK_KEY_456" + ] + + for key in valid_keys: + config = Config(sdk_key=key) + assert config.sdk_key == key + + +def test_sdk_key_validation_invalid_keys(): + """Test that invalid SDK keys are rejected""" + invalid_keys = [ + "sdk-key-with-\x00-null", + "sdk-key-with-\n-newline", + "sdk-key-with-\t-tab", + "sdk-key-with-\x7F-del" + ] + + for key in invalid_keys: + with pytest.raises(ValueError, match="SDK key contains invalid characters"): + Config(sdk_key=key) + + +def test_sdk_key_validation_empty_key(): + """Test that empty SDK keys don't trigger format validation""" + config = Config(sdk_key="") + assert config.sdk_key == "" + + def application_can_be_set_and_read(): application = {"id": "my-id", "version": "abcdef"} config = Config(sdk_key="SDK_KEY", application=application) From 6b0920bf764fcc4428f25788cf10e7788a2553f3 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 29 Sep 2025 14:05:34 +0000 Subject: [PATCH 2/8] fix: Correct non-string SDK key validation test - Update test_validate_sdk_key_non_string to actually test non-string inputs - Test various non-string types: int, None, object, list, dict - Verify proper error message is logged for each invalid type Co-Authored-By: jbailey@launchdarkly.com --- ldclient/testing/impl/test_util.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ldclient/testing/impl/test_util.py b/ldclient/testing/impl/test_util.py index a241d1dd..41083ee9 100644 --- a/ldclient/testing/impl/test_util.py +++ b/ldclient/testing/impl/test_util.py @@ -39,8 +39,13 @@ def test_validate_sdk_key_non_string(): """Test validation of non-string SDK keys""" logger = Mock(spec=logging.Logger) - assert validate_sdk_key("123", logger) is True - logger.warning.assert_not_called() + non_string_values = [123, None, object(), [], {}] + + for value in non_string_values: + result = validate_sdk_key(value, logger) + assert result is False + logger.warning.assert_called_with("SDK key must be a string") + logger.reset_mock() def test_validate_sdk_key_empty(): From 5891f344898b14d1751b4234f63cc3fcb6812f2b Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 29 Sep 2025 15:07:57 +0000 Subject: [PATCH 3/8] fix: Update SDK key validation to follow dotnet pattern - Rename validate_sdk_key to is_valid_sdk_key_format - Use dotnet regex pattern ^[-a-zA-Z0-9._]+$ instead of HTTP header validation - Add maximum length validation of 8192 characters - Change from raising ValueError to silently not setting invalid keys - Update all tests to expect silent failure instead of exceptions - Update documentation to reflect new behavior Co-Authored-By: jbailey@launchdarkly.com --- ldclient/config.py | 11 +++--- ldclient/impl/util.py | 17 ++++----- ldclient/testing/impl/test_util.py | 57 ++++++++++++++---------------- ldclient/testing/test_config.py | 44 +++++++++++++++++++---- 4 files changed, 80 insertions(+), 49 deletions(-) diff --git a/ldclient/config.py b/ldclient/config.py index 9b0dac77..593b26c7 100644 --- a/ldclient/config.py +++ b/ldclient/config.py @@ -9,7 +9,7 @@ from ldclient.feature_store import InMemoryFeatureStore from ldclient.hook import Hook -from ldclient.impl.util import log, validate_application_info, validate_sdk_key +from ldclient.impl.util import log, validate_application_info, is_valid_sdk_key_format from ldclient.interfaces import ( BigSegmentStore, DataSourceUpdateSink, @@ -261,10 +261,10 @@ def __init__( :param omit_anonymous_contexts: Sets whether anonymous contexts should be omitted from index and identify events. :param payload_filter_key: The payload filter is used to selectively limited the flags and segments delivered in the data source payload. """ - if sdk_key and not validate_sdk_key(sdk_key, log): - raise ValueError("SDK key contains invalid characters") - - self.__sdk_key = sdk_key + if is_valid_sdk_key_format(sdk_key): + self.__sdk_key = sdk_key + else: + self.__sdk_key = None self.__base_uri = base_uri.rstrip('/') self.__events_uri = events_uri.rstrip('/') @@ -305,6 +305,7 @@ def __init__( def copy_with_new_sdk_key(self, new_sdk_key: str) -> 'Config': """Returns a new ``Config`` instance that is the same as this one, except for having a different SDK key. + The key will not be updated if the provided key contains invalid characters. :param new_sdk_key: the new SDK key """ diff --git a/ldclient/impl/util.py b/ldclient/impl/util.py index 60f01a3c..5fd81ede 100644 --- a/ldclient/impl/util.py +++ b/ldclient/impl/util.py @@ -53,20 +53,21 @@ def validate_application_value(value: Any, name: str, logger: logging.Logger) -> return value -def validate_sdk_key(sdk_key: str, logger: logging.Logger) -> bool: +def is_valid_sdk_key_format(sdk_key: str) -> bool: """ - Validate that an SDK key contains only characters that are valid for HTTP headers. - Returns True if valid, False if invalid. Logs a generic error message for invalid keys. + Validates that a string does not contain invalid characters and is not too long for our systems. + Returns True if the SDK key format is valid, otherwise False. """ + if sdk_key is None or sdk_key == '': + return True + if not isinstance(sdk_key, str): - logger.warning("SDK key must be a string") return False - if sdk_key == '': - return True # Empty keys are handled separately in _validate() + if len(sdk_key) > 8192: + return False - if re.search(r"[^\x21-\x7E]", sdk_key): - logger.warning("SDK key contains invalid characters") + if not re.match(r'^[-a-zA-Z0-9._]+$', sdk_key): return False return True diff --git a/ldclient/testing/impl/test_util.py b/ldclient/testing/impl/test_util.py index 41083ee9..b979f49c 100644 --- a/ldclient/testing/impl/test_util.py +++ b/ldclient/testing/impl/test_util.py @@ -1,56 +1,53 @@ -import logging -from unittest.mock import Mock -from ldclient.impl.util import validate_sdk_key +from ldclient.impl.util import is_valid_sdk_key_format -def test_validate_sdk_key_valid(): +def test_is_valid_sdk_key_format_valid(): """Test validation of valid SDK keys""" - logger = Mock(spec=logging.Logger) - valid_keys = [ "sdk-12345678-1234-1234-1234-123456789012", "valid-sdk-key-123", - "VALID_SDK_KEY_456" + "VALID_SDK_KEY_456", + "test.key_with.dots", + "test-key-with-hyphens" ] for key in valid_keys: - assert validate_sdk_key(key, logger) is True - logger.warning.assert_not_called() - logger.reset_mock() + assert is_valid_sdk_key_format(key) is True -def test_validate_sdk_key_invalid(): +def test_is_valid_sdk_key_format_invalid(): """Test validation of invalid SDK keys""" - logger = Mock(spec=logging.Logger) - invalid_keys = [ "sdk-key-with-\x00-null", "sdk-key-with-\n-newline", - "sdk-key-with-\t-tab" + "sdk-key-with-\t-tab", + "sdk key with spaces", + "sdk@key#with$special%chars", + "sdk/key\\with/slashes" ] for key in invalid_keys: - assert validate_sdk_key(key, logger) is False - logger.warning.assert_called_with("SDK key contains invalid characters") - logger.reset_mock() + assert is_valid_sdk_key_format(key) is False -def test_validate_sdk_key_non_string(): +def test_is_valid_sdk_key_format_non_string(): """Test validation of non-string SDK keys""" - logger = Mock(spec=logging.Logger) - - non_string_values = [123, None, object(), [], {}] + non_string_values = [123, object(), [], {}] for value in non_string_values: - result = validate_sdk_key(value, logger) - assert result is False - logger.warning.assert_called_with("SDK key must be a string") - logger.reset_mock() + assert is_valid_sdk_key_format(value) is False + + +def test_is_valid_sdk_key_format_empty_and_none(): + """Test validation of empty and None SDK keys""" + assert is_valid_sdk_key_format("") is True + assert is_valid_sdk_key_format(None) is True -def test_validate_sdk_key_empty(): - """Test validation of empty SDK keys""" - logger = Mock(spec=logging.Logger) +def test_is_valid_sdk_key_format_max_length(): + """Test validation of SDK key maximum length""" + valid_key = "a" * 8192 + assert is_valid_sdk_key_format(valid_key) is True - assert validate_sdk_key("", logger) is True - logger.warning.assert_not_called() + invalid_key = "a" * 8193 + assert is_valid_sdk_key_format(invalid_key) is False diff --git a/ldclient/testing/test_config.py b/ldclient/testing/test_config.py index 3a39e584..f9680c06 100644 --- a/ldclient/testing/test_config.py +++ b/ldclient/testing/test_config.py @@ -50,7 +50,9 @@ def test_sdk_key_validation_valid_keys(): valid_keys = [ "sdk-12345678-1234-1234-1234-123456789012", "valid-sdk-key-123", - "VALID_SDK_KEY_456" + "VALID_SDK_KEY_456", + "test.key_with.dots", + "test-key-with-hyphens" ] for key in valid_keys: @@ -59,25 +61,55 @@ def test_sdk_key_validation_valid_keys(): def test_sdk_key_validation_invalid_keys(): - """Test that invalid SDK keys are rejected""" + """Test that invalid SDK keys are not set""" invalid_keys = [ "sdk-key-with-\x00-null", "sdk-key-with-\n-newline", "sdk-key-with-\t-tab", - "sdk-key-with-\x7F-del" + "sdk key with spaces", + "sdk@key#with$special%chars", + "sdk/key\\with/slashes" ] for key in invalid_keys: - with pytest.raises(ValueError, match="SDK key contains invalid characters"): - Config(sdk_key=key) + config = Config(sdk_key=key) + assert config.sdk_key is None def test_sdk_key_validation_empty_key(): - """Test that empty SDK keys don't trigger format validation""" + """Test that empty SDK keys are accepted""" config = Config(sdk_key="") assert config.sdk_key == "" +def test_sdk_key_validation_none_key(): + """Test that None SDK keys are accepted""" + config = Config(sdk_key=None) + assert config.sdk_key is None + + +def test_sdk_key_validation_max_length(): + """Test SDK key maximum length validation""" + valid_key = "a" * 8192 + config = Config(sdk_key=valid_key) + assert config.sdk_key == valid_key + + invalid_key = "a" * 8193 + config = Config(sdk_key=invalid_key) + assert config.sdk_key is None + + +def test_copy_with_new_sdk_key_validation(): + """Test that copy_with_new_sdk_key validates the new key""" + original_config = Config(sdk_key="valid-key") + + new_config = original_config.copy_with_new_sdk_key("another-valid-key") + assert new_config.sdk_key == "another-valid-key" + + invalid_config = original_config.copy_with_new_sdk_key("invalid key with spaces") + assert invalid_config.sdk_key is None + + def application_can_be_set_and_read(): application = {"id": "my-id", "version": "abcdef"} config = Config(sdk_key="SDK_KEY", application=application) From b7eec9696be7bd03e1198592e13dbc8c17cbb0e0 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 29 Sep 2025 15:08:25 +0000 Subject: [PATCH 4/8] fix: Add proper docstring formatting with :param and :return: style - Address GitHub comment about missing docstring format - Follow Python conventions for function documentation Co-Authored-By: jbailey@launchdarkly.com --- ldclient/impl/util.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ldclient/impl/util.py b/ldclient/impl/util.py index 5fd81ede..7ee37359 100644 --- a/ldclient/impl/util.py +++ b/ldclient/impl/util.py @@ -56,7 +56,9 @@ def validate_application_value(value: Any, name: str, logger: logging.Logger) -> def is_valid_sdk_key_format(sdk_key: str) -> bool: """ Validates that a string does not contain invalid characters and is not too long for our systems. - Returns True if the SDK key format is valid, otherwise False. + + :param sdk_key: the SDK key to validate + :return: True if the SDK key format is valid, otherwise False """ if sdk_key is None or sdk_key == '': return True From e5147846ace86bda7a0b0fc6f5dbe1b478343168 Mon Sep 17 00:00:00 2001 From: jsonbailey Date: Mon, 29 Sep 2025 15:48:59 +0000 Subject: [PATCH 5/8] Update logic to align with other validation in repository --- ldclient/config.py | 7 ++---- ldclient/impl/util.py | 35 ++++++++++++++++------------ ldclient/testing/impl/test_util.py | 37 +++++++++++++++++++----------- 3 files changed, 46 insertions(+), 33 deletions(-) diff --git a/ldclient/config.py b/ldclient/config.py index 593b26c7..9f00f993 100644 --- a/ldclient/config.py +++ b/ldclient/config.py @@ -9,7 +9,7 @@ from ldclient.feature_store import InMemoryFeatureStore from ldclient.hook import Hook -from ldclient.impl.util import log, validate_application_info, is_valid_sdk_key_format +from ldclient.impl.util import log, validate_application_info, validate_sdk_key_format from ldclient.interfaces import ( BigSegmentStore, DataSourceUpdateSink, @@ -261,10 +261,7 @@ def __init__( :param omit_anonymous_contexts: Sets whether anonymous contexts should be omitted from index and identify events. :param payload_filter_key: The payload filter is used to selectively limited the flags and segments delivered in the data source payload. """ - if is_valid_sdk_key_format(sdk_key): - self.__sdk_key = sdk_key - else: - self.__sdk_key = None + self.__sdk_key = validate_sdk_key_format(sdk_key, log) self.__base_uri = base_uri.rstrip('/') self.__events_uri = events_uri.rstrip('/') diff --git a/ldclient/impl/util.py b/ldclient/impl/util.py index 7ee37359..ad7fd1f6 100644 --- a/ldclient/impl/util.py +++ b/ldclient/impl/util.py @@ -27,8 +27,13 @@ def timedelta_millis(delta: timedelta) -> float: __BASE_TYPES__ = (str, float, int, bool) +# Maximum length for SDK keys +_MAX_SDK_KEY_LENGTH = 8192 -_retryable_statuses = [400, 408, 429] +_RETRYABLE_STATUSES = [400, 408, 429] + +# Compiled regex pattern for valid characters in application values and SDK keys +_VALID_CHARACTERS_REGEX = re.compile(r"[^a-zA-Z0-9._-]") def validate_application_info(application: dict, logger: logging.Logger) -> dict: @@ -46,33 +51,33 @@ def validate_application_value(value: Any, name: str, logger: logging.Logger) -> logger.warning('Value of application[%s] was longer than 64 characters and was discarded' % name) return "" - if re.search(r"[^a-zA-Z0-9._-]", value): + if _VALID_CHARACTERS_REGEX.search(value): logger.warning('Value of application[%s] contained invalid characters and was discarded' % name) return "" return value -def is_valid_sdk_key_format(sdk_key: str) -> bool: +def validate_sdk_key_format(sdk_key: str, logger: logging.Logger) -> str: """ - Validates that a string does not contain invalid characters and is not too long for our systems. + Validates that an SDK key does not contain invalid characters and is not too long for our systems. :param sdk_key: the SDK key to validate - :return: True if the SDK key format is valid, otherwise False + :param logger: the logger to use for logging warnings + :return: the validated SDK key, or None if the SDK key is invalid """ if sdk_key is None or sdk_key == '': - return True - - if not isinstance(sdk_key, str): - return False + return None - if len(sdk_key) > 8192: - return False + if len(sdk_key) > _MAX_SDK_KEY_LENGTH: + logger.warning('SDK key was longer than %d characters and was discarded' % _MAX_SDK_KEY_LENGTH) + return None - if not re.match(r'^[-a-zA-Z0-9._]+$', sdk_key): - return False + if _VALID_CHARACTERS_REGEX.search(sdk_key): + logger.warning('SDK key contained invalid characters and was discarded') + return None - return True + return sdk_key def _headers(config): @@ -128,7 +133,7 @@ def throw_if_unsuccessful_response(resp): def is_http_error_recoverable(status): if status >= 400 and status < 500: - return status in _retryable_statuses # all other 4xx besides these are unrecoverable + return status in _RETRYABLE_STATUSES # all other 4xx besides these are unrecoverable return True # all other errors are recoverable diff --git a/ldclient/testing/impl/test_util.py b/ldclient/testing/impl/test_util.py index b979f49c..f2fdd848 100644 --- a/ldclient/testing/impl/test_util.py +++ b/ldclient/testing/impl/test_util.py @@ -1,8 +1,10 @@ -from ldclient.impl.util import is_valid_sdk_key_format +import logging +from ldclient.impl.util import validate_sdk_key_format -def test_is_valid_sdk_key_format_valid(): +def test_validate_sdk_key_format_valid(): """Test validation of valid SDK keys""" + logger = logging.getLogger('test') valid_keys = [ "sdk-12345678-1234-1234-1234-123456789012", "valid-sdk-key-123", @@ -12,11 +14,13 @@ def test_is_valid_sdk_key_format_valid(): ] for key in valid_keys: - assert is_valid_sdk_key_format(key) is True + result = validate_sdk_key_format(key, logger) + assert result == key # Should return the same key if valid -def test_is_valid_sdk_key_format_invalid(): +def test_validate_sdk_key_format_invalid(): """Test validation of invalid SDK keys""" + logger = logging.getLogger('test') invalid_keys = [ "sdk-key-with-\x00-null", "sdk-key-with-\n-newline", @@ -27,27 +31,34 @@ def test_is_valid_sdk_key_format_invalid(): ] for key in invalid_keys: - assert is_valid_sdk_key_format(key) is False + result = validate_sdk_key_format(key, logger) + assert result is None # Should return None for invalid keys -def test_is_valid_sdk_key_format_non_string(): +def test_validate_sdk_key_format_non_string(): """Test validation of non-string SDK keys""" + logger = logging.getLogger('test') non_string_values = [123, object(), [], {}] for value in non_string_values: - assert is_valid_sdk_key_format(value) is False + result = validate_sdk_key_format(value, logger) + assert result is None # Should return None for non-string values -def test_is_valid_sdk_key_format_empty_and_none(): +def test_validate_sdk_key_format_empty_and_none(): """Test validation of empty and None SDK keys""" - assert is_valid_sdk_key_format("") is True - assert is_valid_sdk_key_format(None) is True + logger = logging.getLogger('test') + assert validate_sdk_key_format("", logger) is None # Empty string should return None + assert validate_sdk_key_format(None, logger) is None # None should return None -def test_is_valid_sdk_key_format_max_length(): +def test_validate_sdk_key_format_max_length(): """Test validation of SDK key maximum length""" + logger = logging.getLogger('test') valid_key = "a" * 8192 - assert is_valid_sdk_key_format(valid_key) is True + result = validate_sdk_key_format(valid_key, logger) + assert result == valid_key # Should return the same key if valid invalid_key = "a" * 8193 - assert is_valid_sdk_key_format(invalid_key) is False + result = validate_sdk_key_format(invalid_key, logger) + assert result is None # Should return None for keys that are too long From b4f8ab613d4d7493c5c945629117fb687684c814 Mon Sep 17 00:00:00 2001 From: jsonbailey Date: Mon, 29 Sep 2025 17:11:16 +0000 Subject: [PATCH 6/8] fix failing tests --- ldclient/impl/util.py | 11 +++++++---- ldclient/testing/impl/test_util.py | 10 +++++----- ldclient/testing/test_config.py | 8 ++++---- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/ldclient/impl/util.py b/ldclient/impl/util.py index ad7fd1f6..71209c2e 100644 --- a/ldclient/impl/util.py +++ b/ldclient/impl/util.py @@ -64,18 +64,21 @@ def validate_sdk_key_format(sdk_key: str, logger: logging.Logger) -> str: :param sdk_key: the SDK key to validate :param logger: the logger to use for logging warnings - :return: the validated SDK key, or None if the SDK key is invalid + :return: the validated SDK key, or empty string if the SDK key is invalid """ if sdk_key is None or sdk_key == '': - return None + return "" + + if not isinstance(sdk_key, str): + return "" if len(sdk_key) > _MAX_SDK_KEY_LENGTH: logger.warning('SDK key was longer than %d characters and was discarded' % _MAX_SDK_KEY_LENGTH) - return None + return "" if _VALID_CHARACTERS_REGEX.search(sdk_key): logger.warning('SDK key contained invalid characters and was discarded') - return None + return "" return sdk_key diff --git a/ldclient/testing/impl/test_util.py b/ldclient/testing/impl/test_util.py index f2fdd848..1248f109 100644 --- a/ldclient/testing/impl/test_util.py +++ b/ldclient/testing/impl/test_util.py @@ -32,7 +32,7 @@ def test_validate_sdk_key_format_invalid(): for key in invalid_keys: result = validate_sdk_key_format(key, logger) - assert result is None # Should return None for invalid keys + assert result == '' # Should return empty string for invalid keys def test_validate_sdk_key_format_non_string(): @@ -42,14 +42,14 @@ def test_validate_sdk_key_format_non_string(): for value in non_string_values: result = validate_sdk_key_format(value, logger) - assert result is None # Should return None for non-string values + assert result == '' # Should return empty string for non-string values def test_validate_sdk_key_format_empty_and_none(): """Test validation of empty and None SDK keys""" logger = logging.getLogger('test') - assert validate_sdk_key_format("", logger) is None # Empty string should return None - assert validate_sdk_key_format(None, logger) is None # None should return None + assert validate_sdk_key_format("", logger) == '' # Empty string should return empty string + assert validate_sdk_key_format(None, logger) == '' # None should return empty string def test_validate_sdk_key_format_max_length(): @@ -61,4 +61,4 @@ def test_validate_sdk_key_format_max_length(): invalid_key = "a" * 8193 result = validate_sdk_key_format(invalid_key, logger) - assert result is None # Should return None for keys that are too long + assert result == '' # Should return empty string for keys that are too long diff --git a/ldclient/testing/test_config.py b/ldclient/testing/test_config.py index f9680c06..f3f759cb 100644 --- a/ldclient/testing/test_config.py +++ b/ldclient/testing/test_config.py @@ -73,7 +73,7 @@ def test_sdk_key_validation_invalid_keys(): for key in invalid_keys: config = Config(sdk_key=key) - assert config.sdk_key is None + assert config.sdk_key == '' def test_sdk_key_validation_empty_key(): @@ -85,7 +85,7 @@ def test_sdk_key_validation_empty_key(): def test_sdk_key_validation_none_key(): """Test that None SDK keys are accepted""" config = Config(sdk_key=None) - assert config.sdk_key is None + assert config.sdk_key == '' def test_sdk_key_validation_max_length(): @@ -96,7 +96,7 @@ def test_sdk_key_validation_max_length(): invalid_key = "a" * 8193 config = Config(sdk_key=invalid_key) - assert config.sdk_key is None + assert config.sdk_key == '' def test_copy_with_new_sdk_key_validation(): @@ -107,7 +107,7 @@ def test_copy_with_new_sdk_key_validation(): assert new_config.sdk_key == "another-valid-key" invalid_config = original_config.copy_with_new_sdk_key("invalid key with spaces") - assert invalid_config.sdk_key is None + assert invalid_config.sdk_key == '' def application_can_be_set_and_read(): From a19e5bf291310408e95956e1f85a9ef57ba927e1 Mon Sep 17 00:00:00 2001 From: jsonbailey Date: Mon, 29 Sep 2025 21:46:33 +0000 Subject: [PATCH 7/8] fix lint issues --- ldclient/config.py | 6 +++++- ldclient/impl/util.py | 5 +---- ldclient/testing/impl/test_util.py | 11 ++++++----- ldclient/testing/test_config.py | 10 +++++----- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/ldclient/config.py b/ldclient/config.py index 9f00f993..57e3e6c8 100644 --- a/ldclient/config.py +++ b/ldclient/config.py @@ -9,7 +9,11 @@ from ldclient.feature_store import InMemoryFeatureStore from ldclient.hook import Hook -from ldclient.impl.util import log, validate_application_info, validate_sdk_key_format +from ldclient.impl.util import ( + log, + validate_application_info, + validate_sdk_key_format +) from ldclient.interfaces import ( BigSegmentStore, DataSourceUpdateSink, diff --git a/ldclient/impl/util.py b/ldclient/impl/util.py index 71209c2e..e60feb9d 100644 --- a/ldclient/impl/util.py +++ b/ldclient/impl/util.py @@ -61,7 +61,7 @@ def validate_application_value(value: Any, name: str, logger: logging.Logger) -> def validate_sdk_key_format(sdk_key: str, logger: logging.Logger) -> str: """ Validates that an SDK key does not contain invalid characters and is not too long for our systems. - + :param sdk_key: the SDK key to validate :param logger: the logger to use for logging warnings :return: the validated SDK key, or empty string if the SDK key is invalid @@ -71,15 +71,12 @@ def validate_sdk_key_format(sdk_key: str, logger: logging.Logger) -> str: if not isinstance(sdk_key, str): return "" - if len(sdk_key) > _MAX_SDK_KEY_LENGTH: logger.warning('SDK key was longer than %d characters and was discarded' % _MAX_SDK_KEY_LENGTH) return "" - if _VALID_CHARACTERS_REGEX.search(sdk_key): logger.warning('SDK key contained invalid characters and was discarded') return "" - return sdk_key diff --git a/ldclient/testing/impl/test_util.py b/ldclient/testing/impl/test_util.py index 1248f109..ea843e4b 100644 --- a/ldclient/testing/impl/test_util.py +++ b/ldclient/testing/impl/test_util.py @@ -1,4 +1,5 @@ import logging + from ldclient.impl.util import validate_sdk_key_format @@ -12,7 +13,7 @@ def test_validate_sdk_key_format_valid(): "test.key_with.dots", "test-key-with-hyphens" ] - + for key in valid_keys: result = validate_sdk_key_format(key, logger) assert result == key # Should return the same key if valid @@ -23,13 +24,13 @@ def test_validate_sdk_key_format_invalid(): logger = logging.getLogger('test') invalid_keys = [ "sdk-key-with-\x00-null", - "sdk-key-with-\n-newline", + "sdk-key-with-\n-newline", "sdk-key-with-\t-tab", "sdk key with spaces", "sdk@key#with$special%chars", "sdk/key\\with/slashes" ] - + for key in invalid_keys: result = validate_sdk_key_format(key, logger) assert result == '' # Should return empty string for invalid keys @@ -39,7 +40,7 @@ def test_validate_sdk_key_format_non_string(): """Test validation of non-string SDK keys""" logger = logging.getLogger('test') non_string_values = [123, object(), [], {}] - + for value in non_string_values: result = validate_sdk_key_format(value, logger) assert result == '' # Should return empty string for non-string values @@ -58,7 +59,7 @@ def test_validate_sdk_key_format_max_length(): valid_key = "a" * 8192 result = validate_sdk_key_format(valid_key, logger) assert result == valid_key # Should return the same key if valid - + invalid_key = "a" * 8193 result = validate_sdk_key_format(invalid_key, logger) assert result == '' # Should return empty string for keys that are too long diff --git a/ldclient/testing/test_config.py b/ldclient/testing/test_config.py index f3f759cb..b5321d36 100644 --- a/ldclient/testing/test_config.py +++ b/ldclient/testing/test_config.py @@ -54,7 +54,7 @@ def test_sdk_key_validation_valid_keys(): "test.key_with.dots", "test-key-with-hyphens" ] - + for key in valid_keys: config = Config(sdk_key=key) assert config.sdk_key == key @@ -70,7 +70,7 @@ def test_sdk_key_validation_invalid_keys(): "sdk@key#with$special%chars", "sdk/key\\with/slashes" ] - + for key in invalid_keys: config = Config(sdk_key=key) assert config.sdk_key == '' @@ -93,7 +93,7 @@ def test_sdk_key_validation_max_length(): valid_key = "a" * 8192 config = Config(sdk_key=valid_key) assert config.sdk_key == valid_key - + invalid_key = "a" * 8193 config = Config(sdk_key=invalid_key) assert config.sdk_key == '' @@ -102,10 +102,10 @@ def test_sdk_key_validation_max_length(): def test_copy_with_new_sdk_key_validation(): """Test that copy_with_new_sdk_key validates the new key""" original_config = Config(sdk_key="valid-key") - + new_config = original_config.copy_with_new_sdk_key("another-valid-key") assert new_config.sdk_key == "another-valid-key" - + invalid_config = original_config.copy_with_new_sdk_key("invalid key with spaces") assert invalid_config.sdk_key == '' From 6ec6770153c3712aac9a0f4587647af5208177da Mon Sep 17 00:00:00 2001 From: jsonbailey Date: Tue, 30 Sep 2025 15:46:07 +0000 Subject: [PATCH 8/8] validation removes chance of None --- ldclient/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ldclient/config.py b/ldclient/config.py index 57e3e6c8..fbc88ac8 100644 --- a/ldclient/config.py +++ b/ldclient/config.py @@ -547,7 +547,7 @@ def data_source_update_sink(self) -> Optional[DataSourceUpdateSink]: return self._data_source_update_sink def _validate(self): - if self.offline is False and (self.sdk_key is None or self.sdk_key == ''): + if self.offline is False and self.sdk_key == '': log.warning("Missing or blank SDK key")