Skip to content

Commit

Permalink
Raise config error if connection URL is unparseable
Browse files Browse the repository at this point in the history
  • Loading branch information
jdavcs committed May 27, 2023
1 parent 6ff2397 commit ee31eca
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 0 deletions.
Binary file added involucro
Binary file not shown.
24 changes: 24 additions & 0 deletions lib/galaxy/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,7 @@ def config_value_for_host(self, config_option, host):
return val

def _process_config(self, kwargs: Dict[str, Any]) -> None:
self._check_database_connection_strings()
# Backwards compatibility for names used in too many places to fix
self.datatypes_config = self.datatypes_config_file
self.tool_configs = self.tool_config_file
Expand Down Expand Up @@ -1195,6 +1196,29 @@ def _load_theme(path: str, theme_dict: dict):
else:
_load_theme(self.themes_config_file, self.themes)

def _check_database_connection_strings(self):
"""
Verify connection URI strings in galaxy's configuration are parseable with urllib.
"""

def try_parsing(value, name):
try:
urlparse(value)
except ValueError as e:
msg = f"The `{name}` configuration property cannot be parsed as a connection URI."
if "Invalid IPv6 URL" in str(e):
msg += (
"\nBesides an invalid IPv6 format, this may be caused by a bracket character in the `netloc` part of "
"the URI (most likely, the password). In this case, you should percent-encode that character: for `[` "
"use `%5B`, for `]` use `%5D`. For example, if your URI is `postgresql://user:pass[word@host/db`, "
"change it to `postgresql://user:pass%5Bword@host/db`. "
)
raise ConfigurationError(msg) from e

try_parsing(self.database_connection, "database_connection")
try_parsing(self.install_database_connection, "install_database_connection")
try_parsing(self.amqp_internal_connection, "amqp_internal_connection")

def _configure_dataset_storage(self):
# The default for `file_path` has changed in 20.05; we may need to fall back to the old default
self._set_alt_paths("file_path", self._in_data_dir("files")) # this is called BEFORE guessing id/uuid
Expand Down
15 changes: 15 additions & 0 deletions test/unit/config/test_config_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from galaxy import config
from galaxy.config import DEFAULT_EMAIL_FROM_LOCAL_PART
from galaxy.exceptions import ConfigurationError
from galaxy.util.properties import running_from_source


Expand Down Expand Up @@ -52,6 +53,20 @@ def test_assign_email_from(monkeypatch):
assert appconfig.email_from == f"{DEFAULT_EMAIL_FROM_LOCAL_PART}@myhost"


@pytest.mark.parametrize("bracket", ["[", "]"])
def test_error_if_database_connection_contains_brackets(bracket):
uri = f"dbscheme://user:pass{bracket}word@host/db"

with pytest.raises(ConfigurationError):
config.GalaxyAppConfiguration(override_tempdir=False, database_connection=uri)

with pytest.raises(ConfigurationError):
config.GalaxyAppConfiguration(override_tempdir=False, install_database_connection=uri)

with pytest.raises(ConfigurationError):
config.GalaxyAppConfiguration(override_tempdir=False, amqp_internal_connection=uri)


class TestIsFetchWithCeleryEnabled:
def test_disabled_if_celery_disabled(self, appconfig):
appconfig.enable_celery_tasks = False
Expand Down

0 comments on commit ee31eca

Please sign in to comment.