Skip to content

Commit

Permalink
Merge pull request #29 from cbdq-io/bugfix/28-edge-case-of-connection…
Browse files Browse the repository at this point in the history
…-string-becomes-an-invalid-url-according-to-urlparse

Bugfix/28 edge case of connection string becomes an invalid url according to urlparse
  • Loading branch information
dallinb authored Jan 6, 2025
2 parents 64f2650 + 1f97eb4 commit bcdec80
Show file tree
Hide file tree
Showing 13 changed files with 209 additions and 32 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -165,5 +165,6 @@ cython_debug/
.python-version

.qlty/*
!.qlty/qlty.toml
!.qlty/configs/
!.qlty/configs/**
103 changes: 103 additions & 0 deletions .qlty/qlty.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
# This file was automatically generated by `qlty init`.
# You can modify it to suit your needs.
# We recommend you to commit this file to your repository.
#
# This configuration is used by both Qlty CLI and Qlty Cloud.
#
# Qlty CLI -- Code quality toolkit for developers
# Qlty Cloud -- Fully automated Code Health Platform
#
# Try Qlty Cloud: https://qlty.sh
#
# For a guide to configuration, visit https://qlty.sh/d/config
# Or for a full reference, visit https://qlty.sh/d/qlty-toml
config_version = "0"

exclude_patterns = [
"*_min.*",
"*-min.*",
"*.min.*",
"**/*.d.ts",
"**/.yarn/**",
"**/bower_components/**",
"**/build/**",
"**/cache/**",
"**/config/**",
"**/db/**",
"**/deps/**",
"**/dist/**",
"**/extern/**",
"**/external/**",
"**/generated/**",
"**/Godeps/**",
"**/gradlew/**",
"**/mvnw/**",
"**/node_modules/**",
"**/protos/**",
"**/seed/**",
"**/target/**",
"**/testdata/**",
"**/vendor/**",
"**/assets/**",
"**/tests/**",
"CHANGELOG.md",
".devcontainer/devcontainer.json"
]

test_patterns = [
"**/test/**",
"**/spec/**",
"**/*.test.*",
"**/*.spec.*",
"**/*_test.*",
"**/*_spec.*",
"**/test_*.*",
"**/spec_*.*",
]

[[source]]
name = "default"
default = true

[[plugin]]
name = "actionlint"

[[plugin]]
name = "bandit"
version = "1.7.9"

[[plugin]]
name = "checkov"

[[plugin]]
name = "golangci-lint"

[[plugin]]
name = "markdownlint"
version = "0.41.0"

[[plugin]]
name = "osv-scanner"

[[plugin]]
name = "prettier"

[[plugin]]
name = "ripgrep"

[[plugin]]
name = "ruff"
version = "0.5.3"

[[plugin]]
name = "trivy"
drivers = [
"config",
"fs-vuln",
]

[[plugin]]
name = "trufflehog"

[[plugin]]
name = "yamllint"
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
# Changelog


## $( python -c 'import router; print(router.__version__)' )

### Fix

* Handle non-URL compatible characters in namespace connection strings. [Ben Dalling]

* Stop false negative security alert. [Ben Dalling]


## 0.2.1 (2024-12-31)

### Fix
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
.EXPORT_ALL_VARIABLES:

TAG = 0.2.1
TAG = $$( python -c 'import router; print(router.__version__)' )

all: lint build clean test

