From e0b036fe68579b3ffb69b450acf2d17bb7162cf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Fri, 2 Nov 2018 13:54:19 +0100 Subject: Add tests for charsets_provided Fix cases where the q-value is 0 and where a wildcard was sent in the accept-charset header. Also don't send a charset in the content-type of the response if the media type is not text. Thanks to Philip Witty for help figuring this out. --- doc/src/manual/cowboy_rest.asciidoc | 3 + src/cowboy_rest.erl | 13 ++- test/handlers/charsets_provided_empty_h.erl | 30 +++++++ test/handlers/charsets_provided_h.erl | 28 ++++++ test/rest_handler_SUITE.erl | 132 ++++++++++++++++++++++++++++ 5 files changed, 203 insertions(+), 3 deletions(-) create mode 100644 test/handlers/charsets_provided_empty_h.erl create mode 100644 test/handlers/charsets_provided_h.erl diff --git a/doc/src/manual/cowboy_rest.asciidoc b/doc/src/manual/cowboy_rest.asciidoc index 19f4820..dd5fa7e 100644 --- a/doc/src/manual/cowboy_rest.asciidoc +++ b/doc/src/manual/cowboy_rest.asciidoc @@ -175,6 +175,9 @@ req() :: #{ } ---- +Note that Cowboy will only append the charset to the +content-type header of the response if the media type is text. + === content_types_accepted [source,erlang] diff --git a/src/cowboy_rest.erl b/src/cowboy_rest.erl index 63f3d99..24c0fb0 100644 --- a/src/cowboy_rest.erl +++ b/src/cowboy_rest.erl @@ -680,11 +680,16 @@ prioritize_charsets(AcceptCharsets) -> choose_charset(Req, State, []) -> not_acceptable(Req, State); +%% A q-value of 0 means not acceptable. +choose_charset(Req, State, [{_, 0}|Tail]) -> + choose_charset(Req, State, Tail); choose_charset(Req, State=#state{charsets_p=CP}, [Charset|Tail]) -> match_charset(Req, State, Tail, CP, Charset). match_charset(Req, State, Accept, [], _Charset) -> choose_charset(Req, State, Accept); +match_charset(Req, State, _Accept, [Provided|_], {<<"*">>, _}) -> + set_content_type(Req, State#state{charset_a=Provided}); match_charset(Req, State, _Accept, [Provided|_], {Provided, _}) -> set_content_type(Req, State#state{charset_a=Provided}); match_charset(Req, State, Accept, [_|Tail], Charset) -> @@ -695,9 +700,11 @@ set_content_type(Req, State=#state{ charset_a=Charset}) -> ParamsBin = set_content_type_build_params(Params, []), ContentType = [Type, <<"/">>, SubType, ParamsBin], - ContentType2 = case Charset of - undefined -> ContentType; - Charset -> [ContentType, <<"; charset=">>, Charset] + ContentType2 = case {Type, Charset} of + {<<"text">>, Charset} when Charset =/= undefined -> + [ContentType, <<"; charset=">>, Charset]; + _ -> + ContentType end, Req2 = cowboy_req:set_resp_header(<<"content-type">>, ContentType2, Req), encodings_provided(Req2#{charset => Charset}, State). diff --git a/test/handlers/charsets_provided_empty_h.erl b/test/handlers/charsets_provided_empty_h.erl new file mode 100644 index 0000000..0548a5e --- /dev/null +++ b/test/handlers/charsets_provided_empty_h.erl @@ -0,0 +1,30 @@ +%% This module has a text and non-text media type, +%% but provides no charset. All requests will result +%% in a 406 not acceptable. + +-module(charsets_provided_empty_h). + +-export([init/2]). +-export([content_types_provided/2]). +-export([charsets_provided/2]). +-export([get_text_plain/2]). +-export([get_application_json/2]). + +init(Req, Opts) -> + {cowboy_rest, Req, Opts}. + +content_types_provided(Req, State) -> + {[ + {{<<"text">>, <<"plain">>, []}, get_text_plain}, + {{<<"application">>, <<"json">>, []}, get_application_json} + ], Req, State}. + +charsets_provided(Req, State) -> + {[], Req, State}. + +get_text_plain(Req, State) -> + {<<"This is REST!">>, Req, State}. + +get_application_json(Req, State) -> + {<<"{\"hello\": \"rest\"}">>, Req, State}. + diff --git a/test/handlers/charsets_provided_h.erl b/test/handlers/charsets_provided_h.erl new file mode 100644 index 0000000..4a08e78 --- /dev/null +++ b/test/handlers/charsets_provided_h.erl @@ -0,0 +1,28 @@ +%% This module has a text and non-text media type, +%% and provides two charsets. + +-module(charsets_provided_h). + +-export([init/2]). +-export([content_types_provided/2]). +-export([charsets_provided/2]). +-export([get_text_plain/2]). +-export([get_application_json/2]). + +init(Req, Opts) -> + {cowboy_rest, Req, Opts}. + +content_types_provided(Req, State) -> + {[ + {{<<"text">>, <<"plain">>, []}, get_text_plain}, + {{<<"application">>, <<"json">>, []}, get_application_json} + ], Req, State}. + +charsets_provided(Req, State) -> + {[<<"utf-8">>, <<"utf-16">>], Req, State}. + +get_text_plain(Req, State) -> + {<<"This is REST!">>, Req, State}. + +get_application_json(Req, State) -> + {<<"{\"hello\": \"rest\"}">>, Req, State}. diff --git a/test/rest_handler_SUITE.erl b/test/rest_handler_SUITE.erl index 491c7bb..93532fe 100644 --- a/test/rest_handler_SUITE.erl +++ b/test/rest_handler_SUITE.erl @@ -39,6 +39,8 @@ end_per_group(Name, _) -> init_dispatch(_) -> cowboy_router:compile([{'_', [ {"/", rest_hello_h, []}, + {"/charsets_provided", charsets_provided_h, []}, + {"/charsets_provided_empty", charsets_provided_empty_h, []}, {"/charset_in_content_types_provided", charset_in_content_types_provided_h, []}, {"/charset_in_content_types_provided_implicit", @@ -148,6 +150,136 @@ charset_in_content_types_provided_implicit_no_callback(Config) -> {_, <<"text/plain">>} = lists:keyfind(<<"content-type">>, 1, Headers), ok. +charsets_provided_match_text(Config) -> + doc("When the media type is text and the charsets_provided callback exists " + "and the accept-charset header was sent, the selected charset is sent " + "back in the content-type of the response."), + ConnPid = gun_open(Config), + Ref = gun:get(ConnPid, "/charsets_provided", [ + {<<"accept">>, <<"text/plain">>}, + {<<"accept-charset">>, <<"utf-8;q=0.5, utf-16">>}, + {<<"accept-encoding">>, <<"gzip">>} + ]), + {response, _, 200, Headers} = gun:await(ConnPid, Ref), + {_, <<"text/plain; charset=utf-16">>} = lists:keyfind(<<"content-type">>, 1, Headers), + ok. + +charsets_provided_match_other(Config) -> + doc("When the media type is not text and the charsets_provided callback exists " + "and the accept-charset header was sent, the selected charset is not sent " + "back in the content-type of the response."), + ConnPid = gun_open(Config), + Ref = gun:get(ConnPid, "/charsets_provided", [ + {<<"accept">>, <<"application/json">>}, + {<<"accept-charset">>, <<"utf-8;q=0.5, utf-16">>}, + {<<"accept-encoding">>, <<"gzip">>} + ]), + {response, _, 200, Headers} = gun:await(ConnPid, Ref), + {_, <<"application/json">>} = lists:keyfind(<<"content-type">>, 1, Headers), + ok. + +charsets_provided_wildcard_text(Config) -> + doc("When the media type is text and the charsets_provided callback exists " + "and a wildcard accept-charset header was sent, the selected charset is sent " + "back in the content-type of the response."), + ConnPid = gun_open(Config), + Ref = gun:get(ConnPid, "/charsets_provided", [ + {<<"accept">>, <<"text/plain">>}, + {<<"accept-charset">>, <<"*">>}, + {<<"accept-encoding">>, <<"gzip">>} + ]), + {response, _, 200, Headers} = gun:await(ConnPid, Ref), + {_, <<"text/plain; charset=utf-8">>} = lists:keyfind(<<"content-type">>, 1, Headers), + ok. + +charsets_provided_wildcard_other(Config) -> + doc("When the media type is not text and the charsets_provided callback exists " + "and a wildcard accept-charset header was sent, the selected charset is not sent " + "back in the content-type of the response."), + ConnPid = gun_open(Config), + Ref = gun:get(ConnPid, "/charsets_provided", [ + {<<"accept">>, <<"application/json">>}, + {<<"accept-charset">>, <<"*">>}, + {<<"accept-encoding">>, <<"gzip">>} + ]), + {response, _, 200, Headers} = gun:await(ConnPid, Ref), + {_, <<"application/json">>} = lists:keyfind(<<"content-type">>, 1, Headers), + ok. + +charsets_provided_nomatch(Config) -> + doc("Regardless of the media type negotiated, if no charset is found in the " + "accept-charset header match a charset configured in charsets_provided, " + "then a 406 not acceptable response is sent back."), + ConnPid = gun_open(Config), + Ref = gun:get(ConnPid, "/charsets_provided", [ + {<<"accept">>, <<"text/plain">>}, + {<<"accept-charset">>, <<"utf-8;q=0, iso-8859-1">>}, + {<<"accept-encoding">>, <<"gzip">>} + ]), + {response, _, 406, _} = gun:await(ConnPid, Ref), + ok. + +charsets_provided_noheader_text(Config) -> + doc("When the media type is text and the charsets_provided callback exists " + "but the accept-charset header was not sent, the first charset in the " + "list is selected and sent back in the content-type of the response."), + ConnPid = gun_open(Config), + Ref = gun:get(ConnPid, "/charsets_provided", [ + {<<"accept">>, <<"text/plain">>}, + {<<"accept-encoding">>, <<"gzip">>} + ]), + {response, _, 200, Headers} = gun:await(ConnPid, Ref), + {_, <<"text/plain; charset=utf-8">>} = lists:keyfind(<<"content-type">>, 1, Headers), + ok. + +charsets_provided_noheader_other(Config) -> + doc("When the media type is not text and the charsets_provided callback exists " + "but the accept-charset header was not sent, the first charset in the " + "list is selected but is not sent back in the content-type of the response."), + ConnPid = gun_open(Config), + Ref = gun:get(ConnPid, "/charsets_provided", [ + {<<"accept">>, <<"application/json">>}, + {<<"accept-encoding">>, <<"gzip">>} + ]), + {response, _, 200, Headers} = gun:await(ConnPid, Ref), + {_, <<"application/json">>} = lists:keyfind(<<"content-type">>, 1, Headers), + ok. + +charsets_provided_empty(Config) -> + doc("Regardless of the media type negotiated, if the charsets_provided " + "callback returns an empty list a 406 not acceptable response is sent back."), + ConnPid = gun_open(Config), + Ref = gun:get(ConnPid, "/charsets_provided_empty", [ + {<<"accept">>, <<"text/plain">>}, + {<<"accept-charset">>, <<"utf-8q=0.5, utf-16">>}, + {<<"accept-encoding">>, <<"gzip">>} + ]), + {response, _, 406, _} = gun:await(ConnPid, Ref), + ok. + +charsets_provided_empty_wildcard(Config) -> + doc("Regardless of the media type negotiated, if the charsets_provided " + "callback returns an empty list a 406 not acceptable response is sent back."), + ConnPid = gun_open(Config), + Ref = gun:get(ConnPid, "/charsets_provided_empty", [ + {<<"accept">>, <<"text/plain">>}, + {<<"accept-charset">>, <<"*">>}, + {<<"accept-encoding">>, <<"gzip">>} + ]), + {response, _, 406, _} = gun:await(ConnPid, Ref), + ok. + +charsets_provided_empty_noheader(Config) -> + doc("Regardless of the media type negotiated, if the charsets_provided " + "callback returns an empty list a 406 not acceptable response is sent back."), + ConnPid = gun_open(Config), + Ref = gun:get(ConnPid, "/charsets_provided_empty", [ + {<<"accept">>, <<"text/plain">>}, + {<<"accept-encoding">>, <<"gzip">>} + ]), + {response, _, 406, _} = gun:await(ConnPid, Ref), + ok. + provide_callback_missing(Config) -> doc("A 500 response must be sent when the ProvideCallback can't be called."), ConnPid = gun_open(Config), -- cgit v1.2.3