Merge pull request #276 from thalesmg/limit-map-key-length

fix: validate map field name length before attempting to convert to atoms
emqx · Oct 31, 2023 · 83d4a64 · 83d4a64
2 parents 89bbda4 + 317ffe5
commit 83d4a64
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 6 deletions.
diff --git a/src/hocon_maps.erl b/src/hocon_maps.erl
@@ -50,6 +50,10 @@
 
 -type flatten_opts() :: #{rich_value => boolean()}.
 
+-elvis([
+    {elvis_style, atom_naming_convention, #{regex => "^([a-z][a-z0-9]*_?)([a-z0-9]*_?)*(_SUITE)?$"}}
+]).
+
 %% @doc put unboxed value to the richmap box
 %% this function is called places where there is no boxing context
 %% so it has to accept unboxed value.
@@ -89,13 +93,17 @@ update_map_field(Opts, Map, FieldName, GoDeep) ->
     Map1 = maps:without([FieldName], Map),
     Map1#{maybe_atom(Opts, FieldName) => FieldV}.
 
+maybe_atom(#{atom_key := true}, Name) when is_binary(Name), size(Name) > 255 ->
+    error({name_longer_than_255_bytes, Name});
 maybe_atom(#{atom_key := true}, Name) when is_binary(Name) ->
     try
         binary_to_existing_atom(Name, utf8)
     catch
         _:_ ->
             error({non_existing_atom, Name})
     end;
+maybe_atom(#{atom_key := {true, unsafe}}, Name) when is_binary(Name), size(Name) > 255 ->
+    error({name_longer_than_255_bytes, Name});
 maybe_atom(#{atom_key := {true, unsafe}}, Name) when is_binary(Name) ->
     binary_to_atom(Name, utf8);
 maybe_atom(_Opts, Name) ->

diff --git a/src/hocon_tconf.erl b/src/hocon_tconf.erl
@@ -558,7 +558,7 @@ map_field(?MAP(_Name, Type), FieldSchema, Value, Opts) ->
         [] ->
             {[], Value};
         FieldNames ->
-            case get_invalid_name(FieldNames) of
+            case get_invalid_name(FieldNames, Opts) of
                 [] ->
                     %% All objects in this map should share the same schema.
                     NewSc = hocon_schema:override(
@@ -573,6 +573,7 @@ map_field(?MAP(_Name, Type), FieldSchema, Value, Opts) ->
                         #{
                             reason => invalid_map_key,
                             expected_data_type => ?MAP_KEY_RE,
+                            hint => "map keys must have less than 255 bytes",
                             got => InvalidNames
                         },
                     {validation_errs(Opts, Context), Value}
@@ -1302,15 +1303,23 @@ check_index_seq(I, [{Index, V} | Rest], Acc) ->
             }}
     end.
 
-get_invalid_name(Names) ->
+get_invalid_name(Names, Opts) ->
+    AtomKey =
+        case maps:get(atom_key, Opts, false) of
+            true -> true;
+            {true, unsafe} -> true;
+            _ -> false
+        end,
     lists:filter(
         fun(F) ->
-            nomatch =:=
+            DoesNotMatchRE =
                 try
-                    re:run(F, ?MAP_KEY_RE)
+                    nomatch =:= re:run(F, ?MAP_KEY_RE)
                 catch
-                    _:_ -> nomatch
-                end
+                    _:_ -> false
+                end,
+            TooLong = AtomKey andalso string:length(F) > 255,
+            DoesNotMatchRE orelse TooLong
         end,
         Names
     ).

diff --git a/test/hocon_tconf_tests.erl b/test/hocon_tconf_tests.erl
@@ -2436,6 +2436,60 @@ map_atom_keys_test_() ->
                     },
                     V
                 )
+            end)},
+        {"key length > 255 bytes (atom_key = true)",
+            ?_test(begin
+                BadKeyStr = lists:duplicate(256, $a),
+                BadKey = list_to_binary(BadKeyStr),
+                BadMap = #{<<"root">> => #{BadKey => #{<<"foo">> => #{}}}},
+                ?assertThrow(
+                    {_, [
+                        #{
+                            kind := validation_error,
+                            got := [BadKeyStr],
+                            reason := invalid_map_key
+                        }
+                    ]},
+                    hocon_tconf:check_plain(
+                        Sc,
+                        BadMap,
+                        #{atom_key => true}
+                    )
+                )
+            end)},
+        {"key length > 255 bytes (atom_key = {true, unsafe})",
+            ?_test(begin
+                BadKeyStr = lists:duplicate(256, $a),
+                BadKey = list_to_binary(BadKeyStr),
+                BadMap = #{<<"root">> => #{BadKey => #{<<"foo">> => #{}}}},
+                ?assertThrow(
+                    {_, [
+                        #{
+                            kind := validation_error,
+                            got := [BadKeyStr],
+                            reason := invalid_map_key
+                        }
+                    ]},
+                    hocon_tconf:check_plain(
+                        Sc,
+                        BadMap,
+                        #{atom_key => {true, unsafe}}
+                    )
+                )
+            end)},
+        {"key length > 255 bytes (atom_key = false)",
+            ?_test(begin
+                BadKeyStr = lists:duplicate(256, $a),
+                BadKey = list_to_binary(BadKeyStr),
+                BadMap = #{<<"root">> => #{BadKey => #{<<"foo">> => #{}}}},
+                ?assertMatch(
+                    #{<<"root">> := #{BadKey := _}},
+                    hocon_tconf:check_plain(
+                        Sc,
+                        BadMap,
+                        #{atom_key => false}
+                    )
+                )
             end)}
     ].