Expand Down
15 changes: 10 additions & 5 deletions replay_dlq.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class DLQReplayHandler(MessagingHandler):
How long to wait for messages to arrive before exiting.
"""

def __init__(self, dlq_topic_name, connection, timeout, logger: logging.Logger):
def __init__(self, dlq_topic_name, connection: ConnectionStringHelper, timeout, logger: logging.Logger):
super().__init__()
self.dlq_topic_name = dlq_topic_name
self.connection = connection
Expand Down Expand Up @@ -148,13 +148,18 @@ def get_message_hash(self, message: Message) -> str:
if type(body) is str:
body = body.encode()

return hashlib.md5(body).hexdigest()
return hashlib.md5(body, usedforsecurity=False).hexdigest()

def on_start(self, event):
"""Execute a start event."""
self.reactor = event.reactor
self.logger.debug(f'Creating a connection for "{self.connection}".')
self.conn = event.container.connect(self.connection)
self.logger.debug(f'Creating a connection for "{self.connection.netloc()}".')
self.conn = event.container.connect(
url=self.connection.netloc(),
allowed_mechs='PLAIN',
password=self.connection.key_value(),
user=self.connection.key_name()
)
self.logger.debug(f'Creating a receiver for "{self.dlq_topic_name}...".')
self.receiver = event.container.create_receiver(self.conn, source=self.dlq_topic_name)
self.logger.debug(f'Receiver for "{self.dlq_topic_name}" created successfully.')
Expand Down Expand Up @@ -227,7 +232,7 @@ def on_connection_close(self, event):
helper = ConnectionStringHelper(runtime.connection_string)
replayer = DLQReplayHandler(
f'{runtime.dlq_topic_name}/Subscriptions/{runtime.subscription}',
helper.amqp_url(),
helper,
runtime.timeout,
runtime.logger
)
Expand Down
50 changes: 39 additions & 11 deletions router.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ def __init__(self, sbus_connection_string: str) -> None:
self.port(5671)
self.protocol('amqps')
self.parse()
self.netloc(f'{self.protocol()}://{self.hostname()}:{self.port()}')
url = f'{self.protocol()}://{self.key_name()}:{self.key_value()}'
url += f'@{self.hostname()}:{self.port()}'
self.amqp_url(url)
Expand Down Expand Up @@ -177,6 +178,25 @@ def key_value(self, key_value: str = None) -> str:
self._key_value = key_value
return self._key_value

def netloc(self, netloc: str = None) -> str:
"""
Get or set the netloc.
Parameters
----------
netloc : str, optional
The value to set.
Returns
-------
str
The currently set value.
"""
if netloc is not None:
self._netloc = netloc

return self._netloc

def parse(self) -> None:
"""
Parse the connection string.
Expand Down Expand Up @@ -502,8 +522,7 @@ def add(self, name: str, connection_string: str) -> None:
connection_string : str
The connection string for connecting to the namespace.
"""
helper = ConnectionStringHelper(connection_string)
self._namespaces[name] = helper.amqp_url()
self._namespaces[name] = connection_string

def count(self) -> int:
"""
Expand Down Expand Up @@ -536,8 +555,7 @@ def get(self, name: str) -> str:
ValueError
If the provided name is not known.
"""
conn_str = self._namespaces[name]
return conn_str
return self._namespaces[name]


class EnvironmentConfigParser:
Expand Down Expand Up @@ -620,6 +638,10 @@ def get_rules(self) -> list[RouterRule]:

return response

def get_source_connection_string(self) -> str:
"""Get the connection string of the source namespace."""
return self._environ['ROUTER_SOURCE_CONNECTION_STRING']

def get_source_url(self) -> str:
"""
Get the URL of the source Service Bus Namespace.
Expand Down Expand Up @@ -705,7 +727,7 @@ def __init__(self, config: EnvironmentConfigParser):
self.rules = config.get_rules()
self.senders = {}
self.sources = []
self.source_namespace_url = config.get_source_url()
self.source_namespace_connection_string = config.get_source_connection_string()
self.parse_config_data()
start_http_server(config.get_prometheus_port())

Expand Down Expand Up @@ -766,14 +788,20 @@ def on_start(self, event):
"""Respond to a start event."""
logger.debug('Start event.')

for url in self.connections.keys():
hostname = urlparse(url).hostname
for connection_string in self.connections.keys():
connection_details = ConnectionStringHelper(connection_string)
hostname = connection_details.netloc()
logger.debug(f'Creating a connection for {hostname}...')
connection = event.container.connect(url, allowed_mechs='PLAIN')
self.connections[url] = connection
connection = event.container.connect(
url=connection_details.netloc(),
allowed_mechs='PLAIN',
password=connection_details.key_value(),
user=connection_details.key_name()
)
self.connections[connection_string] = connection
logger.info(f'Successfully created a connection for {hostname}.')

connection = self.connections[self.source_namespace_url]
connection = self.connections[self.source_namespace_connection_string]

for source in self.sources:
logger.debug(f'Creating a receiver for "{source}".')
Expand All @@ -793,7 +821,7 @@ def on_start(self, event):

def parse_config_data(self) -> dict:
"""Create a dict of connections with the URL as a key."""
url_list = [self.source_namespace_url]
url_list = [self.source_namespace_connection_string]
service_bus_namespaces = self.config.service_bus_namespaces()
sources = []

