Skip to content

Commit bb24f03

Browse files
committed
fix(sqlite): preserve filenames when applying URI options
Quote the decoded path before building file: URIs in session and memory services. Otherwise # or ? truncates the filename and drops mode=ro, while literal percent escapes select a different database. Extend public-service regressions across reserved characters, relative and absolute paths, and no-query/read-only/read-write modes. The original head fails 36 cases with 54 passing controls; the fix passes all 90. Affected session, registry, factory and SQLite memory suites: 1039 passed, 1 xfailed on Linux/Python 3.12.14. Changed-file pre-commit and targeted mypy pass. Full tox and Windows were not run.
1 parent ab29d22 commit bb24f03

4 files changed

Lines changed: 123 additions & 31 deletions

File tree

‎src/google/adk/memory/_sqlite_memory_service.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
from typing import Literal
2929
from typing import TYPE_CHECKING
3030
import unicodedata
31+
from urllib.parse import quote
3132
from urllib.parse import unquote
3233
from urllib.parse import urlparse
3334

@@ -140,7 +141,9 @@ def _parse_db_path(db_path: str | Path) -> tuple[str, str, bool]:
140141

141142
if parsed.query:
142143
# sqlite3 only treats the filename as a URI when it starts with `file:`.
143-
return normalized_path, f"file:{normalized_path}?{parsed.query}", True
144+
# Escape filename delimiters and literal percent escapes before URI parsing.
145+
connect_uri = f"file:{quote(normalized_path)}?{parsed.query}"
146+
return normalized_path, connect_uri, True
144147

145148
return normalized_path, normalized_path, False
146149

‎src/google/adk/sessions/sqlite_session_service.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import sqlite3
2525
from typing import Any
2626
from typing import cast
27+
from urllib.parse import quote
2728
from urllib.parse import unquote
2829
from urllib.parse import urlparse
2930

@@ -177,7 +178,9 @@ def _parse_db_path(db_path: str) -> tuple[str, str, bool]:
177178

178179
if parsed.query:
179180
# sqlite3 only treats the filename as a URI when it starts with `file:`.
180-
return normalized_path, f"file:{normalized_path}?{parsed.query}", True
181+
# Escape filename delimiters and literal percent escapes before URI parsing.
182+
connect_uri = f"file:{quote(normalized_path)}?{parsed.query}"
183+
return normalized_path, connect_uri, True
181184

182185
return normalized_path, normalized_path, False
183186

