Detect duplicate keys in decoded JSON objects

Closes #433.

Author: whatyouhide

conformance/exemptions.txt

@@ -20,17 +20,6 @@
 # JSON extension field names are not supported yet.
 Recommended.Proto2.JsonInput.FieldNameExtension.Validator
 
-# Rejecting JSON objects with duplicate keys (including the same field spelled in two different
-# casings, e.g. "optionalInt32" and "optional_int32") requires a JSON parser that surfaces
-# duplicate keys, which neither Jason nor the built-in JSON module does. The pinned protoc accepts
-# duplicates; Protobuf's `main` requires them to be rejected. These are "Recommended".
-Recommended.Proto2.JsonInput.FieldNameDuplicate # only-on: latest
-Recommended.Proto2.JsonInput.FieldNameDuplicateDifferentCasing1 # only-on: latest
-Recommended.Proto2.JsonInput.FieldNameDuplicateDifferentCasing2 # only-on: latest
-Recommended.Proto3.JsonInput.FieldNameDuplicate # only-on: latest
-Recommended.Proto3.JsonInput.FieldNameDuplicateDifferentCasing1 # only-on: latest
-Recommended.Proto3.JsonInput.FieldNameDuplicateDifferentCasing2 # only-on: latest
-
 # The proto2-only MessageSet wire format is not implemented. These tests do not exist in the
 # pinned Protobuf dependency yet.
 Recommended.Proto2.ProtobufInput.ValidMessageSetEncoding.SubmessageEncoding.NotUnknown.ProtobufOutput # only-on: latest

lib/protobuf/json/json_library.ex

@@ -1,32 +1,67 @@
 defmodule Protobuf.JSON.JSONLibrary do
   @moduledoc false
-  # Uses `JSON` for Elixir >= 1.18, Jason if Elixir < 1.18 and Jason available,
-  # or returns error otherwise
 
-  cond do
-    Code.ensure_loaded?(JSON) ->
-      def encode_to_iodata(encodable) do
+  alias Protobuf.JSON.{DecodeError, EncodeError, Object}
+
+  # Uses `JSON` for encoding on Elixir >= 1.18 because it supports the Elixir encoder protocol,
+  # Jason otherwise.
+
+  def encode_to_iodata(encodable) do
+    cond do
+      Code.ensure_loaded?(JSON) ->
         try do
-          {:ok, JSON.encode_to_iodata!(encodable)}
+          {:ok, apply(JSON, :encode_to_iodata!, [encodable])}
         rescue
           exception ->
             {:error, exception}
         end
-      end
 
-      def decode(data) do
-        case JSON.decode(data) do
-          {:ok, decoded} -> {:ok, decoded}
-          {:error, error} -> {:error, Protobuf.JSON.DecodeError.new(error)}
-        end
-      end
+      Code.ensure_loaded?(Jason) ->
+        apply(Jason, :encode_to_iodata, [encodable])
+
+      true ->
+        {:error, EncodeError.new(:no_json_lib)}
+    end
+  end
+
+  def decode(data) do
+    cond do
+      Code.ensure_loaded?(JSON) and function_exported?(JSON, :decode, 3) ->
+        decode_with_elixir_json(data)
+
+      Code.ensure_loaded?(Jason) ->
+        apply(Jason, :decode, [data])
+
+      true ->
+        {:error, DecodeError.new(:no_json_lib)}
+    end
+  end
+
+  defp decode_with_elixir_json(data) do
+    case apply(JSON, :decode, [data, nil, json_decoders()]) do
+      {decoded, nil, rest} -> finalize_decode(decoded, rest)
+      {:error, reason} -> {:error, DecodeError.new(reason)}
+    end
+  end
 
-    Code.ensure_loaded?(Jason) ->
-      def encode_to_iodata(encodable), do: Jason.encode_to_iodata(encodable)
-      def decode(data), do: Jason.decode(data)
+  defp finalize_decode(decoded, rest) do
+    if json_whitespace?(rest) do
+      {:ok, decoded}
+    else
+      {:error, DecodeError.new({:trailing_data, rest})}
+    end
+  end
 
-    true ->
-      def encode_to_iodata(_), do: {:error, EncodeError.new(:no_json_lib)}
-      def decode(_), do: {:error, EncodeError.new(:no_json_lib)}
+  defp json_decoders do
+    [object_finish: &object_finish/2]
   end
+
+  defp object_finish(members, old_acc), do: {Object.new(Enum.reverse(members)), old_acc}
+
+  defp json_whitespace?(<<>>), do: true
+
+  defp json_whitespace?(<<byte, rest::binary>>) when byte in [?\s, ?\t, ?\n, ?\r],
+    do: json_whitespace?(rest)
+
+  defp json_whitespace?(_rest), do: false
 end

lib/protobuf/json/object.ex

@@ -0,0 +1,26 @@
+defmodule Protobuf.JSON.Object do
+  @moduledoc false
+
+  # This is an internal structure that we use to avoid building maps right away
+  # when decoding JSON; with maps, we cannot detect duplicate keys (by definition).
+
+  defstruct [:members]
+
+  @type t() :: %__MODULE__{members: [{binary(), term()}]}
+
+  @spec new([{binary(), term()}]) :: t()
+  def new(members) when is_list(members) do
+    %__MODULE__{members: members}
+  end
+
+  @spec to_map(t()) :: %{optional(binary()) => term()}
+  def to_map!(%__MODULE__{members: members}) do
+    Enum.reduce(members, %{}, fn {key, value}, acc ->
+      if Map.has_key?(acc, key) do
+        throw({:duplicated_json_key, key})
+      else
+        Map.put(acc, key, value)
+      end
+    end)
+  end
+end