Fix UnboundLocalError when reading byte_stream

[ekawas-vrify]

The variable mbox is only assigned if next_shape - b_io.tell() >= 16.

If that condition is not met, mbox is never assigned, but the function always tries to return mbox, ms, which will raise UnboundLocalError if mbox was never set.

2 ways to deal with this:

• Raising an exception with a clear message, or
• Assigning a default value to mbox

I chose to assign None to the variable but I am not sure if this makes sense.

[JamesParrott]

Hi Edward, thanks for this. Great spot. Choosing None makes perfect sense - it matches the signature of mbox in Shape:

pyshp/src/shapefile.py

Line 714 in 6fa929c

mbox: MBox | None = None,

.

Are you able to share a Shapefile, reading which with PyShp causes an UnboundLocalError? It would be great to add that to PyShp's tests, if so.

When the shapefile being read is valid and has mmin and mmax bytes this shouldn't raise an error. But yeah, hands up, you're exactly right. After the refactor into PyShp 3, if a Shapefile without mmin and mmax bytes is read, this function will error now. I shoudl've caught this myself before now, sorry.

Historical context: So this function should only be called on all M and Z Shape types and Multipatch, but not PointM or PointZ (they don't have an m-bounding box for a single float m-value). Before the code base was refactored to facilitate static typing, the code in this 'private' function came from here:

pyshp/shapefile.py

Line 1347 in 23002d8

if shapeType in (13,15,18,23,25,28,31):

Any bytes defining mmin, mmax are read for each shape's raw-binary-record in the Shapefile (and named for clarity). They are optional in the spec, e.g.:

But if I recall correctly and haven't missed something elsewhere in the code, previously PyShp 2 never did anything with them. Now, In PyShp 3, whatever's in the Shapefile for that Shape (valid or not), will go into that Shape. But now, if there're M values but no MMin and MMax in there, an MBox is now computed for M and Z types.

Setting mbox = None only sets this computed mbox, if ms_found is True(thy).

[ekawas-vrify]

Hi Edward, thanks for this. Great spot. Choosing None makes perfect sense - it matches the signature of mbox in Shape:

pyshp/src/shapefile.py

Line 714 in 6fa929c

mbox: MBox | None = None,

.
Are you able to share a Shapefile, reading which with PyShp causes an UnboundLocalError? It would be great to add that to PyShp's tests, if so.

When the shapefile being read is valid and has mmin and mmax bytes this shouldn't raise an error. But yeah, hands up, you're exactly right. After the refactor into PyShp 3, if a Shapefile without mmin and mmax bytes is read, this function will error now. I shoudl've caught this myself before now, sorry.

Historical context: So this function should only be called on all M and Z Shape types and Multipatch, but not PointM or PointZ (they don't have an m-bounding box for a single float m-value). Before the code base was refactored to facilitate static typing, the code in this 'private' function came from here:

pyshp/shapefile.py

Line 1347 in 23002d8

if shapeType in (13,15,18,23,25,28,31):

Any bytes defining mmin, mmax are read for each shape's raw-binary-record in the Shapefile (and named for clarity). They are optional in the spec, e.g.:

But if I recall correctly and haven't missed something elsewhere in the code, previously PyShp 2 never did anything with them. Now, In PyShp 3, whatever's in the Shapefile for that Shape (valid or not), will go into that Shape. But now, if there're M values but no MMin and MMax in there, an MBox is now computed for M and Z types.

Setting mbox = None only sets this computed mbox, if ms_found is True(thy).

To be 100% honest, this error seemed to be triggered in our cloud environment and never locally. I can ask if I could share the shapefile but since it wasn't breaking locally, I couldn't give you a test to reproduce it with.

[JamesParrott]

To be 100% honest, this error seemed to be triggered in our cloud environment and never locally. I can ask if I could share the shapefile but since it wasn't breaking locally, I couldn't give you a test to reproduce it with.

No problem. Maybe there was an older version of PyShp without this bug installed locally. I've opened an issue for anyone to build one.