diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index 080670de6..685543c84 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -481,8 +481,7 @@ schema("/bridges_probe") -> ?BAD_REQUEST('ALREADY_EXISTS', <<"bridge already exists">>); {error, not_found} -> Conf = filter_out_request_body(Conf0), - {ok, _} = emqx_bridge:create(BridgeType, BridgeName, Conf), - lookup_from_all_nodes(BridgeType, BridgeName, 201) + create_bridge(BridgeType, BridgeName, Conf) end; '/bridges'(get, _Params) -> Nodes = mria:running_nodes(), @@ -508,8 +507,7 @@ schema("/bridges_probe") -> {ok, _} -> RawConf = emqx:get_raw_config([bridges, BridgeType, BridgeName], #{}), Conf = deobfuscate(Conf1, RawConf), - {ok, _} = emqx_bridge:create(BridgeType, BridgeName, Conf), - lookup_from_all_nodes(BridgeType, BridgeName, 200); + update_bridge(BridgeType, BridgeName, Conf); {error, not_found} -> ?BRIDGE_NOT_FOUND(BridgeType, BridgeName) end @@ -609,6 +607,20 @@ lookup_from_local_node(BridgeType, BridgeName) -> Error -> Error end. +create_bridge(BridgeType, BridgeName, Conf) -> + create_or_update_bridge(BridgeType, BridgeName, Conf, 201). + +update_bridge(BridgeType, BridgeName, Conf) -> + create_or_update_bridge(BridgeType, BridgeName, Conf, 200). + +create_or_update_bridge(BridgeType, BridgeName, Conf, HttpStatusCode) -> + case emqx_bridge:create(BridgeType, BridgeName, Conf) of + {ok, _} -> + lookup_from_all_nodes(BridgeType, BridgeName, HttpStatusCode); + {error, #{kind := validation_error} = Reason} -> + ?BAD_REQUEST(map_to_json(Reason)) + end. + '/bridges/:id/enable/:enable'(put, #{bindings := #{id := Id, enable := Enable}}) -> ?TRY_PARSE_ID( Id, @@ -929,7 +941,7 @@ filter_out_request_body(Conf) -> <<"type">>, <<"name">>, <<"status">>, - <<"error">>, + <<"status_reason">>, <<"node_status">>, <<"node_metrics">>, <<"metrics">>, @@ -1033,3 +1045,8 @@ deobfuscate(NewConf, OldConf) -> #{}, NewConf ). + +map_to_json(M) -> + emqx_json:encode( + emqx_map_lib:jsonable_map(M, fun(K, V) -> {K, emqx_map_lib:binary_string(V)} end) + ). diff --git a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl index 932d7261a..47a23e71c 100644 --- a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl +++ b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl @@ -322,6 +322,33 @@ t_http_crud_apis(Config) -> end ), + %% Test bad updates + {ok, 400, PutFail1} = request( + put, + uri(["bridges", BridgeID]), + maps:remove(<<"url">>, ?HTTP_BRIDGE(URL2, Name)) + ), + ?assertMatch( + #{<<"reason">> := <<"required_field">>}, + emqx_json:decode(maps:get(<<"message">>, emqx_json:decode(PutFail1, [return_maps])), [ + return_maps + ]) + ), + {ok, 400, PutFail2} = request( + put, + uri(["bridges", BridgeID]), + maps:put(<<"curl">>, URL2, maps:remove(<<"url">>, ?HTTP_BRIDGE(URL2, Name))) + ), + ?assertMatch( + #{ + <<"reason">> := <<"unknown_fields">>, + <<"unknown">> := <<"curl">> + }, + emqx_json:decode(maps:get(<<"message">>, emqx_json:decode(PutFail2, [return_maps])), [ + return_maps + ]) + ), + %% delete the bridge {ok, 204, <<>>} = request(delete, uri(["bridges", BridgeID]), []), {ok, 200, <<"[]">>} = request(get, uri(["bridges"]), []), @@ -387,6 +414,9 @@ t_http_crud_apis(Config) -> ?assert(not maps:is_key(<<"status_reason">>, FixedBridge)), ?assert(not maps:is_key(<<"status_reason">>, FixedNodeStatus)), {ok, 204, <<>>} = request(delete, uri(["bridges", BridgeID]), []), + + %% Try create bridge with bad characters as name + {ok, 400, _} = request(post, uri(["bridges"]), ?HTTP_BRIDGE(URL1, <<"隋达"/utf8>>)), ok. t_http_bridges_local_topic(Config) -> diff --git a/changes/ce/fix-10226.en.md b/changes/ce/fix-10226.en.md new file mode 100644 index 000000000..2d833d2dc --- /dev/null +++ b/changes/ce/fix-10226.en.md @@ -0,0 +1 @@ +Don't crash on validation error in `/bridges` API, return `400` instead.