Skip to content

Commit

Permalink
Convert string coordinates to floats, or raise an error
Browse files Browse the repository at this point in the history
This fixes an issue where we were silently accepting non-numeric coordinates in the GeoJSON decoder, such that you could wind up doing things like decoding a point like `%Geo.Point{coordinates: {"100.0", "-10.0"}}`. This would obviously not have gone well for you later in your processing pipeline.

The fix here, suggested by @LostKobrakai, is to convert those strings to numbers where we can do so unambiguously. While such inputs are clearly invalid, it's easy enough to handle them in the way that the user was hoping that we should probably just do it. In cases where there's any ambiguity at all, we raise an ArgumentError.

Resolves #175
  • Loading branch information
s3cur3 committed Aug 30, 2024
1 parent 4a7e08f commit e0aed25
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 3 deletions.
50 changes: 47 additions & 3 deletions lib/geo/json/decoder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ defmodule Geo.JSON.Decoder do
Enum.map(Map.get(geo_json, "geometries"), fn x ->
do_decode(
Map.get(x, "type"),
Map.get(x, "coordinates"),
ensure_numeric(x["coordinates"]),
Map.get(x, "properties", %{}),
crs
)
Expand All @@ -62,7 +62,7 @@ defmodule Geo.JSON.Decoder do

do_decode(
Map.get(geo_json, "type"),
Map.get(geo_json, "coordinates"),
ensure_numeric(geo_json["coordinates"]),
Map.get(geo_json, "properties", %{}),
crs
)
Expand Down Expand Up @@ -205,7 +205,12 @@ defmodule Geo.JSON.Decoder do
properties: properties
}
else
do_decode(Map.get(geometry, "type"), Map.get(geometry, "coordinates"), properties, nil)
do_decode(
Map.get(geometry, "type"),
ensure_numeric(geometry["coordinates"]),
properties,
nil
)
end
end

Expand All @@ -231,4 +236,43 @@ defmodule Geo.JSON.Decoder do
defp get_srid(nil) do
nil
end

# Fast paths for the common (correct) cases
defp ensure_numeric(num) when is_number(num), do: num
defp ensure_numeric([x, y] = l) when is_number(x) and is_number(y), do: l

defp ensure_numeric([x, y, z] = l)
when is_number(x) and is_number(y) and (is_number(z) or is_nil(z)) do
l
end

defp ensure_numeric([x, y, z, m] = l)
when is_number(x) and is_number(y) and (is_number(z) or is_nil(z)) and
(is_number(m) or is_nil(z)) do
l
end

defp ensure_numeric(l) when is_list(l) do
Enum.map(l, fn
num when is_number(num) ->
num

str when is_binary(str) ->
try do
String.to_float(str)
catch
ArgumentError ->
raise ArgumentError, "expected a numeric coordinate, got the string #{inspect(str)}"
end

nil ->
nil

l when is_list(l) ->
Enum.map(l, &ensure_numeric/1)

other ->
raise ArgumentError, "expected a numeric coordinate, got: #{inspect(other)}"
end)
end
end
44 changes: 44 additions & 0 deletions test/geo/json_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,50 @@ defmodule Geo.JSON.Test do
assert geom.geometries == []
end

test "Decode seamlessly converts coordinates that are numbers-as-strings" do
check all(
x <- float(),
y <- float()
) do
json = """
{
"properties": {},
"geometry": {
"type": "Point",
"coordinates": ["#{x}", "#{y}"]
},
"type": "Feature"
}
"""

assert %Geo.Point{coordinates: {^x, ^y}} = Jason.decode!(json) |> Geo.JSON.decode!()
end
end

test "Decode rejects geometries with non-numeric coordinates" do
for {bad_x, bad_y} <- [
{" 100.0", "0.0"},
{"100.0", "0.0?"},
{"100.", "0.0"},
{"100.0", nil, "0.0"}
] do
json = """
{
"properties": {},
"geometry": {
"type": "Point",
"coordinates": [#{inspect(bad_x)}, #{inspect(bad_y)}]
},
"type": "Feature"
}
"""

assert_raise ArgumentError, fn ->
Jason.decode!(json) |> Geo.JSON.decode!()
end
end
end

property "encodes and decodes back to the correct Point struct" do
check all(
x <- float(),
Expand Down

0 comments on commit e0aed25

Please sign in to comment.