Skip to content

Commit

Permalink
Fix off-by-one NamedTupleProxy array bounds check
Browse files Browse the repository at this point in the history
Add to an existing test case to exercise this: previously accessing
r.name would crash, but r.score would raise an exception as expected.
Fixes #1279.
  • Loading branch information
jmarshall committed Apr 19, 2024
1 parent d32073b commit 7907822
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
4 changes: 2 additions & 2 deletions pysam/libctabixproxies.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -365,14 +365,14 @@ cdef class NamedTupleProxy(TupleProxy):
'''set attribute.'''
cdef int idx
idx, f = self.map_key2field[key]
if self.nfields < idx:
if idx >= self.nfields:
raise KeyError("field %s not set" % key)
TupleProxy.__setitem__(self, idx, str(value))

def __getattr__(self, key):
cdef int idx
idx, f = self.map_key2field[key]
if self.nfields < idx:
if idx >= self.nfields:
raise KeyError("field %s not set" % key)
if f == str:
return force_str(self.fields[idx],
Expand Down
9 changes: 9 additions & 0 deletions tests/tabix_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,9 @@ def testRead(self):
self.assertEqual(c[0], r.contig)
self.assertEqual(int(c[1]), r.start)
self.assertEqual(int(c[2]), r.end)
# Needs lambda so that the property getter isn't called too soon
self.assertRaises(KeyError, lambda: r.name)
self.assertRaises(KeyError, lambda: r.score)
self.assertEqual(list(c), list(r))
self.assertEqual("\t".join(map(str, c)),
str(r))
Expand All @@ -524,6 +527,12 @@ def testWrite(self):
self.assertEqual(int(c[2]) + 1, r.end)
self.assertEqual(str(int(c[2]) + 1), r[2])

with self.assertRaises(IndexError):
r.name = "test"

with self.assertRaises(IndexError):
r.score = 1


class TestVCF(unittest.TestCase):

Expand Down

0 comments on commit 7907822

Please sign in to comment.