From 8a3bb5e67f67750e90f483201e5e2a090e627f69 Mon Sep 17 00:00:00 2001 From: Bedram Tamang Date: Thu, 23 Jul 2026 14:44:59 -0700 Subject: [PATCH 1/2] fix(orm): harden ConnectionFactory against bad port and unknown driver build_url()/make() previously surfaced bad connection configs as raw, low-level errors: - mysql/postgres with a missing/empty 'port' produced a URL with a bare trailing colon that crashed at engine creation with ValueError: invalid literal for int() with base 10: ''. - an unsupported driver raised a raw KeyError from DRIVER_URLS[driver]. - make()'s 'Unsupported driver' fallback was unreachable dead code, since create_engine()/build_url() always raised the raw KeyError first. Hardening: - Add per-driver default ports (mysql 3306, postgres 5432); an explicit port still takes precedence, so the happy path is byte-for-byte unchanged. - build_url() and make() now raise the existing framework-level DriverNotFound with a message listing the supported drivers. The check in make() runs before any engine is built, so it is reachable and meaningful; the dead match fallback is removed in favour of a CONNECTIONS lookup. The documented 'url' passthrough still short-circuits field assembly, so supplying a full url bypasses both driver and port validation. Regression tests cover: mysql/postgres missing and empty-string ports, explicit-port precedence, unsupported driver via build_url() and make(), url passthrough, and unchanged valid sqlite/mysql/postgres URLs. --- .../masoniteorm/connections/factory.py | 43 +++++--- .../tests/masoniteorm/config/test_db_url.py | 99 +++++++++++++++++++ .../models/test_model_attributes.py | 12 +-- 3 files changed, 136 insertions(+), 18 deletions(-) diff --git a/fastapi_startkit/src/fastapi_startkit/masoniteorm/connections/factory.py b/fastapi_startkit/src/fastapi_startkit/masoniteorm/connections/factory.py index 04a68781..4abcf407 100644 --- a/fastapi_startkit/src/fastapi_startkit/masoniteorm/connections/factory.py +++ b/fastapi_startkit/src/fastapi_startkit/masoniteorm/connections/factory.py @@ -4,6 +4,7 @@ from sqlalchemy.pool import NullPool from sqlalchemy.ext.asyncio import create_async_engine, AsyncEngine +from fastapi_startkit.exceptions.exceptions import DriverNotFound from fastapi_startkit.masoniteorm.connections.connection import Connection from fastapi_startkit.masoniteorm.connections.sqlite_connection import SQliteConnection from fastapi_startkit.masoniteorm.connections.postgres_connection import ( @@ -19,13 +20,35 @@ class ConnectionFactory: "postgres": "postgresql+asyncpg", } + # Sensible per-driver defaults so a missing/empty "port" no longer surfaces as a + # raw SQLAlchemy `ValueError: invalid literal for int()` at engine-creation time. + DEFAULT_PORTS = { + "mysql": 3306, + "postgres": 5432, + } + + CONNECTIONS = { + "sqlite": SQliteConnection, + "mysql": MySQLConnection, + "postgres": PostgresConnection, + } + + @classmethod + def _unsupported_driver_error(cls, driver: Any) -> DriverNotFound: + supported = ", ".join(sorted(cls.DRIVER_URLS)) + return DriverNotFound( + f"Unsupported database driver {driver!r}. Supported drivers are: {supported}." + ) + @classmethod def build_url(cls, config: dict) -> str: if url := config.get("url"): return str(url) driver = config["driver"] - scheme = cls.DRIVER_URLS[driver] + scheme = cls.DRIVER_URLS.get(driver) + if scheme is None: + raise cls._unsupported_driver_error(driver) db = config.get("database", "") if driver == "sqlite": @@ -34,7 +57,7 @@ def build_url(cls, config: dict) -> str: user = config.get("username", "") pwd = config.get("password", "") host = config.get("host", "localhost") - port = config.get("port", "") + port = config.get("port") or cls.DEFAULT_PORTS[driver] return f"{scheme}://{user}:{pwd}@{host}:{port}/{db}" @classmethod @@ -50,15 +73,11 @@ def create_engine(cls, cfg: dict) -> AsyncEngine: kwargs["poolclass"] = StaticPool return create_async_engine(url, **kwargs) - def make(self, config: dict, name: str) -> type[Connection]: - engine = self.create_engine(config) + def make(self, config: dict, name: str) -> Connection: driver = config["driver"] - match driver: - case "sqlite": - return SQliteConnection(engine, config) - case "postgres": - return PostgresConnection(engine, config) - case "mysql": - return MySQLConnection(engine, config) + connection_class = type(self).CONNECTIONS.get(driver) + if connection_class is None: + raise type(self)._unsupported_driver_error(driver) - raise ValueError(f"Unsupported driver: {driver}") + engine = self.create_engine(config) + return connection_class(engine, config) diff --git a/fastapi_startkit/tests/masoniteorm/config/test_db_url.py b/fastapi_startkit/tests/masoniteorm/config/test_db_url.py index e094d51a..56de4253 100644 --- a/fastapi_startkit/tests/masoniteorm/config/test_db_url.py +++ b/fastapi_startkit/tests/masoniteorm/config/test_db_url.py @@ -1,5 +1,6 @@ import unittest +from fastapi_startkit.exceptions.exceptions import DriverNotFound from fastapi_startkit.masoniteorm.connections.factory import ConnectionFactory @@ -76,3 +77,101 @@ def test_direct_url_passthrough_takes_precedence(self): } url = ConnectionFactory.build_url(config) self.assertEqual(url, "mysql+aiomysql://admin:pw@prod-host:3306/live") + + +class TestConnectionFactoryMissingPort(unittest.TestCase): + """A missing/empty port must fall back to a sane per-driver default instead of + producing a URL with a bare trailing colon that crashes at engine creation with + `ValueError: invalid literal for int() with base 10: ''`.""" + + def test_mysql_missing_port_uses_default_3306(self): + config = { + "driver": "mysql", + "host": "localhost", + "database": "mydb", + "username": "root", + "password": "secret", + } + url = ConnectionFactory.build_url(config) + self.assertEqual(url, "mysql+aiomysql://root:secret@localhost:3306/mydb") + + def test_postgres_missing_port_uses_default_5432(self): + config = { + "driver": "postgres", + "host": "db.example.com", + "database": "mydb", + "username": "user", + "password": "pass", + } + url = ConnectionFactory.build_url(config) + self.assertEqual(url, "postgresql+asyncpg://user:pass@db.example.com:5432/mydb") + + def test_mysql_empty_string_port_uses_default(self): + config = { + "driver": "mysql", + "host": "localhost", + "port": "", + "database": "mydb", + "username": "root", + "password": "secret", + } + url = ConnectionFactory.build_url(config) + self.assertEqual(url, "mysql+aiomysql://root:secret@localhost:3306/mydb") + + def test_postgres_empty_string_port_uses_default(self): + config = { + "driver": "postgres", + "host": "localhost", + "port": "", + "database": "mydb", + "username": "user", + "password": "pass", + } + url = ConnectionFactory.build_url(config) + self.assertEqual(url, "postgresql+asyncpg://user:pass@localhost:5432/mydb") + + def test_explicit_port_is_preserved(self): + """An explicitly configured port must win over the default (happy path).""" + config = { + "driver": "postgres", + "host": "localhost", + "port": 6543, + "database": "mydb", + "username": "user", + "password": "pass", + } + url = ConnectionFactory.build_url(config) + self.assertEqual(url, "postgresql+asyncpg://user:pass@localhost:6543/mydb") + + +class TestConnectionFactoryUnsupportedDriver(unittest.TestCase): + """An unknown driver must raise a friendly framework error, not a raw KeyError.""" + + def test_build_url_unknown_driver_raises_driver_not_found(self): + with self.assertRaises(DriverNotFound) as ctx: + ConnectionFactory.build_url({"driver": "oracle", "database": "mydb"}) + message = str(ctx.exception) + self.assertIn("oracle", message) + # The message should guide the user toward the supported drivers. + self.assertIn("sqlite", message) + self.assertIn("mysql", message) + self.assertIn("postgres", message) + + def test_build_url_does_not_raise_raw_keyerror(self): + with self.assertRaises(DriverNotFound): + ConnectionFactory.build_url({"driver": "cassandra"}) + + def test_make_unknown_driver_raises_driver_not_found(self): + """make()'s driver check is reachable and fires before any engine is built.""" + with self.assertRaises(DriverNotFound): + ConnectionFactory().make({"driver": "oracle", "database": "mydb"}, "oracle") + + def test_url_passthrough_bypasses_driver_validation(self): + """The documented `url` stopgap short-circuits field assembly entirely, so an + unknown driver name is irrelevant when a full url is supplied.""" + config = { + "driver": "oracle", + "url": "sqlite+aiosqlite:///db.sqlite3", + } + url = ConnectionFactory.build_url(config) + self.assertEqual(url, "sqlite+aiosqlite:///db.sqlite3") diff --git a/fastapi_startkit/tests/masoniteorm/models/test_model_attributes.py b/fastapi_startkit/tests/masoniteorm/models/test_model_attributes.py index 40a4b5bf..b3957105 100644 --- a/fastapi_startkit/tests/masoniteorm/models/test_model_attributes.py +++ b/fastapi_startkit/tests/masoniteorm/models/test_model_attributes.py @@ -1,8 +1,8 @@ import pendulum import pytest -from unittest.mock import MagicMock, patch from fastapi_startkit.carbon import Carbon +from fastapi_startkit.exceptions.exceptions import DriverNotFound from fastapi_startkit.masoniteorm.models.fields import DateTimeField from fastapi_startkit.masoniteorm.connections.factory import ConnectionFactory from fastapi_startkit.masoniteorm.connections.manager import DatabaseManager @@ -86,8 +86,9 @@ def test_connection_is_cached(self, db): assert conn1 is conn2 def test_connection_raises_for_missing_driver(self): - # create_engine is called before the driver switch, so mock it to isolate - # the "Unsupported driver" branch in ConnectionFactory.make(). + # make() now validates the driver before any engine is built, so an + # unsupported driver fails fast with a friendly framework error rather + # than a raw KeyError/ValueError from deep inside SQLAlchemy. factory = ConnectionFactory() bad_config = { "default": "mssql", @@ -96,9 +97,8 @@ def test_connection_raises_for_missing_driver(self): }, } dm = DatabaseManager(factory, bad_config) - with patch.object(ConnectionFactory, "create_engine", return_value=MagicMock()): - with pytest.raises(ValueError, match="Unsupported driver"): - dm.connection("mssql") + with pytest.raises(DriverNotFound, match="mssql"): + dm.connection("mssql") # --------------------------------------------------------------------------- From 758dd08c516b8f30ba0bb11901baa2d5bf73d4bf Mon Sep 17 00:00:00 2001 From: Bedram Tamang Date: Thu, 23 Jul 2026 14:49:44 -0700 Subject: [PATCH 2/2] style: satisfy ruff format in factory.py --- .../src/fastapi_startkit/masoniteorm/connections/factory.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/fastapi_startkit/src/fastapi_startkit/masoniteorm/connections/factory.py b/fastapi_startkit/src/fastapi_startkit/masoniteorm/connections/factory.py index 4abcf407..e193b8c7 100644 --- a/fastapi_startkit/src/fastapi_startkit/masoniteorm/connections/factory.py +++ b/fastapi_startkit/src/fastapi_startkit/masoniteorm/connections/factory.py @@ -36,9 +36,7 @@ class ConnectionFactory: @classmethod def _unsupported_driver_error(cls, driver: Any) -> DriverNotFound: supported = ", ".join(sorted(cls.DRIVER_URLS)) - return DriverNotFound( - f"Unsupported database driver {driver!r}. Supported drivers are: {supported}." - ) + return DriverNotFound(f"Unsupported database driver {driver!r}. Supported drivers are: {supported}.") @classmethod def build_url(cls, config: dict) -> str: