Skip to content

Commit

Permalink
Limit validity of URIFilename
Browse files Browse the repository at this point in the history
Fixes #501
  • Loading branch information
rogerbinns committed Nov 28, 2023
1 parent 465fea4 commit 40b52db
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 7 deletions.
4 changes: 3 additions & 1 deletion apsw/__init__.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -2222,7 +2222,9 @@ class URIFilename:
or the main database flag is set.
You can safely pass it on to the :class:`VFSFile` constructor
which knows how to get the name back out."""
which knows how to get the name back out. The URIFilename is
only valid for the duration of the xOpen call. If you save
and use the object later you will get an exception."""
def filename(self) -> str:
"""Returns the filename."""
...
Expand Down
37 changes: 33 additions & 4 deletions apsw/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -5374,8 +5374,7 @@ def sourceCheckFunction(self, filename, name, lines):
f"file { filename } function { name } calls PyGILState_Ensure but does not have MakeExistingException"
)
# not further checked
if name.split("_")[0] in ("ZeroBlobBind", "APSWVFS", "APSWVFSFile", "APSWBuffer", "FunctionCBInfo",
"apswurifilename"):
if name.split("_")[0] in ("ZeroBlobBind", "APSWVFS", "APSWVFSFile", "APSWBuffer", "FunctionCBInfo"):
return

checks = {
Expand Down Expand Up @@ -5457,6 +5456,11 @@ def sourceCheckFunction(self, filename, name, lines):
"apswfcntl": {
"req": {}
},
"apswurifilename": {
"req": {
"check": "CHECK_SCOPE"
}
}
}

prefix, base = name.split("_", 1)
Expand Down Expand Up @@ -10400,13 +10404,17 @@ def __init__(self):

def xOpen(self, name, flags):
assert isinstance(name, apsw.URIFilename)
name_catch.append(name)
name_catch.append((name, str(name)))
raise apsw.SQLError()

t = TVFS()

with contextlib.suppress(apsw.SQLError):
apsw.Connection("/tmp/uri_test", vfs="uritest")

self.assertEqual(len(name_catch), 1)
uriname, urinamestr=name_catch[0]

objects = (self.db,
db2,
db3,
Expand All @@ -10416,7 +10424,7 @@ def xOpen(self, name, flags):
apsw.zeroblob(3),
blob,
blob2,
name_catch[0],
uriname,
apsw.VFS("aname", ""),
apsw.VFSFile("", self.db.db_filename("main"),
[apsw.SQLITE_OPEN_MAIN_DB | apsw.SQLITE_OPEN_CREATE | apsw.SQLITE_OPEN_READWRITE, 0]),
Expand All @@ -10425,6 +10433,27 @@ def xOpen(self, name, flags):
)
for o in objects:
self.assertNotEqual(repr(o), str(o))
# issue 501
if isinstance(o, apsw.URIFilename):
self.assertNotEqual(repr(o), urinamestr)
self.assertNotEqual(str(o), urinamestr)

# more issue 501
with contextlib.suppress(ValueError):
uriname.filename()
1/0
with contextlib.suppress(ValueError):
uriname.parameters
1/0
with contextlib.suppress(ValueError):
uriname.uri_boolean("name", False)
1/0
with contextlib.suppress(ValueError):
uriname.uri_int("name", 0)
1/0
with contextlib.suppress(ValueError):
uriname.uri_parameter("name")
1/0

# This test is run last by deliberate name choice. If it did
# uncover any bugs there isn't much that can be done to turn the
Expand Down
4 changes: 4 additions & 0 deletions doc/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ Provide detail when C implemented objects are printed. For example

Added :meth:`URIFilename.parameters` (:issue:`496`)

:class:`URIFilename` are only valid for the duration of the
:meth:`VFS.xOpen` call. If you save and use the object later you will
get an exception. (:issue:`501`)

3.44.0.0
========

Expand Down
4 changes: 3 additions & 1 deletion src/apsw.docstrings
Original file line number Diff line number Diff line change
Expand Up @@ -2865,7 +2865,9 @@
"or the main database flag is set.\n" \
"\n" \
"You can safely pass it on to the :class:`VFSFile` constructor\n" \
"which knows how to get the name back out.\n"
"which knows how to get the name back out. The URIFilename is\n" \
"only valid for the duration of the xOpen call. If you save\n" \
"and use the object later you will get an exception.\n"

