From e0aed25ec9c234685a3c9d15c64899222382af12 Mon Sep 17 00:00:00 2001
From: Tyler Young <tyler@tylerayoung.com>
Date: Fri, 30 Aug 2024 19:02:56 -0400
Subject: [PATCH] Convert string coordinates to floats, or raise an error

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
---
 lib/geo/json/decoder.ex | 50 ++++++++++++++++++++++++++++++++++++++---
 test/geo/json_test.exs  | 44 ++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 3 deletions(-)

diff --git a/lib/geo/json/decoder.ex b/lib/geo/json/decoder.ex
index b42999d..d615eb5 100644
--- a/lib/geo/json/decoder.ex
+++ b/lib/geo/json/decoder.ex
@@ -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
             )
@@ -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
         )
@@ -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
 
@@ -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
diff --git a/test/geo/json_test.exs b/test/geo/json_test.exs
index 0dbbf5b..1b7f059 100644
--- a/test/geo/json_test.exs
+++ b/test/geo/json_test.exs
@@ -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(),