Skip to content

Fix headers escaping and parsing #531

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions azure-kusto-data/azure/kusto/data/client_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
from .env_utils import get_env
from azure.kusto.data._version import VERSION

NONE = "[none]"
NONE = "NONE"

REPLACE_REGEX = re.compile(r"[\r\n\s{}|]+")
REPLACE_REGEX = re.compile(r"[^a-zA-Z0-9_.\-()]")
MAX_HEADER_LENGTH = 100


@functools.lru_cache(maxsize=1)
Expand Down Expand Up @@ -57,18 +58,22 @@ def format_header(args: List[Tuple[str, str]]) -> str:


def escape_field(field: str):
return f"{{{REPLACE_REGEX.sub('_', field)}}}"
return f"{REPLACE_REGEX.sub('_', field[0:MAX_HEADER_LENGTH])}"


@dataclass
class ClientDetails:
application_for_tracing: str
user_name_for_tracing: str
version_for_tracing: str = format_version()
should_escape: bool = True

def __post_init__(self):
self.application_for_tracing = self.application_for_tracing or default_script()
self.user_name_for_tracing = self.user_name_for_tracing or default_user()
if self.should_escape:
self.application_for_tracing = escape_field(self.application_for_tracing)
self.user_name_for_tracing = escape_field(self.user_name_for_tracing)

@staticmethod
def set_connector_details(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does a static method creating details is called set_connector_details

Expand All @@ -80,17 +85,17 @@ def set_connector_details(
override_user: Optional[str] = None,
additional_fields: Optional[List[Tuple[str, str]]] = None,
) -> "ClientDetails":
params = [("Kusto." + name, version)]
params = [("Kusto." + escape_field(name), escape_field(version))]

app_name = app_name or default_script()
app_version = app_version or NONE
app_name = escape_field(app_name or default_script())
app_version = escape_field(app_version or NONE)

params.append(("App." + escape_field(app_name), app_version))
params.append(("App." + app_name, app_version))
params.extend(additional_fields or [])

user = NONE

if send_user:
user = override_user or default_user()

return ClientDetails(application_for_tracing=format_header(params), user_name_for_tracing=user)
return ClientDetails(application_for_tracing=format_header(params), user_name_for_tracing=user, should_escape=False)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should_escape = true ?

8 changes: 3 additions & 5 deletions azure-kusto-data/azure/kusto/data/kcsb.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ def __init__(self, connection_string: str):
https://kusto.azurewebsites.net/docs/concepts/kusto_connection_strings.html
"""
assert_string_is_not_empty(connection_string)
self._client_details: Optional[ClientDetails] = None
self._internal_dict = {}
self._token_provider = None
self._async_token_provider = None
Expand Down Expand Up @@ -644,7 +645,7 @@ def user_name_for_tracing(self, value: str):

@property
def client_details(self) -> ClientDetails:
return ClientDetails(self.application_for_tracing, self.user_name_for_tracing)
return self._client_details or ClientDetails(self.application_for_tracing, self.user_name_for_tracing)

def _set_connector_details(
self,
Expand All @@ -666,10 +667,7 @@ def _set_connector_details(
:param app_version: The version of the containing application
:param additional_fields: Additional fields to add to the header
"""
client_details = ClientDetails.set_connector_details(name, version, app_name, app_version, send_user, override_user, additional_fields)

self.application_for_tracing = client_details.application_for_tracing
self.user_name_for_tracing = client_details.user_name_for_tracing
self._client_details = ClientDetails.set_connector_details(name, version, app_name, app_version, send_user, override_user, additional_fields)

def __str__(self) -> str:
dict_copy = self._internal_dict.copy()
Expand Down
66 changes: 62 additions & 4 deletions azure-kusto-data/tests/test_client_request_properties.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,10 @@ def test_set_connector_name_and_version():
)

assert params.request_headers["x-ms-client-request-id"] is not None
assert params.request_headers["x-ms-user"] == "[none]"
assert params.request_headers["x-ms-user"] == "NONE"
assert params.request_headers["x-ms-client-version"].startswith("Kusto.Python.Client:")

assert params.request_headers["x-ms-app"].startswith("Kusto.myConnector:{myVersion}|App.")
assert params.request_headers["x-ms-app"].startswith("Kusto.myConnector:myVersion|App.")


def test_set_connector_no_app_version():
Expand All @@ -167,7 +167,7 @@ def test_set_connector_no_app_version():
assert len(params.request_headers["x-ms-user"]) > 0
assert params.request_headers["x-ms-client-version"].startswith("Kusto.Python.Client:")

assert params.request_headers["x-ms-app"].startswith("Kusto.myConnector:{myVersion}|App.")
assert params.request_headers["x-ms-app"].startswith("Kusto.myConnector:myVersion|App.")


def test_set_connector_full():
Expand Down Expand Up @@ -198,4 +198,62 @@ def test_set_connector_full():
assert params.request_headers["x-ms-user"] == "myUser"
assert params.request_headers["x-ms-client-version"].startswith("Kusto.Python.Client:")

assert params.request_headers["x-ms-app"] == "Kusto.myConnector:{myVersion}|App.{myApp}:{myAppVersion}|myField:{myValue}"
assert params.request_headers["x-ms-app"] == "Kusto.myConnector:myVersion|App.myApp:myAppVersion|myField:myValue"


def test_set_connector_escaped():
kcsb = KustoConnectionStringBuilder("test")
kcsb._set_connector_details(
"Café",
"1 . 0",
app_name=r"my|test\{}\app",
app_version="s" * 1000,
send_user=True,
override_user="myUser",
additional_fields=[("myField", "myValue")],
)
crp = ClientRequestProperties()

params = ExecuteRequestParams._from_query(
"somequery",
"somedatabase",
crp,
{},
timedelta(seconds=10),
timedelta(seconds=10),
timedelta(seconds=10),
kcsb.client_details,
)

assert params.request_headers["x-ms-client-request-id"] is not None
assert params.request_headers["x-ms-user"] == "myUser"
assert params.request_headers["x-ms-client-version"].startswith("Kusto.Python.Client:")

assert (
params.request_headers["x-ms-app"] == "Kusto.Caf_:1_._0"
"|App.my_test____app:ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss|myField:myValue"
)


def test_kcsb_direct_escaped():
kcsb = KustoConnectionStringBuilder("test")
kcsb.application_for_tracing = "Café"
kcsb.user_name_for_tracing = "user with spaces"
crp = ClientRequestProperties()

params = ExecuteRequestParams._from_query(
"somequery",
"somedatabase",
crp,
{},
timedelta(seconds=10),
timedelta(seconds=10),
timedelta(seconds=10),
kcsb.client_details,
)

assert params.request_headers["x-ms-client-request-id"] is not None
assert params.request_headers["x-ms-user"] == "user_with_spaces"
assert params.request_headers["x-ms-client-version"].startswith("Kusto.Python.Client:")

assert params.request_headers["x-ms-app"] == "Caf_"
Loading