From 255a0f4fc05f336b1cc15903489e77a4e16a1437 Mon Sep 17 00:00:00 2001 From: Danil Zagoskin Date: Sat, 15 Feb 2025 00:32:54 +0300 Subject: [PATCH 1/2] better bad request reporting --- src/erf_router.erl | 39 ++++++++++++++++++++------------------- src/erf_util.erl | 7 ++++++- test/erf_SUITE.erl | 2 +- test/erf_router_SUITE.erl | 2 +- 4 files changed, 28 insertions(+), 22 deletions(-) diff --git a/src/erf_router.erl b/src/erf_router.erl index cee7ee7..a77b295 100644 --- a/src/erf_router.erl +++ b/src/erf_router.erl @@ -252,7 +252,7 @@ handle_ast(API, #{callback := Callback} = Opts) -> erl_syntax:clause( [ erl_syntax:match_expr( - erl_syntax:variable('Request'), + erl_syntax:variable('Request0'), erl_syntax:map_expr( none, [ @@ -286,6 +286,18 @@ handle_ast(API, #{callback := Callback} = Opts) -> erl_syntax:variable('IsValidRequest'), IsValidRequestAST ), + erl_syntax:match_expr( + erl_syntax:variable('Request'), + erl_syntax:map_expr( + erl_syntax:variable('Request0'), + [ + erl_syntax:map_field_assoc( + erl_syntax:atom('path_parameters'), + erl_syntax:variable('PathParameters') + ) + ] + ) + ), erl_syntax:case_expr( erl_syntax:variable('IsValidRequest'), [ @@ -303,19 +315,7 @@ handle_ast(API, #{callback := Callback} = Opts) -> utf8 ) ), - [ - erl_syntax:map_expr( - erl_syntax:variable('Request'), - [ - erl_syntax:map_field_assoc( - erl_syntax:atom('path_parameters'), - erl_syntax:variable( - 'PathParameters' - ) - ) - ] - ) - ] + [erl_syntax:variable('Request')] ) ] ), @@ -323,16 +323,17 @@ handle_ast(API, #{callback := Callback} = Opts) -> [ erl_syntax:tuple([ erl_syntax:atom(false), - erl_syntax:variable('_Reason') + erl_syntax:variable('Reason') ]) ], none, [ - erl_syntax:tuple( + erl_syntax:application( + erl_syntax:atom(erf_util), + erl_syntax:atom(handle_invalid_request), [ - erl_syntax:integer(400), - erl_syntax:list([]), - erl_syntax:atom(undefined) + erl_syntax:variable('Request'), + erl_syntax:variable('Reason') ] ) ] diff --git a/src/erf_util.erl b/src/erf_util.erl index 76c381b..16a1bf0 100644 --- a/src/erf_util.erl +++ b/src/erf_util.erl @@ -16,7 +16,8 @@ %%% EXTERNAL EXPORTS -export([ to_pascal_case/1, - to_snake_case/1 + to_snake_case/1, + handle_invalid_request/2 ]). %%%----------------------------------------------------------------------------- @@ -86,3 +87,7 @@ to_snake_case([_C | Rest], [$_ | _T] = Acc) -> to_snake_case(Rest, Acc); to_snake_case([_C | Rest], Acc) -> to_snake_case(Rest, [$_ | Acc]). + + +handle_invalid_request(_Request, Reason) -> + {400, [{<<"content-type">>, <<"text/plain">>}], [io_lib:print(Reason), "\n"]}. diff --git a/test/erf_SUITE.erl b/test/erf_SUITE.erl index d0ef7ac..7f703da 100644 --- a/test/erf_SUITE.erl +++ b/test/erf_SUITE.erl @@ -101,7 +101,7 @@ foo(_Conf) -> ), ?assertMatch( - {ok, {{"HTTP/1.1", 400, "Bad Request"}, _Result2Headers, <<>>}}, + {ok, {{"HTTP/1.1", 400, "Bad Request"}, _Result2Headers, <<_/binary>>}}, httpc:request( post, {"http://localhost:8789/1/foo", [], "application/json", <<"\"foobar\"">>}, diff --git a/test/erf_router_SUITE.erl b/test/erf_router_SUITE.erl index 840f1ac..367b501 100644 --- a/test/erf_router_SUITE.erl +++ b/test/erf_router_SUITE.erl @@ -166,7 +166,7 @@ foo(_Conf) -> meck:expect(get_foo_request_body, is_valid, fun(_Value) -> {false, reason} end), - ?assertEqual({400, [], undefined}, Mod:handle(Req)), + ?assertMatch({400, [{<<"content-type">>, _}], _}, Mod:handle(Req)), NotAllowedReq = #{ path => [<<"1">>, <<"foo">>], From 43e7321d2c205b1564b8054f9338f963dac59a85 Mon Sep 17 00:00:00 2001 From: Danil Zagoskin Date: Sat, 15 Feb 2025 00:45:49 +0300 Subject: [PATCH 2/2] prevent bad responses from being sent to client --- src/erf_router.erl | 35 +++++++++++++++++++++++++++++++++-- src/erf_util.erl | 24 +++++++++++++++++++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/erf_router.erl b/src/erf_router.erl index a77b295..4f0372c 100644 --- a/src/erf_router.erl +++ b/src/erf_router.erl @@ -249,6 +249,9 @@ handle_ast(API, #{callback := Callback} = Opts) -> Request ), + Responses = maps:get(responses, Operation, undefined), + ValidateResponseAST = validate_response(Responses), + erl_syntax:clause( [ erl_syntax:match_expr( @@ -305,7 +308,7 @@ handle_ast(API, #{callback := Callback} = Opts) -> [erl_syntax:atom(true)], none, [ - erl_syntax:application( + erl_syntax:match_expr(erl_syntax:variable('Response'), erl_syntax:application( erl_syntax:atom(Callback), erl_syntax:atom( erlang:binary_to_atom( @@ -316,7 +319,8 @@ handle_ast(API, #{callback := Callback} = Opts) -> ) ), [erl_syntax:variable('Request')] - ) + )), + ValidateResponseAST ] ), erl_syntax:clause( @@ -692,6 +696,33 @@ is_valid_request(RawParameters, Request) -> ] ). +-spec validate_response(Responses) -> Result when + Responses :: undefined | #{ '*' | erf_parser:status_code() := erf_parser:response() }, + Result :: erl_syntax:syntaxTree(). +validate_response(undefined) -> + erl_syntax:atom('ok'); +validate_response(#{} = Responses) -> + Validators = erl_syntax:map_expr( + none, + [response_validation_map_assoc(R) || R <- maps:to_list(Responses)] + ), + erl_syntax:application( + erl_syntax:atom(erf_util), + erl_syntax:atom(validate_response), + [erl_syntax:variable('Request'), erl_syntax:variable('Response'), Validators] + ). + +response_validation_map_assoc({Status, #{body := #{ref := ResponseBodyRef}}}) -> + StatusAST = case Status of + '*' -> erl_syntax:atom('*'); + _ when is_integer(Status) -> erl_syntax:integer(Status) + end, + ResponseBodyModule = + erlang:binary_to_atom(erf_util:to_snake_case(ResponseBodyRef)), + erl_syntax:map_field_assoc(StatusAST, erl_syntax:atom(ResponseBodyModule)). + + + -spec load_binary(ModuleName, Bin) -> Result when ModuleName :: atom(), Bin :: binary(), diff --git a/src/erf_util.erl b/src/erf_util.erl index 16a1bf0..e8ff88e 100644 --- a/src/erf_util.erl +++ b/src/erf_util.erl @@ -13,11 +13,14 @@ %% limitations under the License -module(erf_util). +-include_lib("kernel/include/logger.hrl"). + %%% EXTERNAL EXPORTS -export([ to_pascal_case/1, to_snake_case/1, - handle_invalid_request/2 + handle_invalid_request/2, + validate_response/3 ]). %%%----------------------------------------------------------------------------- @@ -91,3 +94,22 @@ to_snake_case([_C | Rest], Acc) -> handle_invalid_request(_Request, Reason) -> {400, [{<<"content-type">>, <<"text/plain">>}], [io_lib:print(Reason), "\n"]}. + +validate_response(Request, {Status, _, #{} = RespBody} = Response, Validators) -> + IsValid = case Validators of + #{Status := ValidatorMod} -> + ValidatorMod:is_valid(RespBody); + #{'*' := ValidatorMod} -> + ValidatorMod:is_valid(RespBody); + #{} -> + unknown_status + end, + case IsValid of + true -> + Response; + Other -> + ?LOG(error, "[erf] Bad response (~0p)~n ~120p~n for request ~120p", [Other, Response, Request]), + {500, [{<<"content-type">>, <<"text/plain">>}], <<"Internal server error">>} + end; +validate_response(_Request, {_, _, _} = Response, _) -> + Response.