Skip to content

Commit

Permalink
Revert changes from #418 (#426)
Browse files Browse the repository at this point in the history
  • Loading branch information
caspervdw committed Nov 11, 2021
1 parent ddb440b commit 327c919
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 147 deletions.
3 changes: 1 addition & 2 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ Version 0.12 (unreleased)

**Bug fixes**

* Protect ``pygeos.from_geojson`` against segfaults by running the function in a
subprocess (GEOS 3.10.0 only) (#418).
* ...


**Acknowledgments**
Expand Down
18 changes: 3 additions & 15 deletions pygeos/io.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@
import numpy as np

from . import Geometry # noqa
from . import geos_capi_version_string, geos_version_string, lib
from . import geos_capi_version_string, lib
from .decorators import requires_geos
from .enum import ParamEnum
from .may_segfault import may_segfault

__all__ = [
"from_geojson",
Expand Down Expand Up @@ -408,19 +407,14 @@ def from_wkb(geometry, on_invalid="raise", **kwargs):
return lib.from_wkb(geometry, invalid_handler, **kwargs)


@requires_geos("3.10.0")
@requires_geos("3.10.1")
def from_geojson(geometry, on_invalid="raise", **kwargs):
"""Creates geometries from GeoJSON representations (strings).
If a GeoJSON is a FeatureCollection, it is read as a single geometry
(with type GEOMETRYCOLLECTION). This may be unpacked using the ``pygeos.get_parts``.
Properties are not read.
.. note::
For GEOS 3.10.0, this function is executed in a subprocess. This is because invalid
GeoJSON input may result in a crash. For GEOS 3.10.1 the issue is expected to be fixed.
The GeoJSON format is defined in `RFC 7946 <https://geojson.org/>`__.
The following are currently unsupported:
Expand Down Expand Up @@ -463,13 +457,7 @@ def from_geojson(geometry, on_invalid="raise", **kwargs):
# of array elements)
geometry = np.asarray(geometry, dtype=object)

# GEOS 3.10.0 may segfault on invalid GeoJSON input. This bug is currently
# solved in main branch, expected fix in (3, 10, 1)
if geos_version_string == "3.10.0": # so not on dev versions!
_from_geojson = may_segfault(lib.from_geojson)
else:
_from_geojson = lib.from_geojson
return _from_geojson(geometry, invalid_handler, **kwargs)
return lib.from_geojson(geometry, invalid_handler, **kwargs)


def from_shapely(geometry, **kwargs):
Expand Down
76 changes: 0 additions & 76 deletions pygeos/may_segfault.py

This file was deleted.

20 changes: 10 additions & 10 deletions pygeos/tests/test_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ def test_pickle_with_srid():
assert pygeos.get_srid(pickle.loads(pickled)) == 4326


@pytest.mark.skipif(pygeos.geos_version < (3, 10, 0), reason="GEOS < 3.10")
@pytest.mark.skipif(pygeos.geos_version < (3, 10, 1), reason="GEOS < 3.10.1")
@pytest.mark.parametrize(
"geojson,expected",
[
Expand All @@ -761,7 +761,7 @@ def test_from_geojson(geojson, expected):
assert_geometries_equal(actual, expected)


@pytest.mark.skipif(pygeos.geos_version < (3, 10, 0), reason="GEOS < 3.10")
@pytest.mark.skipif(pygeos.geos_version < (3, 10, 1), reason="GEOS < 3.10.1")
def test_from_geojson_exceptions():
with pytest.raises(TypeError, match="Expected bytes or string, got int"):
pygeos.from_geojson(1)
Expand All @@ -775,28 +775,28 @@ def test_from_geojson_exceptions():
with pytest.raises(pygeos.GEOSException, match="type must be array, but is null"):
pygeos.from_geojson('{"type": "LineString", "coordinates": null}')

# Note: The two below tests may make GEOS 3.10.0 crash
# https://trac.osgeo.org/geos/ticket/1138
with pytest.raises((pygeos.GEOSException, OSError)):
# Note: The two below tests are the reason that from_geojson is disabled for
# GEOS 3.10.0 See https://trac.osgeo.org/geos/ticket/1138
with pytest.raises(pygeos.GEOSException, match="key 'type' not found"):
pygeos.from_geojson('{"geometry": null, "properties": []}')

with pytest.raises((pygeos.GEOSException, OSError)):
with pytest.raises(pygeos.GEOSException, match="key 'type' not found"):
pygeos.from_geojson('{"no": "geojson"}')


@pytest.mark.skipif(pygeos.geos_version < (3, 10, 0), reason="GEOS < 3.10")
@pytest.mark.skipif(pygeos.geos_version < (3, 10, 1), reason="GEOS < 3.10.1")
def test_from_geojson_warn_on_invalid():
with pytest.warns(Warning, match="Invalid GeoJSON"):
assert pygeos.from_geojson("", on_invalid="warn") is None


@pytest.mark.skipif(pygeos.geos_version < (3, 10, 0), reason="GEOS < 3.10")
@pytest.mark.skipif(pygeos.geos_version < (3, 10, 1), reason="GEOS < 3.10.1")
def test_from_geojson_ignore_on_invalid():
with pytest.warns(None):
assert pygeos.from_geojson("", on_invalid="ignore") is None


@pytest.mark.skipif(pygeos.geos_version < (3, 10, 0), reason="GEOS < 3.10")
@pytest.mark.skipif(pygeos.geos_version < (3, 10, 1), reason="GEOS < 3.10.1")
def test_from_geojson_on_invalid_unsupported_option():
with pytest.raises(ValueError, match="not a valid option"):
pygeos.from_geojson(GEOJSON_GEOMETRY, on_invalid="unsupported_option")
Expand Down Expand Up @@ -850,7 +850,7 @@ def test_to_geojson_point_empty(geom):
assert pygeos.to_geojson(geom)


@pytest.mark.skipif(pygeos.geos_version < (3, 10, 0), reason="GEOS < 3.10")
@pytest.mark.skipif(pygeos.geos_version < (3, 10, 1), reason="GEOS < 3.10.1")
@pytest.mark.parametrize("geom", all_types)
def test_geojson_all_types(geom):
if pygeos.get_type_id(geom) == pygeos.GeometryType.LINEARRING:
Expand Down
44 changes: 0 additions & 44 deletions pygeos/tests/test_misc.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import ctypes
import os
import sys
import warnings
from itertools import chain
from string import ascii_letters, digits
from unittest import mock
Expand All @@ -11,7 +9,6 @@

import pygeos
from pygeos.decorators import multithreading_enabled, requires_geos
from pygeos.may_segfault import may_segfault


@pytest.fixture
Expand Down Expand Up @@ -187,44 +184,3 @@ def test_multithreading_enabled_preserves_flag():
def test_multithreading_enabled_ok(args, kwargs):
result = set_first_element(42, *args, **kwargs)
assert result[0] == 42


def my_unstable_func(event=None):
if event == "segfault":
ctypes.string_at(0) # segfault
elif event == "exit":
exit(1)
elif event == "raise":
raise ValueError("This is a test")
elif event == "warn":
warnings.warn("This is a test", RuntimeWarning)
elif event == "return":
return "This is a test"


def test_may_segfault():
if os.name == "nt":
match = "access violation"
else:
match = "GEOS crashed"
with pytest.raises(OSError, match=match):
may_segfault(my_unstable_func)("segfault")


def test_may_segfault_exit():
with pytest.raises(OSError, match="GEOS crashed with exit code 1."):
may_segfault(my_unstable_func)("exit")


def test_may_segfault_raises():
with pytest.raises(ValueError, match="This is a test"):
may_segfault(my_unstable_func)("raise")


def test_may_segfault_returns():
assert may_segfault(my_unstable_func)("return") == "This is a test"


def test_may_segfault_warns():
with pytest.warns(RuntimeWarning, match="This is a test"):
may_segfault(my_unstable_func)("warn")

0 comments on commit 327c919

Please sign in to comment.