‎tests/unittests/cli/test_service_registry.py‎

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
import os
1516
from pathlib import Path
1617
import sqlite3
1718
from types import SimpleNamespace
@@ -176,25 +177,51 @@ async def test_sqlite_session_uri_preserves_read_only_mode(
176177
await session_service.close()
177178

178179

179-
async def test_sqlite_session_uri_decodes_database_path(
180+
@pytest.mark.parametrize("relative", [False, True])
181+
@pytest.mark.parametrize("mode", [None, "ro", "rw"])
182+
@pytest.mark.parametrize(
183+
"filename",
184+
[
185+
"sessions #1.db",
186+
pytest.param(
187+
"sessions ?1.db",
188+
marks=pytest.mark.skipif(
189+
os.name == "nt",
190+
reason="Question marks are not valid Windows filenames",
191+
),
192+
),
193+
"sessions %23.db",
194+
"sessions +&= ü.db",
195+
"sessions.db",
196+
],
197+
)
198+
async def test_sqlite_session_uri_preserves_path_and_mode(
180199
registry: service_registry.ServiceRegistry,
181200
monkeypatch: pytest.MonkeyPatch,
182201
tmp_path: Path,
202+
relative: bool,
203+
mode: str | None,
204+
filename: str,
183205
) -> None:
184-
"""An encoded SQLite URI opens the existing database at its decoded path."""
206+
"""Encoded filenames select the same database and retain URI write permissions."""
207+
monkeypatch.chdir(tmp_path)
185208
monkeypatch.setattr(
186209
"google.adk.sessions.sqlite_session_service.SqliteSessionService",
187210
SqliteSessionService,
188211
)
189-
db_path = tmp_path / "sessions #1.db"
212+
db_path = tmp_path / filename
190213
writable_service = SqliteSessionService(str(db_path))
191-
session = await writable_service.create_session(
192-
app_name="app", user_id="user", state={"count": 1}
193-
)
194-
await writable_service.close()
214+
try:
215+
session = await writable_service.create_session(
216+
app_name="app", user_id="user", state={"count": 1}
217+
)
218+
finally:
219+
await writable_service.close()
195220

221+
uri_path = filename if relative else db_path.as_posix()
222+
query = f"?mode={mode}&cache=shared" if mode else ""
196223
session_service = registry.create_session_service(
197-
f"sqlite:///{quote(db_path.as_posix())}"
224+
f"sqlite:///{quote(uri_path)}{query}"
198225
)
199226
assert isinstance(session_service, SqliteSessionService)
200227
try:
@@ -203,9 +230,25 @@ async def test_sqlite_session_uri_decodes_database_path(
203230
)
204231
assert stored_session is not None
205232
assert stored_session.state == {"count": 1}
233+
if mode == "ro":
234+
with pytest.raises(sqlite3.OperationalError, match="readonly"):
235+
await session_service.create_session(app_name="app", user_id="user")
236+
else:
237+
await session_service.create_session(
238+
app_name="app", user_id="user", session_id="new-session"
239+
)
206240
finally:
207241
await session_service.close()
208242

243+
reopened_service = SqliteSessionService(str(db_path))
244+
try:
245+
added_session = await reopened_service.get_session(
246+
app_name="app", user_id="user", session_id="new-session"
247+
)
248+
assert (added_session is None) == (mode == "ro")
249+
finally:
250+
await reopened_service.close()
251+
209252

210253
def test_create_session_service_sqlite_ignores_unsupported_kwargs(
211254
registry, mock_services, caplog

‎tests/unittests/memory/test_sqlite_memory_service.py‎

Lines changed: 64 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@
1717
import asyncio
1818
from contextlib import closing
1919
from datetime import datetime
20+
import os
2021
from pathlib import Path
2122
import sqlite3
2223
from typing import Literal
24+
from urllib.parse import quote
2325

2426
import aiosqlite
2527
from google.adk.events.event import Event
@@ -158,36 +160,77 @@ async def test_in_memory_uri_variants(uri):
158160
await service.close()
159161

160162

161-
@pytest.mark.asyncio
163+
@pytest.mark.parametrize("scheme", ["sqlite", "sqlite+aiosqlite"])
164+
@pytest.mark.parametrize("relative", [False, True])
165+
@pytest.mark.parametrize("mode", [None, "ro", "rw"])
162166
@pytest.mark.parametrize(
163-
"ro_uri_fn",
167+
"filename",
164168
[
165-
lambda p: f"sqlite:///{p / 'ro_abs.db'}?mode=ro",
166-
lambda p: f"sqlite+aiosqlite:///{p / 'ro_abs.db'}?mode=ro",
169+
"memory #1.db",
170+
pytest.param(
171+
"memory ?1.db",
172+
marks=pytest.mark.skipif(
173+
os.name == "nt",
174+
reason="Question marks are not valid Windows filenames",
175+
),
176+
),
177+
"memory %23.db",
178+
"memory +&= ü.db",
179+
"memory.db",
167180
],
168181
)
169-
async def test_readonly_uri_mode_restricts_writes(tmp_path, ro_uri_fn):
170-
"""Read-only URI query parameter enables searching existing rows and rejects new writes."""
171-
db_path = tmp_path / "ro_abs.db"
182+
async def test_sqlite_uri_preserves_path_and_mode(
183+
tmp_path: Path,
184+
monkeypatch: pytest.MonkeyPatch,
185+
scheme: str,
186+
relative: bool,
187+
mode: str | None,
188+
filename: str,
189+
) -> None:
190+
"""Encoded filenames select the same memories and retain URI write permissions."""
191+
monkeypatch.chdir(tmp_path)
192+
db_path = tmp_path / filename
172193
rw_service = SqliteMemoryService(db_path=db_path, fts="off")
173-
event = _make_event("user", "readonly test content", 1.0)
174-
await rw_service.add_events_to_memory(
175-
app_name="app", user_id="user", events=[event], session_id="s1"
176-
)
194+
event = _make_event("user", "saved content", 1.0, event_id="saved")
195+
try:
196+
await rw_service.add_events_to_memory(
197+
app_name="app", user_id="user", events=[event], session_id="s1"
198+
)
199+
finally:
200+
await rw_service.close()
177201

178-
ro_uri = ro_uri_fn(tmp_path)
179-
ro_service = SqliteMemoryService(ro_uri, fts="off")
180-
resp = await ro_service.search_memory(
181-
app_name="app", user_id="user", query="readonly"
202+
uri_path = filename if relative else db_path.as_posix()
203+
query = f"?mode={mode}&cache=shared" if mode else ""
204+
service = SqliteMemoryService(
205+
f"{scheme}:///{quote(uri_path)}{query}", fts="off"
182206
)
183-
assert len(resp.memories) == 1
184-
assert resp.memories[0].content.parts[0].text == "readonly test content"
207+
try:
208+
response = await service.search_memory(
209+
app_name="app", user_id="user", query="saved"
210+
)
211+
assert len(response.memories) == 1
212+
assert response.memories[0].content.parts[0].text == "saved content"
213+
new_event = _make_event("user", "new content", 2.0, event_id="new")
214+
if mode == "ro":
215+
with pytest.raises(sqlite3.OperationalError, match="readonly"):
216+
await service.add_events_to_memory(
217+
app_name="app", user_id="user", events=[new_event], session_id="s1"
218+
)
219+
else:
220+
await service.add_events_to_memory(
221+
app_name="app", user_id="user", events=[new_event], session_id="s1"
222+
)
223+
finally:
224+
await service.close()
185225

186-
with pytest.raises((sqlite3.OperationalError, aiosqlite.OperationalError)):
187-
new_event = _make_event("user", "cannot write", 2.0)
188-
await ro_service.add_events_to_memory(
189-
app_name="app", user_id="user", events=[new_event], session_id="s1"
226+
reopened_service = SqliteMemoryService(db_path=db_path, fts="off")
227+
try:
228+
response = await reopened_service.search_memory(
229+
app_name="app", user_id="user", query="new"
190230
)
231+
assert len(response.memories) == (0 if mode == "ro" else 1)
232+
finally:
233+
await reopened_service.close()
191234

192235

193236
def test_sqlite_uri_does_not_create_sqlite_colon_directory(tmp_path):

0 commit comments

Comments
 (0)