Skip to content

Commit bd24331

Browse files
authored
prevent scoped_session closing the connection when there are active sessions (#1786)
### Feature or Bugfix - Bugfix ### Detail Currently if there are nested scoped_session is calls the inner call will close the connection and wipe all the tracked objects. As a result post commit modifications to the objects will not be commited properly. The following code demostrates the issue... ```python import datetime from sqlalchemy import Column, Integer, String from sqlalchemy.ext.declarative import declarative_base from dataall.base.db import get_engine # Create base class for declarative models Base = declarative_base() # Define a sample model class TestUser(Base): __tablename__ = 'test_users' id = Column(Integer, primary_key=True) name = Column(String) email = Column(String) test_dt = Column(String) # Create engine and connect to database # engine = create_engine('postgresql://postgres:docker@localhost:5432/dataall') engine = get_engine(envname='dkrcompose') # Create tables Base.metadata.create_all(engine.engine) # Create session factory with engine.scoped_session() as session: new_user = TestUser(name='John Doe', email='john@example.com') session.add(new_user) session.commit() with engine.scoped_session() as session2: # end of this block will call session.close() and will wipe the tracked objects (new_user) new_user = session2.query(TestUser).filter_by(name='John Doe').first() # this change will not be commited because `new_user` is no longer tracked by the session new_user.name = 'Jane Doe' new_user.test_dt = datetime.datetime.now() ``` The fix provided here is to introduce an active_session counter and only close the connection when all sessions are done. ### Security Please answer the questions below briefly where applicable, or write `N/A`. Based on [OWASP 10](https://owasp.org/Top10/en/). - Does this PR introduce or modify any input fields or queries - this includes fetching data from storage outside the application (e.g. a database, an S3 bucket)? - Is the input sanitized? - What precautions are you taking before deserializing the data you consume? - Is injection prevented by parametrizing queries? - Have you ensured no `eval` or similar functions are used? - Does this PR introduce any functionality or component that requires authorization? - How have you ensured it respects the existing AuthN/AuthZ mechanisms? - Are you logging failed auth attempts? - Are you using or adding any cryptographic features? - Do you use a standard proven implementations? - Are the used keys controlled by the customer? Where are they stored? - Are you introducing any new policies/roles/users? - Have you used the least-privilege principle? How? By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 99e9ec6 commit bd24331

1 file changed

Lines changed: 6 additions & 2 deletions

File tree

backend/dataall/base/db/connection.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
from dataall.base.db import Base
1212
from dataall.base.db.dbconfig import DbConfig
1313
from dataall.base.utils import Parameter
14-
from dataall.base.aws.sts import SessionHelper
1514

1615
try:
1716
from urllib import quote_plus, unquote_plus
@@ -41,6 +40,7 @@ def __init__(self, dbconfig: DbConfig):
4140

4241
self.sessions = {}
4342
self._session = None
43+
self._active_sessions = 0
4444

4545
def session(self):
4646
if self._session is None:
@@ -52,13 +52,17 @@ def session(self):
5252
def scoped_session(self):
5353
s = self.session()
5454
try:
55+
self._active_sessions += 1
5556
yield s
5657
s.commit()
5758
except Exception as e:
5859
s.rollback()
5960
raise e
6061
finally:
61-
s.close()
62+
self._active_sessions -= 1
63+
if self._active_sessions == 0:
64+
s.close()
65+
self._session = None
6266

6367
def dispose(self):
6468
self.engine.dispose()

0 commit comments

Comments
 (0)