Update logic to align with other validation in repository

Author: jsonbailey

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('/')

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
 
 

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