Expand Down
4 changes: 2 additions & 2 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ radon-max-cc = 5
addopts = --color=no --cov --durations 3 --verbose
bdd_features_base_dir = tests/features/
markers =
system_tests: Marks tests as system tests.
unit_tests: Marks tests as unit tests.
system: Marks tests as system tests.
unit: Marks tests as unit tests.
15 changes: 11 additions & 4 deletions tests/features/connection-string-helper.feature
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
@unit
Feature: Connection String Helper
In order to connect to Azure Service Bus
As a developer
Expand All @@ -7,12 +8,18 @@ Feature: Connection String Helper
Given Azure Service Bus Connection String <sbus_connection_string>
When the Azure Service Bus Connection String is parsed
Then the AMQP URL is <amqp_url>
And key_name is <key_name>
And key_value is <key_value>
And netloc is <netloc>

Examples:
| sbus_connection_string | amqp_url |
| Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=SAS_KEY_VALUE; | amqps://RootManageSharedAccessKey:SAS_KEY_VALUE@localhost:5671 |
| Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=SAS_KEY_VALUE=; | amqps://RootManageSharedAccessKey:SAS_KEY_VALUE=@localhost:5671 |
| Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=SAS_KEY_VALUE;UseDevelopmentEmulator=true; | amqp://RootManageSharedAccessKey:SAS_KEY_VALUE@localhost:5672 |
| sbus_connection_string | amqp_url | key_name | key_value | netloc |
| Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=SAS_KEY_VALUE; | amqps://RootManageSharedAccessKey:SAS_KEY_VALUE@localhost:5671 | RootManageSharedAccessKey | SAS_KEY_VALUE | amqps://localhost:5671 |
| Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=SAS_KEY_VALUE=; | amqps://RootManageSharedAccessKey:SAS_KEY_VALUE=@localhost:5671 | RootManageSharedAccessKey | SAS_KEY_VALUE= | amqps://localhost:5671 |
| Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=A+A+/aa= | amqps://RootManageSharedAccessKey:A+A+/aa=@localhost:5671 | RootManageSharedAccessKey | A+A+/aa= | amqps://localhost:5671 |
| Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=A+A+/aa=; | amqps://RootManageSharedAccessKey:A+A+/aa=@localhost:5671 | RootManageSharedAccessKey | A+A+/aa= | amqps://localhost:5671 |
| Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=A+A+/aa=;UseDevelopmentEmulator=true; | amqp://RootManageSharedAccessKey:A+A+/aa=@localhost:5672 | RootManageSharedAccessKey | A+A+/aa= | amqp://localhost:5672 |
| Endpoint=sb://localhost;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=SAS_KEY_VALUE;UseDevelopmentEmulator=true; | amqp://RootManageSharedAccessKey:SAS_KEY_VALUE@localhost:5672 | RootManageSharedAccessKey | SAS_KEY_VALUE | amqp://localhost:5672 |

Scenario Outline: Invalid Connection Strings
Given Azure Service Bus Connection String <sbus_connection_string>
Expand Down
1 change: 1 addition & 0 deletions tests/features/data-flow.feature
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
@system
Feature: Data Flow
In order to confirm rules
As a Service Bus Router
Expand Down
1 change: 1 addition & 0 deletions tests/features/environment-config-parser.feature
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
@unit
Feature: Environment Configuration Parser

Test that we can correctly extract configuration from
Expand Down
1 change: 1 addition & 0 deletions tests/features/router-rule.feature
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
@unit
Feature: Router Rule
In order to route a message
As a Router
Expand Down
21 changes: 21 additions & 0 deletions tests/step_defs/test_connection_string_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,27 @@ def _():
pass


@then(parsers.parse('key_name is {expected_key_name}'))
def _(expected_key_name: str, sbus_connection_string: str):
"""key_name is <key_name>."""
widget = ConnectionStringHelper(sbus_connection_string)
assert widget.key_name() == expected_key_name


@then(parsers.parse('key_value is {expected_key_value}'))
def _(expected_key_value: str, sbus_connection_string: str):
"""key_value is <key_value>."""
widget = ConnectionStringHelper(sbus_connection_string)
assert expected_key_value == widget.key_value()


@then(parsers.parse('netloc is {expected_netloc}'))
def _(expected_netloc: str, sbus_connection_string: str):
"""netloc is <netloc>."""
widget = ConnectionStringHelper(sbus_connection_string)
assert widget.netloc() == expected_netloc


@then(parsers.parse('the AMQP URL is {expected_url}'))
def _(expected_url: str, sbus_connection_string):
"""the AMQP URL is <amqp_url>."""
Expand Down
Loading

0 comments on commit bcdec80

Please sign in to comment.