Opened 2 years ago
Closed 2 years ago
#34026 closed Bug (fixed)
WKBReader.read() crashes on strings.
Reported by: | Benoît Vinot | Owned by: | Leo Tom |
---|---|---|---|
Component: | GIS | Version: | 4.1 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
Hi,
There is a potential type error in the implementation of the read
method of the WKBReader
class in contrib.gis
. Here is a small example to create the problem:
from django.contrib.gis.geos.prototypes.io import _WKBReader wkb='0101000020E6100000C07DD40B0FA8F5BF8794C5DD53DD4740' # <-- string _WKBReader().read(wkb)
This code works fine:
from django.contrib.gis.geos.prototypes.io import _WKBReader wkb='0101000020E6100000C07DD40B0FA8F5BF8794C5DD53DD4740' _WKBReader().read(wkb.encode()) # <--- Converted into bytes
I suggest replacing the current implementation of this method by:
def read(self, wkb): "Return a _pointer_ to C GEOS Geometry object from the given WKB." if isinstance(wkb, memoryview): wkb_s = bytes(wkb) return wkb_reader_read(self.ptr, wkb_s, len(wkb_s)) elif isinstance(wkb, bytes): return wkb_reader_read_hex(self.ptr, wkb, len(wkb)) elif isinstance(wkb, str): wkb_s = wkb.encode() # <------------------- Ensure bytes return wkb_reader_read_hex(self.ptr, wkb_s, len(wkb_s)) else: raise TypeError
Change History (15)
comment:1 by , 2 years ago
Description: | modified (diff) |
---|
comment:2 by , 2 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 2 years ago
Summary: | WKBReader Type Error → WKBReader.read() crashes on strings. |
---|
follow-up: 7 comment:5 by , 2 years ago
Replying to Benoît Vinot:
Thanks. Do you want me to create a PR?
comment:6 by , 2 years ago
comment:7 by , 2 years ago
Replying to Leo Tom:
Replying to Benoît Vinot:
Thanks. Do you want me to create a PR?
Can I claim and work on this issue ?
Yes, if you want. :-)
comment:8 by , 2 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:9 by , 2 years ago
Has patch: | set |
---|
follow-up: 12 comment:11 by , 2 years ago
Needs tests: | set |
---|
Please uncheck "Needs tests" when you add one to the PR.
comment:12 by , 2 years ago
Replying to Tim Graham:
Please uncheck "Needs tests" when you add one to the PR.
I just have to write a test in django/tests/gis_tests/geos_tests/test_io.py right. So all I have to do is inside the test03_wkbreader function add a string as well right?
comment:13 by , 2 years ago
Needs tests: | unset |
---|
comment:14 by , 2 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Agreed, it doesn't work as documented.