#define URIFilename_filename_DOC "filename($self)\n--\n\nURIFilename.filename() -> str\n\n" \
"Returns the filename.\n"
Expand Down
24 changes: 23 additions & 1 deletion src/vfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,9 @@ apswvfs_xOpen(sqlite3_vfs *vfs, const char *zName, sqlite3_file *file, int infla
PyObject *vargs[] = {NULL, (PyObject *)(vfs->pAppData), nameobject, flags};
if (vargs[2] && vargs[3])
pyresult = PyObject_VectorcallMethod(apst.xOpen, vargs + 1, 3 | PY_VECTORCALL_ARGUMENTS_OFFSET, NULL);
/* issue 501 */
if (inflags & (SQLITE_OPEN_URI | SQLITE_OPEN_MAIN_DB))
((APSWURIFilename *)nameobject)->filename = 0;
if (!pyresult)
{
result = MakeSqliteMsgFromPyException(NULL);
Expand Down Expand Up @@ -3031,16 +3034,26 @@ static PyTypeObject APSWVFSFileType =
or the main database flag is set.
You can safely pass it on to the :class:`VFSFile` constructor
which knows how to get the name back out.
which knows how to get the name back out. The URIFilename is
only valid for the duration of the xOpen call. If you save
and use the object later you will get an exception.
*/

#define CHECK_SCOPE \
do \
{ \
if (!self->filename) \
return PyErr_Format(PyExc_ValueError, "URIFilename is out of scope"); \
} while (0)

/** .. method:: filename() -> str
Returns the filename.
*/
static PyObject *
apswurifilename_filename(APSWURIFilename *self)
{
CHECK_SCOPE;
return convertutf8string(self->filename);
}

Expand All @@ -3054,6 +3067,7 @@ apswurifilename_filename(APSWURIFilename *self)
static PyObject *
apswurifilename_parameters(APSWURIFilename *self)
{
CHECK_SCOPE;
int i, count = 0;
for (i = 0;; i++)
if (!sqlite3_uri_key(self->filename, i))
Expand Down Expand Up @@ -3088,6 +3102,7 @@ apswurifilename_parameters(APSWURIFilename *self)
static PyObject *
apswurifilename_uri_parameter(APSWURIFilename *self, PyObject *const *fast_args, Py_ssize_t fast_nargs, PyObject *fast_kwnames)
{
CHECK_SCOPE;
const char *res, *name;
{
URIFilename_uri_parameter_CHECK;
Expand All @@ -3109,6 +3124,7 @@ apswurifilename_uri_parameter(APSWURIFilename *self, PyObject *const *fast_args,
static PyObject *
apswurifilename_uri_int(APSWURIFilename *self, PyObject *const *fast_args, Py_ssize_t fast_nargs, PyObject *fast_kwnames)
{
CHECK_SCOPE;
const char *name = NULL;
long long res = 0, default_;

Expand All @@ -3134,6 +3150,7 @@ apswurifilename_uri_int(APSWURIFilename *self, PyObject *const *fast_args, Py_ss
static PyObject *
apswurifilename_uri_boolean(APSWURIFilename *self, PyObject *const *fast_args, Py_ssize_t fast_nargs, PyObject *fast_kwnames)
{
CHECK_SCOPE;
const char *name = NULL;
int default_ = 0, res;

Expand All @@ -3155,6 +3172,9 @@ apswurifilename_uri_boolean(APSWURIFilename *self, PyObject *const *fast_args, P
static PyObject *
apswurifilename_tp_str(APSWURIFilename *self)
{
/* CHECK_SCOPE not needed since we manually check */
if (!self->filename)
return PyUnicode_FromFormat("<apsw.URIFilename object (out of scope) at %p>", self);
return PyUnicode_FromFormat("<apsw.URIFilename object \"%s\" at %p>", self->filename, self);
}

Expand Down Expand Up @@ -3182,3 +3202,5 @@ static PyTypeObject APSWURIFilenameType =
.tp_str = (reprfunc)apswurifilename_tp_str,
.tp_getset = APSWURIFilename_getset,
};

#undef CHECK_SCOPE

0 comments on commit 40b52db

Please sign in to comment.